From ec4200068c65689ad8da53b3226f10de01dd161f Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Wed, 13 Feb 2013 06:30:06 -0800 Subject: [PATCH] process: allow ticker to cross communicate better Using external memory values allows for quick communication between js and cc land, so we can check if the js land callback needs to be run. (this is where I meant that manually tracking nextTickQueue.length would be helpful) Also did some minor cleanup of removing the old Tick and StartTickSpinner functions, and a few unneeded comments. Conflicts: src/node.cc --- src/node.cc | 87 ++++++++++++++++++++++++++------------------------- src/node.js | 90 ++++++++++++++++++++++++++++------------------------- 2 files changed, 93 insertions(+), 84 deletions(-) diff --git a/src/node.cc b/src/node.cc index c4131069e52..df89f3da323 100644 --- a/src/node.cc +++ b/src/node.cc @@ -100,7 +100,7 @@ Persistent process_symbol; Persistent domain_symbol; static Persistent process; -static Persistent process_tickWDCallback; +static Persistent process_tickDomainCallback; static Persistent process_tickFromSpinner; static Persistent process_tickCallback; @@ -144,6 +144,12 @@ static uv_idle_t idle_immediate_dummy; static bool need_immediate_cb; static Persistent immediate_callback_sym; +// for quick ref to tickCallback values +struct { + uint32_t length; + uint32_t index; + uint32_t depth; +} tick_infobox; #ifdef OPENSSL_NPN_NEGOTIATED static bool use_npn = true; @@ -167,7 +173,10 @@ static uv_async_t dispatch_debug_messages_async; Isolate* node_isolate = NULL; -static void Tick(void) { +static void Spin(uv_idle_t* handle, int status) { + assert((uv_idle_t*) handle == &tick_spinner); + assert(status == 0); + // Avoid entering a V8 scope. if (!need_tick_cb) return; need_tick_cb = false; @@ -196,26 +205,9 @@ static void Tick(void) { } -static void Spin(uv_idle_t* handle, int status) { - assert((uv_idle_t*) handle == &tick_spinner); - assert(status == 0); - Tick(); -} - - -static void StartTickSpinner() { - need_tick_cb = true; - // TODO: this tick_spinner shouldn't be necessary. An ev_prepare should be - // sufficent, the problem is only in the case of the very last "tick" - - // there is nothing left to do in the event loop and libev will exit. The - // ev_prepare callback isn't called before exiting. Thus we start this - // tick_spinner to keep the event loop alive long enough to handle it. - uv_idle_start(&tick_spinner, Spin); -} - - static Handle NeedTickCallback(const Arguments& args) { - StartTickSpinner(); + need_tick_cb = true; + uv_idle_start(&tick_spinner, Spin); return Undefined(node_isolate); } @@ -905,19 +897,19 @@ Handle FromConstructorTemplate(Persistent t, Handle -MCWithDomain(const Handle object, +MakeDomainCallback(const Handle object, const Handle callback, int argc, Handle argv[]) { - // lazy load _tickWDCallback - if (process_tickWDCallback.IsEmpty()) { - Local cb_v = process->Get(String::New("_tickWDCallback")); + // lazy load _tickDomainCallback + if (process_tickDomainCallback.IsEmpty()) { + Local cb_v = process->Get(String::New("_tickDomainCallback")); if (!cb_v->IsFunction()) { - fprintf(stderr, "process._tickWDCallback assigned to non-function\n"); + fprintf(stderr, "process._tickDomainCallback assigned to non-function\n"); abort(); } Local cb = cb_v.As(); - process_tickWDCallback = Persistent::New(cb); + process_tickDomainCallback = Persistent::New(cb); } // lazy load domain specific symbols @@ -965,8 +957,14 @@ MCWithDomain(const Handle object, return Undefined(node_isolate); } + if (tick_infobox.length == 0) { + tick_infobox.index = 0; + tick_infobox.depth = 0; + return ret; + } + // process nextTicks after call - process_tickWDCallback->Call(process, 0, NULL); + process_tickDomainCallback->Call(process, 0, NULL); if (try_catch.HasCaught()) { FatalException(try_catch); @@ -984,17 +982,15 @@ MakeCallback(const Handle object, Handle argv[]) { HandleScope scope; - Local callback_v = object->Get(symbol); - if (!callback_v->IsFunction()) { - String::Utf8Value method(symbol); - fprintf(stderr, "Non-function in MakeCallback. method = %s\n", *method); - abort(); - } - - Local callback = Local::Cast(callback_v); + Local callback = object->Get(symbol).As(); + Local domain = object->Get(domain_symbol); // TODO Hook for long stack traces to be made here. + // has domain, off with you + if (!domain->IsNull() && !domain->IsUndefined()) + return scope.Close(MakeDomainCallback(object, callback, argc, argv)); + // lazy load no domain next tick callbacks if (process_tickCallback.IsEmpty()) { Local cb_v = process->Get(String::New("_tickCallback")); @@ -1006,12 +1002,6 @@ MakeCallback(const Handle object, process_tickCallback = Persistent::New(cb); } - // has domain, off with you - if (object->Has(domain_symbol) && - !object->Get(domain_symbol)->IsNull() && - !object->Get(domain_symbol)->IsUndefined()) - return scope.Close(MCWithDomain(object, callback, argc, argv)); - TryCatch try_catch; Local ret = callback->Call(object, argc, argv); @@ -1021,6 +1011,12 @@ MakeCallback(const Handle object, return Undefined(node_isolate); } + if (tick_infobox.length == 0) { + tick_infobox.index = 0; + tick_infobox.depth = 0; + return scope.Close(ret); + } + // process nextTicks after call process_tickCallback->Call(process, 0, NULL); @@ -2477,6 +2473,13 @@ Handle SetupProcessObject(int argc, char *argv[]) { NODE_SET_METHOD(process, "binding", Binding); + // values use to cross communicate with processNextTick + Local info_box = Object::New(); + info_box->SetIndexedPropertiesToExternalArrayData(&tick_infobox, + kExternalUnsignedIntArray, + 3); + process->Set(String::NewSymbol("_tickInfoBox"), info_box); + return process; } diff --git a/src/node.js b/src/node.js index bc2ff0a292c..3d1adf5af09 100644 --- a/src/node.js +++ b/src/node.js @@ -309,17 +309,24 @@ startup.processNextTick = function() { var _needTickCallback = process._needTickCallback; var nextTickQueue = []; - var nextTickQueueLength = 0; - var nextTickIndex = 0; - var tickDepth = 0; var usingDomains = false; var needSpinner = true; var inTick = false; + // this infobox thing is used so that the C++ code in src/node.cc + // can have easy accesss to our nextTick state, and avoid unnecessary + // calls into process._tickCallback. + // order is [length, index, depth] + // Never write code like this without very good reason! + var infoBox = process._tickInfoBox; + var length = 0; + var index = 1; + var depth = 2; + process._tickCallback = _tickCallback; process._tickFromSpinner = _tickFromSpinner; // needs to be accessible from cc land - process._tickWDCallback = _tickWDCallback; + process._tickDomainCallback = _tickDomainCallback; process.nextTick = nextTick; // the maximum number of times it'll process something like @@ -332,13 +339,13 @@ process.maxTickDepth = 1000; function tickDone(tickDepth_) { - if (nextTickQueueLength !== 0) { - if (nextTickQueueLength <= nextTickIndex) { + if (infoBox[length] !== 0) { + if (infoBox[length] <= infoBox[index]) { nextTickQueue = []; - nextTickQueueLength = 0; + infoBox[length] = 0; } else { - nextTickQueue.splice(0, nextTickIndex); - nextTickQueueLength = nextTickQueue.length; + nextTickQueue.splice(0, infoBox[index]); + infoBox[length] = nextTickQueue.length; if (needSpinner) { _needTickCallback(); needSpinner = false; @@ -346,8 +353,8 @@ } } inTick = false; - nextTickIndex = 0; - tickDepth = tickDepth_; + infoBox[index] = 0; + infoBox[depth] = tickDepth_; } function maxTickWarn() { @@ -364,44 +371,43 @@ function _tickFromSpinner() { needSpinner = true; // coming from spinner, reset! - if (tickDepth !== 0) - tickDepth = 0; + if (infoBox[depth] !== 0) + infoBox[depth] = 0; // no callbacks to run - if (nextTickQueueLength === 0) - return nextTickIndex = tickDepth = 0; - if (nextTickQueue[nextTickQueueLength - 1].domain) - _tickWDCallback(); + if (infoBox[length] === 0) + return infoBox[index] = infoBox[depth] = 0; + if (nextTickQueue[infoBox[length] - 1].domain) + _tickDomainCallback(); else _tickCallback(); } - // run callback that have no domain - // this is very hot, so has been super optimized - // this will be overridden if user uses domains + // run callbacks that have no domain + // using domains will cause this to be overridden function _tickCallback() { var callback, nextTickLength, threw; if (inTick) return; - if (nextTickQueueLength === 0) { - nextTickIndex = 0; - tickDepth = 0; + if (infoBox[length] === 0) { + infoBox[index] = 0; + infoBox[depth] = 0; return; } inTick = true; - while (tickDepth++ < process.maxTickDepth) { - nextTickLength = nextTickQueueLength; - if (nextTickIndex === nextTickLength) + while (infoBox[depth]++ < process.maxTickDepth) { + nextTickLength = infoBox[length]; + if (infoBox[index] === nextTickLength) return tickDone(0); - while (nextTickIndex < nextTickLength) { - callback = nextTickQueue[nextTickIndex++].callback; + while (infoBox[index] < nextTickLength) { + callback = nextTickQueue[infoBox[index]++].callback; threw = true; try { callback(); threw = false; } finally { - if (threw) tickDone(tickDepth); + if (threw) tickDone(infoBox[depth]); } } } @@ -409,7 +415,7 @@ tickDone(0); } - function _tickWDCallback() { + function _tickDomainCallback() { var nextTickLength, tock, callback, threw; // if you add a nextTick in a domain's error handler, then @@ -418,7 +424,7 @@ // that error handler ALSO triggers multiple MakeCallbacks, then // it'll try to keep clearing the queue, since the finally block // fires *before* the error hits the top level and is handled. - if (tickDepth >= process.maxTickDepth) + if (infoBox[depth] >= process.maxTickDepth) return _needTickCallback(); if (inTick) return; @@ -426,15 +432,15 @@ // always do this at least once. otherwise if process.maxTickDepth // is set to some negative value, or if there were repeated errors - // preventing tickDepth from being cleared, we'd never process any + // preventing depth from being cleared, we'd never process any // of them. - while (tickDepth++ < process.maxTickDepth) { - nextTickLength = nextTickQueueLength; - if (nextTickIndex === nextTickLength) + while (infoBox[depth]++ < process.maxTickDepth) { + nextTickLength = infoBox[length]; + if (infoBox[index] === nextTickLength) return tickDone(0); - while (nextTickIndex < nextTickLength) { - tock = nextTickQueue[nextTickIndex++]; + while (infoBox[index] < nextTickLength) { + tock = nextTickQueue[infoBox[index]++]; callback = tock.callback; if (tock.domain) { if (tock.domain._disposed) continue; @@ -446,8 +452,8 @@ threw = false; } finally { // finally blocks fire before the error hits the top level, - // so we can't clear the tickDepth at this point. - if (threw) tickDone(tickDepth); + // so we can't clear the depth at this point. + if (threw) tickDone(infoBox[depth]); } if (tock.domain) { tock.domain.exit(); @@ -462,7 +468,7 @@ // on the way out, don't bother. it won't get fired anyway. if (process._exiting) return; - if (tickDepth >= process.maxTickDepth) + if (infoBox[depth] >= process.maxTickDepth) maxTickWarn(); var obj = { callback: callback }; @@ -470,13 +476,13 @@ obj.domain = process.domain; // user has opt'd to use domains, so override default functionality if (!usingDomains) { - process._tickCallback = _tickWDCallback; + process._tickCallback = _tickDomainCallback; usingDomains = true; } } nextTickQueue.push(obj); - nextTickQueueLength++; + infoBox[length]++; } };