Skip to content

Commit

Permalink
Revert "Move MakeCallback to JS"
Browse files Browse the repository at this point in the history
This reverts commit 0109a9f.

Also included:  Port all the changes to process._makeCallback into the
C++ version.  Immediate nextTick, etc.

This yields a slight boost in several benchmarks.  V8 is optimizing and
deoptimizing process._makeCallback repeatedly.
  • Loading branch information
isaacs committed Feb 16, 2013
1 parent 401cef7 commit 95ac576
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 77 deletions.
71 changes: 51 additions & 20 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Persistent<String> process_symbol;
Persistent<String> domain_symbol;

static Persistent<Object> process;
static Persistent<Function> process_tickCallback;

static Persistent<String> exports_symbol;

Expand All @@ -114,7 +115,9 @@ static Persistent<String> heap_used_symbol;

static Persistent<String> fatal_exception_symbol;

static Persistent<Function> process_makeCallback;
static Persistent<String> enter_symbol;
static Persistent<String> exit_symbol;
static Persistent<String> disposed_symbol;


static bool print_eval = false;
Expand Down Expand Up @@ -926,11 +929,6 @@ 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);
Expand All @@ -945,27 +943,60 @@ MakeCallback(const Handle<Object> object,

TryCatch try_catch;

if (process_makeCallback.IsEmpty()) {
Local<Value> cb_v = process->Get(String::New("_makeCallback"));
if (!cb_v->IsFunction()) {
fprintf(stderr, "process._makeCallback assigned to non-function\n");
abort();
if (enter_symbol.IsEmpty()) {
enter_symbol = NODE_PSYMBOL("enter");
exit_symbol = NODE_PSYMBOL("exit");
disposed_symbol = NODE_PSYMBOL("_disposed");
}

Local<Value> domain_v = object->Get(domain_symbol);
Local<Object> domain;
Local<Function> enter;
Local<Function> exit;
if (!domain_v->IsUndefined()) {
domain = domain_v->ToObject();
if (domain->Get(disposed_symbol)->BooleanValue()) {
// domain has been disposed of.
return Undefined(node_isolate);
}
Local<Function> cb = cb_v.As<Function>();
process_makeCallback = Persistent<Function>::New(cb);
enter = Local<Function>::Cast(domain->Get(enter_symbol));
enter->Call(domain, 0, NULL);
}

Local<Array> argArray = Array::New(argc);
for (int i = 0; i < argc; i++) {
argArray->Set(Integer::New(i, node_isolate), argv[i]);
if (try_catch.HasCaught()) {
FatalException(try_catch);
return Undefined(node_isolate);
}

Local<Value> object_l = Local<Value>::New(node_isolate, object);
Local<Value> symbol_l = Local<Value>::New(node_isolate, symbol);
Local<Function> callback = Local<Function>::Cast(callback_v);
Local<Value> ret = callback->Call(object, argc, argv);

Local<Value> args[3] = { object_l, symbol_l, argArray };
if (try_catch.HasCaught()) {
FatalException(try_catch);
return Undefined(node_isolate);
}

Local<Value> ret = process_makeCallback->Call(process, ARRAY_SIZE(args), args);
if (!domain_v->IsUndefined()) {
exit = Local<Function>::Cast(domain->Get(exit_symbol));
exit->Call(domain, 0, NULL);
}

if (try_catch.HasCaught()) {
FatalException(try_catch);
return Undefined(node_isolate);
}

// process nextTicks after every time we get called.
if (process_tickCallback.IsEmpty()) {
Local<Value> cb_v = process->Get(String::New("_tickCallback"));
if (!cb_v->IsFunction()) {
fprintf(stderr, "process._tickCallback assigned to non-function\n");
abort();
}
Local<Function> cb = cb_v.As<Function>();
process_tickCallback = Persistent<Function>::New(cb);
}
process_tickCallback->Call(process, NULL, 0);

if (try_catch.HasCaught()) {
FatalException(try_catch);
Expand Down
52 changes: 0 additions & 52 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
startup.processAssert();
startup.processConfig();
startup.processNextTick();
startup.processMakeCallback();
startup.processStdio();
startup.processKillAndExit();
startup.processSignalHandlers();
Expand Down Expand Up @@ -297,57 +296,6 @@
});
};

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();
}

// 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();

// process the nextTicks after each time we get called.
process._tickCallback();
return ret;
};
};

startup.processNextTick = function() {
var nextTickQueue = [];
var nextTickIndex = 0;
Expand Down
4 changes: 0 additions & 4 deletions test/message/stdin_messages.out
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ SyntaxError: Strict mode code may not include a with statement
at Socket.EventEmitter.emit (events.js:*:*)
at _stream_readable.js:*:*
at process._tickCallback (node.js:*:*)
at process._makeCallback (node.js:*:*)
42
42

Expand All @@ -27,7 +26,6 @@ Error: hello
at Socket.EventEmitter.emit (events.js:*:*)
at _stream_readable.js:*:*
at process._tickCallback (node.js:*:*)
at process._makeCallback (node.js:*:*)

[stdin]:1
throw new Error("hello")
Expand All @@ -41,7 +39,6 @@ Error: hello
at Socket.EventEmitter.emit (events.js:*:*)
at _stream_readable.js:*:*
at process._tickCallback (node.js:*:*)
at process._makeCallback (node.js:*:*)
100

[stdin]:1
Expand All @@ -56,7 +53,6 @@ ReferenceError: y is not defined
at Socket.EventEmitter.emit (events.js:*:*)
at _stream_readable.js:*:*
at process._tickCallback (node.js:*:*)
at process._makeCallback (node.js:*:*)

[stdin]:1
var ______________________________________________; throw 10
Expand Down
1 change: 0 additions & 1 deletion test/message/timeout_throw.out
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
ReferenceError: undefined_reference_error_maker is not defined
at null._onTimeout (*test*message*timeout_throw.js:*:*)
at Timer.listOnTimeout [as ontimeout] (timers.js:*:*)
at process._makeCallback (node.js:*:*)

0 comments on commit 95ac576

Please sign in to comment.