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 <[email protected]>
nodejs · Mar 31, 2014 · e9ce8fc · e9ce8fc
1 parent 5d2aef1
commit e9ce8fc
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 40 deletions.
diff --git 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
@@ -243,14 +243,14 @@ 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)                          \
   V(process_object, v8::Object)                                               \
   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
@@ -323,17 +323,12 @@ static void Close(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-Local<Object> BuildStatsObject(Environment* env, const uv_stat_t* s) {
+Local<Value> 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<Object> stats = env->stats_constructor_function()->NewInstance();
-  if (stats.IsEmpty()) {
-    return handle_scope.Escape(Local<Object>());
-  }
-
   // 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<Object> 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<Value> val = Integer::New(env->isolate(), s->st_##name);            \
-    if (val.IsEmpty())                                                        \
-      return handle_scope.Escape(Local<Object>());                            \
-    stats->Set(env->name ## _string(), val);                                  \
-  }
+  Local<Value> name = Integer::New(env->isolate(), s->st_##name);             \
+  if (name.IsEmpty())                                                         \
+    return handle_scope.Escape(Local<Object>());                              \
+
   X(dev)
   X(mode)
   X(nlink)
@@ -360,39 +355,67 @@ Local<Object> BuildStatsObject(Environment* env, const uv_stat_t* s) {
   X(rdev)
 # if defined(__POSIX__)
   X(blksize)
+# else
+  Local<Value> blksize = Undefined(env->isolate());
 # endif
 #undef X
 
+  // Numbers.
 #define X(name)                                                               \
-  {                                                                           \
-    Local<Value> val = Number::New(env->isolate(),                            \
-                                   static_cast<double>(s->st_##name));        \
-    if (val.IsEmpty())                                                        \
-      return handle_scope.Escape(Local<Object>());                            \
-    stats->Set(env->name ## _string(), val);                                  \
-  }
+  Local<Value> name = Number::New(env->isolate(),                             \
+                                  static_cast<double>(s->st_##name));         \
+  if (name.IsEmpty())                                                         \
+    return handle_scope.Escape(Local<Object>());                              \
+
   X(ino)
   X(size)
 # if defined(__POSIX__)
   X(blocks)
+# else
+  Local<Value> blocks = Undefined(env->isolate());
 # endif
 #undef X
 
-#define X(name, rec)                                                          \
-  {                                                                           \
-    double msecs = static_cast<double>(s->st_##rec.tv_sec) * 1000;            \
-    msecs += static_cast<double>(s->st_##rec.tv_nsec / 1000000);              \
-    Local<Value> val = v8::Date::New(env->isolate(), msecs);                  \
-    if (val.IsEmpty())                                                        \
-      return handle_scope.Escape(Local<Object>());                            \
-    stats->Set(env->name ## _string(), val);                                  \
-  }
-  X(atime, atim)
-  X(mtime, mtim)
-  X(ctime, ctim)
-  X(birthtime, birthtim)
+  // Dates.
+#define X(name)                                                               \
+  Local<Value> name##_msec =                                                  \
+    Number::New(env->isolate(),                                               \
+        (static_cast<double>(s->st_##name.tv_sec) * 1000) +                   \
+        (static_cast<double>(s->st_##name.tv_nsec / 1000000)));               \
+                                                                              \
+  if (name##_msec.IsEmpty())                                                  \
+    return handle_scope.Escape(Local<Object>());                              \
+
+  X(atim)
+  X(mtim)
+  X(ctim)
+  X(birthtim)
 #undef X
 
+  // Pass stats as the first argument, this is the object we are modifying.
+  Local<Value> 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<Value> stats =
+    env->fs_stats_constructor_function()->NewInstance(ARRAY_SIZE(argv), argv);
+
+  if (stats.IsEmpty())
+    return handle_scope.Escape(Local<Object>());
+
   return handle_scope.Escape(stats);
 }
 
@@ -1081,18 +1104,24 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+void FSInitialize(const FunctionCallbackInfo<Value>& args) {
+  Local<Function> stats_constructor = args[0].As<Function>();
+  assert(stats_constructor->IsFunction());
+
+  Environment* env = Environment::GetCurrent(args.GetIsolate());
+  env->set_fs_stats_constructor_function(stats_constructor);
+}
 
 void InitFs(Handle<Object> target,
             Handle<Value> unused,
             Handle<Context> context,
             void* priv) {
   Environment* env = Environment::GetCurrent(context);
 
-  // Initialize the stats object
-  Local<Function> 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
@@ -126,7 +126,7 @@ void AppendExceptionLine(Environment* env,
 
 NO_RETURN void FatalError(const char* location, const char* message);
 
-v8::Local<v8::Object> BuildStatsObject(Environment* env, const uv_stat_t* s);
+v8::Local<v8::Value> BuildStatsObject(Environment* env, const uv_stat_t* s);
 
 enum Endianness {
   kLittleEndian,  // _Not_ LITTLE_ENDIAN, clashes with endian.h.