node: Do not use fn.apply() in process._makeCallback

nodejs · Jan 30, 2013 · 0168109 · 0168109
1 parent f64d1fe
commit 0168109
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 15 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -908,6 +908,11 @@ MakeCallback(const Handle<Object> object,
              Handle<Value> argv[]) {
   HandleScope scope;
 
+  if (argc > 6) {
+    fprintf(stderr, "node::MakeCallback - Too many args (%d)\n", argc);
+    abort();
+  }
+
   Local<Value> callback_v = object->Get(symbol);
   if (!callback_v->IsFunction()) {
     String::Utf8Value method(symbol);
@@ -918,18 +923,6 @@ MakeCallback(const Handle<Object> object,
     abort();
   }
 
-  Local<Function> callback = Local<Function>::Cast(callback_v);
-
-  return scope.Close(MakeCallback(object, callback, argc, argv));
-}
-
-Handle<Value>
-MakeCallback(const Handle<Object> object,
-             const Handle<Function> callback,
-             int argc,
-             Handle<Value> argv[]) {
-  HandleScope scope;
-
   // TODO Hook for long stack traces to be made here.
 
   TryCatch try_catch;
@@ -950,9 +943,9 @@ MakeCallback(const Handle<Object> object,
   }
 
   Local<Value> object_l = Local<Value>::New(node_isolate, object);
-  Local<Value> callback_l = Local<Value>::New(node_isolate, callback);
+  Local<Value> symbol_l = Local<Value>::New(node_isolate, symbol);
 
-  Local<Value> args[3] = { object_l, callback_l, argArray };
+  Local<Value> args[3] = { object_l, symbol_l, argArray };
 
   Local<Value> ret = process_makeCallback->Call(process, ARRAY_SIZE(args), args);
 

diff --git a/src/node.js b/src/node.js
@@ -296,14 +296,47 @@
   };
 
   startup.processMakeCallback = function() {
+    // Along with EventEmitter.emit, this is the hottest code in node.
+    // Everything that comes from C++ into JS passes through here.
     process._makeCallback = function(obj, fn, args) {
       var domain = obj.domain;
       if (domain) {
         if (domain._disposed) return;
         domain.enter();
       }
 
-      var ret = fn.apply(obj, args);
+      // I know what you're thinking, why not just use fn.apply
+      // Because we hit this function a lot, and really want to make sure
+      // that V8 can optimize it as well as possible.
+      var ret;
+      switch (args.length) {
+        case 0:
+          ret = obj[fn]();
+          break;
+        case 1:
+          ret = obj[fn](args[0]);
+          break;
+        case 2:
+          ret = obj[fn](args[0], args[1]);
+          break;
+        case 3:
+          ret = obj[fn](args[0], args[1], args[2]);
+          break;
+        case 4:
+          ret = obj[fn](args[0], args[1], args[2], args[3]);
+          break;
+        case 5:
+          ret = obj[fn](args[0], args[1], args[2], args[3], args[4]);
+          break;
+        case 6:
+          ret = obj[fn](args[0], args[1], args[2], args[3], args[4], args[5]);
+          break;
+
+        default:
+          // How did we even get here?  This should abort() in C++ land!
+          throw new Error('too many args to makeCallback');
+          break;
+      }
 
       if (domain) domain.exit();