From e9ce8fc82a6d12e790511b62852e820d8be03186 Mon Sep 17 00:00:00 2001 From: James Pickard Date: Thu, 27 Feb 2014 15:45:18 -0500 Subject: [PATCH] fs: improve performance of all stat functions By building the fs.Stats object in JS, which is returned by all fs stat functions, calls to v8::Object::Set() are removed. This also includes creating all associated Date objects in JS, rather than using v8::Date::New(). Both these changes have significant performance gains. Note that the returned value from fs.stat changes slightly for non-POSIX systems. Whereas before the stats object would be missing blocks and blksize keys, it now has these keys with undefined as the value. Signed-off-by: Trevor Norris --- lib/fs.js | 33 +++++++++++++- src/env.h | 2 +- src/node_file.cc | 103 +++++++++++++++++++++++++++---------------- src/node_internals.h | 2 +- 4 files changed, 100 insertions(+), 40 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 4fc1314c879e46..129d4ee192bbdb 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -117,7 +117,38 @@ function nullCheck(path, callback) { return true; } -fs.Stats = binding.Stats; +// Static method to set the stats properties on a Stats object. +fs.Stats = function( + dev, + mode, + nlink, + uid, + gid, + rdev, + ino, + size, + blocks, + atim_msec, + mtim_msec, + ctim_msec, + birthtim_msec) { + this.dev = dev; + this.mode = mode; + this.nlink = nlink; + this.uid = uid; + this.gid = gid; + this.rdev = rdev; + this.ino = ino; + this.size = size; + this.blocks = blocks; + this.atime = new Date(atim_msec); + this.mtime = new Date(mtim_msec); + this.ctime = new Date(ctim_msec); + this.birthtime = new Date(birthtim_msec); +}; + +// Create a C++ binding to the function which creates a Stats object. +binding.FSInitialize(fs.Stats); fs.Stats.prototype._checkModeProperty = function(property) { return ((this.mode & constants.S_IFMT) === property); diff --git a/src/env.h b/src/env.h index 6a3b5944be8bbb..95e1df6e59329c 100644 --- a/src/env.h +++ b/src/env.h @@ -243,6 +243,7 @@ namespace node { V(buffer_constructor_function, v8::Function) \ V(context, v8::Context) \ V(domain_array, v8::Array) \ + V(fs_stats_constructor_function, v8::Function) \ V(gc_info_callback_function, v8::Function) \ V(module_load_list_array, v8::Array) \ V(pipe_constructor_template, v8::FunctionTemplate) \ @@ -250,7 +251,6 @@ namespace node { V(script_context_constructor_template, v8::FunctionTemplate) \ V(script_data_constructor_function, v8::Function) \ V(secure_context_constructor_template, v8::FunctionTemplate) \ - V(stats_constructor_function, v8::Function) \ V(tcp_constructor_template, v8::FunctionTemplate) \ V(tick_callback_function, v8::Function) \ V(tls_wrap_constructor_function, v8::Function) \ diff --git a/src/node_file.cc b/src/node_file.cc index 1d454abb3b4dea..8b18b82c963da9 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -323,17 +323,12 @@ static void Close(const FunctionCallbackInfo& args) { } -Local BuildStatsObject(Environment* env, const uv_stat_t* s) { +Local BuildStatsObject(Environment* env, const uv_stat_t* s) { // If you hit this assertion, you forgot to enter the v8::Context first. assert(env->context() == env->isolate()->GetCurrentContext()); EscapableHandleScope handle_scope(env->isolate()); - Local stats = env->stats_constructor_function()->NewInstance(); - if (stats.IsEmpty()) { - return handle_scope.Escape(Local()); - } - // The code below is very nasty-looking but it prevents a segmentation fault // when people run JS code like the snippet below. It's apparently more // common than you would expect, several people have reported this crash... @@ -345,13 +340,13 @@ Local BuildStatsObject(Environment* env, const uv_stat_t* s) { // // We need to check the return value of Integer::New() and Date::New() // and make sure that we bail out when V8 returns an empty handle. + + // Integers. #define X(name) \ - { \ - Local val = Integer::New(env->isolate(), s->st_##name); \ - if (val.IsEmpty()) \ - return handle_scope.Escape(Local()); \ - stats->Set(env->name ## _string(), val); \ - } + Local name = Integer::New(env->isolate(), s->st_##name); \ + if (name.IsEmpty()) \ + return handle_scope.Escape(Local()); \ + X(dev) X(mode) X(nlink) @@ -360,39 +355,67 @@ Local BuildStatsObject(Environment* env, const uv_stat_t* s) { X(rdev) # if defined(__POSIX__) X(blksize) +# else + Local blksize = Undefined(env->isolate()); # endif #undef X + // Numbers. #define X(name) \ - { \ - Local val = Number::New(env->isolate(), \ - static_cast(s->st_##name)); \ - if (val.IsEmpty()) \ - return handle_scope.Escape(Local()); \ - stats->Set(env->name ## _string(), val); \ - } + Local name = Number::New(env->isolate(), \ + static_cast(s->st_##name)); \ + if (name.IsEmpty()) \ + return handle_scope.Escape(Local()); \ + X(ino) X(size) # if defined(__POSIX__) X(blocks) +# else + Local blocks = Undefined(env->isolate()); # endif #undef X -#define X(name, rec) \ - { \ - double msecs = static_cast(s->st_##rec.tv_sec) * 1000; \ - msecs += static_cast(s->st_##rec.tv_nsec / 1000000); \ - Local val = v8::Date::New(env->isolate(), msecs); \ - if (val.IsEmpty()) \ - return handle_scope.Escape(Local()); \ - stats->Set(env->name ## _string(), val); \ - } - X(atime, atim) - X(mtime, mtim) - X(ctime, ctim) - X(birthtime, birthtim) + // Dates. +#define X(name) \ + Local name##_msec = \ + Number::New(env->isolate(), \ + (static_cast(s->st_##name.tv_sec) * 1000) + \ + (static_cast(s->st_##name.tv_nsec / 1000000))); \ + \ + if (name##_msec.IsEmpty()) \ + return handle_scope.Escape(Local()); \ + + X(atim) + X(mtim) + X(ctim) + X(birthtim) #undef X + // Pass stats as the first argument, this is the object we are modifying. + Local argv[] = { + dev, + mode, + nlink, + uid, + gid, + rdev, + ino, + size, + blocks, + atim_msec, + mtim_msec, + ctim_msec, + birthtim_msec + }; + + // Call out to JavaScript to create the stats object. + Local stats = + env->fs_stats_constructor_function()->NewInstance(ARRAY_SIZE(argv), argv); + + if (stats.IsEmpty()) + return handle_scope.Escape(Local()); + return handle_scope.Escape(stats); } @@ -1081,6 +1104,13 @@ static void FUTimes(const FunctionCallbackInfo& args) { } } +void FSInitialize(const FunctionCallbackInfo& args) { + Local stats_constructor = args[0].As(); + assert(stats_constructor->IsFunction()); + + Environment* env = Environment::GetCurrent(args.GetIsolate()); + env->set_fs_stats_constructor_function(stats_constructor); +} void InitFs(Handle target, Handle unused, @@ -1088,11 +1118,10 @@ void InitFs(Handle target, void* priv) { Environment* env = Environment::GetCurrent(context); - // Initialize the stats object - Local constructor = - FunctionTemplate::New(env->isolate())->GetFunction(); - target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "Stats"), constructor); - env->set_stats_constructor_function(constructor); + // Function which creates a new Stats object. + target->Set( + FIXED_ONE_BYTE_STRING(env->isolate(), "FSInitialize"), + FunctionTemplate::New(env->isolate(), FSInitialize)->GetFunction()); NODE_SET_METHOD(target, "close", Close); NODE_SET_METHOD(target, "open", Open); diff --git a/src/node_internals.h b/src/node_internals.h index 7844a9c11449bc..d38a3f019fbcaa 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -126,7 +126,7 @@ void AppendExceptionLine(Environment* env, NO_RETURN void FatalError(const char* location, const char* message); -v8::Local BuildStatsObject(Environment* env, const uv_stat_t* s); +v8::Local BuildStatsObject(Environment* env, const uv_stat_t* s); enum Endianness { kLittleEndian, // _Not_ LITTLE_ENDIAN, clashes with endian.h.