From d171259377c7d7645a79f23ed623e6fd3139f9e1 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 24 Nov 2015 10:54:50 -0500 Subject: [PATCH 01/29] timers: refactor timers Consolidates the implementation of regular and internal (_unrefActive) timers. Includes an optimization for listOnTimeout() that previously only internal timers had. (_runOnTimeout) Also includes some minor other cleanup. --- lib/timers.js | 277 ++++++++++----------------------- test/message/timeout_throw.out | 1 + 2 files changed, 80 insertions(+), 198 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 36fbff43dbc183..e16f8ef597a017 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -2,7 +2,7 @@ const Timer = process.binding('timer_wrap').Timer; const L = require('internal/linkedlist'); -const assert = require('assert').ok; +const assert = require('assert'); const util = require('util'); const debug = util.debuglog('timer'); const kOnTimeout = Timer.kOnTimeout | 0; @@ -18,28 +18,48 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // and a linked list. This technique is described in the libev manual: // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts -// Object containing all lists, timers -// key = time in milliseconds -// value = list -var lists = {}; +// Object maps containing linked lists of timers, keyed and sorted by their +// duration in milliseconds. +// - key = time in milliseconds +// - value = linked list +const refedLists = {}; +const unrefedLists = {}; +// The difference between these two objects is that the former contains timers +// that will keep the process open if they are the only thing left, while the +// latter will not. -// call this whenever the item is active (not idle) -// it will reset its timeout. -// the main function - creates lists on demand and the watchers associated -// with them. + +// Schedule or re-schedule a timer. +// Call this whenever the item is active (not idle). +// The item must have been enroll()'d first. exports.active = function(item) { + insert(item, false); +}; + +// Internal APIs that need timeouts should use `_unrefActive()` instead of +// `active()` so that they do not unnecessarily keep the process open. +exports._unrefActive = function(item) { + insert(item, true); +}; + + +// The underlying logic for scheduling or re-scheduling a timer. +function insert(item, unrefed) { const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) return; item._idleStart = Timer.now(); var list; + const lists = unrefed ? unrefedLists : refedLists; if (lists[msecs]) { list = lists[msecs]; } else { list = new Timer(); + if (unrefed) list.unref(); + list._unrefed = unrefed; list.start(msecs, 0); L.init(list); @@ -62,55 +82,62 @@ function listOnTimeout() { var now = Timer.now(); debug('now: %s', now); - var diff, first, threw; - while (first = L.peek(list)) { - diff = now - first._idleStart; + var diff, timer, threw; + while (timer = L.peek(list)) { + diff = now - timer._idleStart; if (diff < msecs) { list.start(msecs - diff, 0); debug('%d list wait because diff is %d', msecs, diff); return; } else { - L.remove(first); - assert(first !== L.peek(list)); - - if (!first._onTimeout) continue; - - // v0.4 compatibility: if the timer callback throws and the - // domain or uncaughtException handler ignore the exception, - // other timers that expire on this tick should still run. - // - // https://github.com/joyent/node/issues/2631 - var domain = first.domain; - if (domain && domain._disposed) - continue; - - try { - if (domain) - domain.enter(); - threw = true; - first._called = true; - first._onTimeout(); - if (domain) - domain.exit(); - threw = false; - } finally { - if (threw) { - // We need to continue processing after domain error handling - // is complete, but not by using whatever domain was left over - // when the timeout threw its exception. - var oldDomain = process.domain; - process.domain = null; - process.nextTick(listOnTimeoutNT, list); - process.domain = oldDomain; - } + L.remove(timer); + assert(timer !== L.peek(list)); + + if (!timer._onTimeout) continue; + + var domain = timer.domain; + if (domain) { + + // v0.4 compatibility: if the timer callback throws and the + // domain or uncaughtException handler ignore the exception, + // other timers that expire on this tick should still run. + // + // https://github.com/joyent/node/issues/2631 + if (domain._disposed) + return; + + domain.enter(); } + + timer._called = true; + _runOnTimeout(timer, list); + + if (domain) + domain.exit(); } } debug('%d list empty', msecs); assert(L.isEmpty(list)); list.close(); - delete lists[msecs]; + if (list._unrefed) { + delete unrefedLists[msecs]; + } else { + delete refedLists[msecs]; + } +} + + +// An optimization so that the try/finally only de-optimizes what is in this +// smaller function. +function _runOnTimeout(timer, list) { + var threw = true; + try { + timer._onTimeout(); + threw = false; + } finally { + if (threw) process.nextTick(listOnTimeoutNT, list); + } } @@ -122,12 +149,12 @@ function listOnTimeoutNT(list) { function reuse(item) { L.remove(item); - var list = lists[item._idleTimeout]; + var list = refedLists[item._idleTimeout]; // if empty - reuse the watcher if (list && L.isEmpty(list)) { debug('reuse hit'); list.stop(); - delete lists[item._idleTimeout]; + delete refedLists[item._idleTimeout]; return list; } @@ -135,6 +162,7 @@ function reuse(item) { } +// Remove a timer. Cancels the timeout and resets the relevant timer properties. const unenroll = exports.unenroll = function(item) { var list = reuse(item); if (list) { @@ -146,7 +174,9 @@ const unenroll = exports.unenroll = function(item) { }; -// Does not start the time, just sets up the members needed. +// Make a regular object able to act as a timer by setting some properties. +// This function does not start the timer, see `active()`. +// Using existing objects as timers slightly reduces object overhead. exports.enroll = function(item, msecs) { if (typeof msecs !== 'number') { throw new TypeError('"msecs" argument must be a number'); @@ -495,152 +525,3 @@ exports.clearImmediate = function(immediate) { process._needImmediateCallback = false; } }; - - -// Internal APIs that need timeouts should use timers._unrefActive instead of -// timers.active as internal timeouts shouldn't hold the loop open - -var unrefList, unrefTimer; - -function _makeTimerTimeout(timer) { - var domain = timer.domain; - var msecs = timer._idleTimeout; - - L.remove(timer); - - // Timer has been unenrolled by another timer that fired at the same time, - // so don't make it timeout. - if (msecs <= 0) - return; - - if (!timer._onTimeout) - return; - - if (domain) { - if (domain._disposed) - return; - - domain.enter(); - } - - debug('unreftimer firing timeout'); - timer._called = true; - _runOnTimeout(timer); - - if (domain) - domain.exit(); -} - -function _runOnTimeout(timer) { - var threw = true; - try { - timer._onTimeout(); - threw = false; - } finally { - if (threw) process.nextTick(unrefTimeout); - } -} - -function unrefTimeout() { - var now = Timer.now(); - - debug('unrefTimer fired'); - - var timeSinceLastActive; - var nextTimeoutTime; - var nextTimeoutDuration; - var minNextTimeoutTime = TIMEOUT_MAX; - var timersToTimeout = []; - - // The actual timer fired and has not yet been rearmed, - // let's consider its next firing time is invalid for now. - // It may be set to a relevant time in the future once - // we scanned through the whole list of timeouts and if - // we find a timeout that needs to expire. - unrefTimer.when = -1; - - // Iterate over the list of timeouts, - // call the onTimeout callback for those expired, - // and rearm the actual timer if the next timeout to expire - // will expire before the current actual timer. - var cur = unrefList._idlePrev; - while (cur !== unrefList) { - timeSinceLastActive = now - cur._idleStart; - - if (timeSinceLastActive < cur._idleTimeout) { - // This timer hasn't expired yet, but check if its expiring time is - // earlier than the actual timer's expiring time - - nextTimeoutDuration = cur._idleTimeout - timeSinceLastActive; - nextTimeoutTime = now + nextTimeoutDuration; - if (minNextTimeoutTime === TIMEOUT_MAX || - (nextTimeoutTime < minNextTimeoutTime)) { - // We found a timeout that will expire earlier, - // store its next timeout time now so that we - // can rearm the actual timer accordingly when - // we scanned through the whole list. - minNextTimeoutTime = nextTimeoutTime; - } - } else { - // We found a timer that expired. Do not call its _onTimeout callback - // right now, as it could mutate any item of the list (including itself). - // Instead, add it to another list that will be processed once the list - // of current timers has been fully traversed. - timersToTimeout.push(cur); - } - - cur = cur._idlePrev; - } - - var nbTimersToTimeout = timersToTimeout.length; - for (var timerIdx = 0; timerIdx < nbTimersToTimeout; ++timerIdx) - _makeTimerTimeout(timersToTimeout[timerIdx]); - - - // Rearm the actual timer with the timeout delay - // of the earliest timeout found. - if (minNextTimeoutTime !== TIMEOUT_MAX) { - unrefTimer.start(minNextTimeoutTime - now, 0); - unrefTimer.when = minNextTimeoutTime; - debug('unrefTimer rescheduled'); - } else if (L.isEmpty(unrefList)) { - debug('unrefList is empty'); - } -} - - -exports._unrefActive = function(item) { - var msecs = item._idleTimeout; - if (!msecs || msecs < 0) return; - assert(msecs >= 0); - - L.remove(item); - - if (!unrefList) { - debug('unrefList initialized'); - unrefList = {}; - L.init(unrefList); - - debug('unrefTimer initialized'); - unrefTimer = new Timer(); - unrefTimer.unref(); - unrefTimer.when = -1; - unrefTimer[kOnTimeout] = unrefTimeout; - } - - var now = Timer.now(); - item._idleStart = now; - - var when = now + msecs; - - // If the actual timer is set to fire too late, or not set to fire at all, - // we need to make it fire earlier - if (unrefTimer.when === -1 || unrefTimer.when > when) { - unrefTimer.start(msecs, 0); - unrefTimer.when = when; - debug('unrefTimer scheduled'); - } - - debug('unrefList append to end'); - L.append(unrefList, item); -}; diff --git a/test/message/timeout_throw.out b/test/message/timeout_throw.out index 46d9cd2630142a..28e0ba4c4cc248 100644 --- a/test/message/timeout_throw.out +++ b/test/message/timeout_throw.out @@ -3,4 +3,5 @@ ^ ReferenceError: undefined_reference_error_maker is not defined at null._onTimeout (*test*message*timeout_throw.js:*:*) + at _runOnTimeout (timers.js:*:*) at Timer.listOnTimeout (timers.js:*:*) From ba5fd2cdfa831e3a35b29ecfe7994ab30f8269d7 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 24 Nov 2015 10:57:27 -0500 Subject: [PATCH 02/29] timers: greatly improve code comments Describes the How and Why of the timers implementation, as well as adding comments in spots that should allow for an easier understanding of what is going on. The timers implementation is very efficient, at a cost. That cost is readable understandability, and this aims to improve that. --- lib/timers.js | 54 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index e16f8ef597a017..d71619483447f3 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -10,13 +10,34 @@ const kOnTimeout = Timer.kOnTimeout | 0; // Timeout values > TIMEOUT_MAX are set to 1. const TIMEOUT_MAX = 2147483647; // 2^31-1 -// IDLE TIMEOUTS + +// HOW and WHY the timers implementation works the way it does. +// +// Since many parts of Node.js and user applications rely on timeouts, there may +// be a very large amount of timeouts scheduled at any given time. +// Therefore, it is very important that the timers implementation is very fast +// and efficient. +// +// Note: It is suggested you first read though the lib/internal/linkedlist.js +// linked list implementation. How it works is somewhat counter-intuitive to +// most JavaScript code, and timers depend on it extensively. +// +// To achieve a high level of efficiency, the implementation is as distributed +// and lazy as possible. +// +// Object maps are kept which contain linked lists keyed by their duration in +// milliseconds. +// The linked lists within are TimerWrap C++ handles with a linked list appended +// to their JavaScript representation. // -// Because often many sockets will have the same idle timeout we will not -// use one timeout watcher per item. It is too much overhead. Instead -// we'll use a single watcher for all sockets with the same timeout value -// and a linked list. This technique is described in the libev manual: +// With this, each list of timers with the same duration reduces the overhead of +// trying to track when all of the timers should timeout by only tracking the +// first timeout, and adjusting to sooner or later timeouts when applicable. +// This technique is described in the libev manual: // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts +// +// The timers are distributed into separate lists by milliseconds primarly to +// reduce the impact of the linked list's O(n) (linear) insertion time. // Object maps containing linked lists of timers, keyed and sorted by their @@ -45,6 +66,9 @@ exports._unrefActive = function(item) { // The underlying logic for scheduling or re-scheduling a timer. +// +// Inserts a timer into an existing timers list, or creates a new TimerWrap +// backed list if one does not already exist for the specified timeout duration. function insert(item, unrefed) { const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) return; @@ -54,9 +78,13 @@ function insert(item, unrefed) { var list; const lists = unrefed ? unrefedLists : refedLists; + // Use an existing list if there is one, otherwise we need to make a new one. if (lists[msecs]) { list = lists[msecs]; } else { + // Make a new linked list of timers. + // The list object is a TimerWrap, a C++ handle backing the timers for + // efficiency, and the linked list is appended onto it. list = new Timer(); if (unrefed) list.unref(); list._unrefed = unrefed; @@ -85,11 +113,18 @@ function listOnTimeout() { var diff, timer, threw; while (timer = L.peek(list)) { diff = now - timer._idleStart; + + // Check if this loop iteration is too early for the timer. + // This happens if one of two things is true: + // - There are more timers scheduled for later in the list. (Most common) + // - The earliest timer was canceled (unenrolled). (Less common) if (diff < msecs) { list.start(msecs - diff, 0); debug('%d list wait because diff is %d', msecs, diff); return; } else { + // The actual logic for when a timeout happens. + L.remove(timer); assert(timer !== L.peek(list)); @@ -117,6 +152,9 @@ function listOnTimeout() { } } + // If `L.peek(list)` returned nothing, the list was either empty or we have + // called all of the timer timeouts. + // As such, we can remove the timer list. (The backing handle) debug('%d list empty', msecs); assert(L.isEmpty(list)); list.close(); @@ -146,6 +184,12 @@ function listOnTimeoutNT(list) { } +// A convenience function for re-using timer handles (lists) more easily. +// +// This mostly exists to fix https://github.com/nodejs/node/issues/1264. +// Handles in libuv take at least one `uv_run` to be registered as unreferenced. +// Re-using an existing handle allows us to skip that, so that a second `uv_run` +// will return no active handles, even when running `setTimeout(fn).unref()`. function reuse(item) { L.remove(item); From b9870e1519204b321708f7fddb290613030835c2 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 25 Nov 2015 11:17:23 -0500 Subject: [PATCH 03/29] timers: (fixup) refactor insert a bit more See: https://github.com/nodejs/node/pull/4007#discussion_r45770232 --- lib/timers.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index d71619483447f3..1d9a275b737966 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -75,13 +75,11 @@ function insert(item, unrefed) { item._idleStart = Timer.now(); - var list; const lists = unrefed ? unrefedLists : refedLists; // Use an existing list if there is one, otherwise we need to make a new one. - if (lists[msecs]) { - list = lists[msecs]; - } else { + var list = lists[msecs]; + if (!list) { // Make a new linked list of timers. // The list object is a TimerWrap, a C++ handle backing the timers for // efficiency, and the linked list is appended onto it. From acb53d04c14a9c6dffb93948fc2627138e76f4e5 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 25 Nov 2015 14:04:05 -0500 Subject: [PATCH 04/29] timers: (fixup) remove some more unnecessary indentation --- lib/timers.js | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 1d9a275b737966..b059004c7bda0c 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -120,34 +120,34 @@ function listOnTimeout() { list.start(msecs - diff, 0); debug('%d list wait because diff is %d', msecs, diff); return; - } else { - // The actual logic for when a timeout happens. + } - L.remove(timer); - assert(timer !== L.peek(list)); + // The actual logic for when a timeout happens. - if (!timer._onTimeout) continue; + L.remove(timer); + assert(timer !== L.peek(list)); - var domain = timer.domain; - if (domain) { + if (!timer._onTimeout) continue; - // v0.4 compatibility: if the timer callback throws and the - // domain or uncaughtException handler ignore the exception, - // other timers that expire on this tick should still run. - // - // https://github.com/joyent/node/issues/2631 - if (domain._disposed) - return; + var domain = timer.domain; + if (domain) { - domain.enter(); - } + // v0.4 compatibility: if the timer callback throws and the + // domain or uncaughtException handler ignore the exception, + // other timers that expire on this tick should still run. + // + // https://github.com/joyent/node/issues/2631 + if (domain._disposed) + return; - timer._called = true; - _runOnTimeout(timer, list); - - if (domain) - domain.exit(); + domain.enter(); } + + timer._called = true; + _runOnTimeout(timer, list); + + if (domain) + domain.exit(); } // If `L.peek(list)` returned nothing, the list was either empty or we have From 6493176f67e1a1c96fc21ae452049609348ecfdc Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 26 Nov 2015 17:46:50 -0500 Subject: [PATCH 05/29] timers: (fixup) fix my understanding of how timers work --- lib/timers.js | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index b059004c7bda0c..c294b96e540d50 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -22,22 +22,32 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // linked list implementation. How it works is somewhat counter-intuitive to // most JavaScript code, and timers depend on it extensively. // -// To achieve a high level of efficiency, the implementation is as distributed -// and lazy as possible. +// In order to be as efficient as possible, the implementation is entirely lazy. // // Object maps are kept which contain linked lists keyed by their duration in // milliseconds. // The linked lists within are TimerWrap C++ handles with a linked list appended // to their JavaScript representation. // -// With this, each list of timers with the same duration reduces the overhead of -// trying to track when all of the timers should timeout by only tracking the -// first timeout, and adjusting to sooner or later timeouts when applicable. -// This technique is described in the libev manual: +// With this, constant-time insertion (append), removal, and timeout is possible +// in the JavaScript layer. Any one list of timers is able to be sorted by just +// appending to it because all timers share the same duration. Therefore, any +// timer added later will always have been scheduled to timeouts later, thus +// only needing to be appended. Any less-than constant overhead is left for a +// lower and inherently faster layer to handle. +// There is, however, a minor overhead from accessing the specific list from the +// map, or creating a new one, but it is trivial in comparison. +// Removal from an object-property linked list is also constant-time as can be +// seen in the lib/internal/linkedlist.js implementation. +// Timeouts only need to process any timers due to currently timeout, which will +// always be at the beginning of the list for reasons stated above. Any timers +// after the first one encountered that does not yet need to timeout will also +// always be due to timeout at a later time. +// +// The technique is described (in lesser detail) within the libev manual: // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts // -// The timers are distributed into separate lists by milliseconds primarly to -// reduce the impact of the linked list's O(n) (linear) insertion time. +// This is literally about as efficient as timer implementations get. // Object maps containing linked lists of timers, keyed and sorted by their @@ -67,8 +77,9 @@ exports._unrefActive = function(item) { // The underlying logic for scheduling or re-scheduling a timer. // -// Inserts a timer into an existing timers list, or creates a new TimerWrap -// backed list if one does not already exist for the specified timeout duration. +// Appends a timer onto the end of an existing timers list, or creates a new +// TimerWrap backed list if one does not already exist for the specified timeout +// duration. function insert(item, unrefed) { const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) return; From e8db191ed826106abb8e2b6d5851f5ed32c8b01a Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 27 Nov 2015 12:27:56 -0500 Subject: [PATCH 06/29] timers: (fixup) handle domains properly again --- lib/timers.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/timers.js b/lib/timers.js index c294b96e540d50..07d3d04f3eaac5 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -183,7 +183,16 @@ function _runOnTimeout(timer, list) { timer._onTimeout(); threw = false; } finally { - if (threw) process.nextTick(listOnTimeoutNT, list); + if (!threw) return; + + // We need to continue processing after domain error handling + // is complete, but not by using whatever domain was left over + // when the timeout threw its exception. + const domain = process.domain; + process.domain = null; + // If we threw, we need to process the rest of the list in nextTick. + process.nextTick(listOnTimeoutNT, list); + process.domain = domain; } } From 189c148eac2166c3cf28525051479d79d656a6d3 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 2 Dec 2015 11:45:24 -0500 Subject: [PATCH 07/29] timers: (fixup) clean up code comments --- lib/timers.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 07d3d04f3eaac5..8dcd613018c7ed 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -22,7 +22,8 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // linked list implementation. How it works is somewhat counter-intuitive to // most JavaScript code, and timers depend on it extensively. // -// In order to be as efficient as possible, the implementation is entirely lazy. +// In order to be as efficient as possible, the implementation is does as little +// as possible, and only when required. // // Object maps are kept which contain linked lists keyed by their duration in // milliseconds. @@ -32,9 +33,9 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // With this, constant-time insertion (append), removal, and timeout is possible // in the JavaScript layer. Any one list of timers is able to be sorted by just // appending to it because all timers share the same duration. Therefore, any -// timer added later will always have been scheduled to timeouts later, thus -// only needing to be appended. Any less-than constant overhead is left for a -// lower and inherently faster layer to handle. +// timer added later will always have been scheduled to timeout later, thus +// only needing to be appended. Any less-than constant overhead is contained +// within the TimerWrap's inherently faster libuv bindings. // There is, however, a minor overhead from accessing the specific list from the // map, or creating a new one, but it is trivial in comparison. // Removal from an object-property linked list is also constant-time as can be @@ -46,8 +47,6 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // // The technique is described (in lesser detail) within the libev manual: // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts -// -// This is literally about as efficient as timer implementations get. // Object maps containing linked lists of timers, keyed and sorted by their @@ -163,7 +162,7 @@ function listOnTimeout() { // If `L.peek(list)` returned nothing, the list was either empty or we have // called all of the timer timeouts. - // As such, we can remove the timer list. (The backing handle) + // As such, we can remove and clean up the timer list. (The TimerWrap) debug('%d list empty', msecs); assert(L.isEmpty(list)); list.close(); From 339ee8c4415345c91aa7b60573376a6276848757 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 2 Dec 2015 11:46:32 -0500 Subject: [PATCH 08/29] timers: (fixup) correct domains dispose thing --- lib/timers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 8dcd613018c7ed..eed0c6cd13a472 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -142,13 +142,13 @@ function listOnTimeout() { var domain = timer.domain; if (domain) { - // v0.4 compatibility: if the timer callback throws and the + // If the timer callback throws and the // domain or uncaughtException handler ignore the exception, // other timers that expire on this tick should still run. // // https://github.com/joyent/node/issues/2631 if (domain._disposed) - return; + continue; domain.enter(); } From 234b5805bc81b5992e91dc127465abd122c70b03 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 16 Dec 2015 18:12:03 -0500 Subject: [PATCH 09/29] timers: (fixup) address bnoordhuis nits --- lib/timers.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index eed0c6cd13a472..17f0fbcb6b4284 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -44,9 +44,6 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // always be at the beginning of the list for reasons stated above. Any timers // after the first one encountered that does not yet need to timeout will also // always be due to timeout at a later time. -// -// The technique is described (in lesser detail) within the libev manual: -// http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts // Object maps containing linked lists of timers, keyed and sorted by their @@ -85,7 +82,7 @@ function insert(item, unrefed) { item._idleStart = Timer.now(); - const lists = unrefed ? unrefedLists : refedLists; + const lists = unrefed === true ? unrefedLists : refedLists; // Use an existing list if there is one, otherwise we need to make a new one. var list = lists[msecs]; @@ -94,7 +91,7 @@ function insert(item, unrefed) { // The list object is a TimerWrap, a C++ handle backing the timers for // efficiency, and the linked list is appended onto it. list = new Timer(); - if (unrefed) list.unref(); + if (unrefed === true) list.unref(); list._unrefed = unrefed; list.start(msecs, 0); From 7e6bb6d084aa9c53e2b92dfeee030cb353df6c9c Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 18 Dec 2015 16:08:43 -0500 Subject: [PATCH 10/29] timers: (fixup) some more nits --- lib/timers.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 17f0fbcb6b4284..59d537df68f7ab 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -150,8 +150,7 @@ function listOnTimeout() { domain.enter(); } - timer._called = true; - _runOnTimeout(timer, list); + _tryOnTimeout(timer, list); if (domain) domain.exit(); @@ -163,7 +162,7 @@ function listOnTimeout() { debug('%d list empty', msecs); assert(L.isEmpty(list)); list.close(); - if (list._unrefed) { + if (list._unrefed === true) { delete unrefedLists[msecs]; } else { delete refedLists[msecs]; @@ -173,7 +172,8 @@ function listOnTimeout() { // An optimization so that the try/finally only de-optimizes what is in this // smaller function. -function _runOnTimeout(timer, list) { +function _tryOnTimeout(timer, list) { + timer._called = true; var threw = true; try { timer._onTimeout(); From 65a2c8f15b67e2d2633a4f09a633db8047e2cddd Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 14 Jan 2016 12:05:38 -0500 Subject: [PATCH 11/29] timers: (fixup) fix message test --- test/message/timeout_throw.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/message/timeout_throw.out b/test/message/timeout_throw.out index 28e0ba4c4cc248..eed7ec273f1ea9 100644 --- a/test/message/timeout_throw.out +++ b/test/message/timeout_throw.out @@ -3,5 +3,5 @@ ^ ReferenceError: undefined_reference_error_maker is not defined at null._onTimeout (*test*message*timeout_throw.js:*:*) - at _runOnTimeout (timers.js:*:*) + at _tryOnTimeout (timers.js:*:*) at Timer.listOnTimeout (timers.js:*:*) From c3cb93995febfa736b6ab2d2c241c1c2db7861f5 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 14 Jan 2016 12:06:24 -0500 Subject: [PATCH 12/29] timers: (fixup) add a debuglog statement --- lib/timers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/timers.js b/lib/timers.js index 59d537df68f7ab..9bcded3d1b0a54 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -87,6 +87,7 @@ function insert(item, unrefed) { // Use an existing list if there is one, otherwise we need to make a new one. var list = lists[msecs]; if (!list) { + debug('no %d list was found in insert, creating a new one', msecs); // Make a new linked list of timers. // The list object is a TimerWrap, a C++ handle backing the timers for // efficiency, and the linked list is appended onto it. From 298d30fd2fb4e57babc2f775e5254531b4d0149a Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 14 Jan 2016 12:08:13 -0500 Subject: [PATCH 13/29] timers: (fixup) rvagg's list optimization Prevents the JS representation of TimerWraps from deopting from being a linked list. --- lib/timers.js | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 9bcded3d1b0a54..eaf1f0b4609c45 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -91,16 +91,21 @@ function insert(item, unrefed) { // Make a new linked list of timers. // The list object is a TimerWrap, a C++ handle backing the timers for // efficiency, and the linked list is appended onto it. - list = new Timer(); - if (unrefed === true) list.unref(); - list._unrefed = unrefed; - list.start(msecs, 0); - + list = { + _idleNext: null, // Create the list object with the linkedlist properties + _idlePrev: null, // to prevent any unecessary hidden class changes. + _timer: new Timer(), + _unrefed: unrefed, + msecs: msecs + }; L.init(list); + list._timer._list = list; + + if (unrefed === true) list._timer.unref(); + list._timer.start(msecs, 0); lists[msecs] = list; - list.msecs = msecs; - list[kOnTimeout] = listOnTimeout; + list._timer[kOnTimeout] = listOnTimeout; } L.append(list, item); @@ -108,8 +113,8 @@ function insert(item, unrefed) { }; function listOnTimeout() { - var msecs = this.msecs; - var list = this; + var list = this._list; + var msecs = list.msecs; debug('timeout callback %d', msecs); @@ -125,7 +130,7 @@ function listOnTimeout() { // - There are more timers scheduled for later in the list. (Most common) // - The earliest timer was canceled (unenrolled). (Less common) if (diff < msecs) { - list.start(msecs - diff, 0); + this.start(msecs - diff, 0); debug('%d list wait because diff is %d', msecs, diff); return; } @@ -162,7 +167,7 @@ function listOnTimeout() { // As such, we can remove and clean up the timer list. (The TimerWrap) debug('%d list empty', msecs); assert(L.isEmpty(list)); - list.close(); + this.close(); if (list._unrefed === true) { delete unrefedLists[msecs]; } else { @@ -195,7 +200,7 @@ function _tryOnTimeout(timer, list) { function listOnTimeoutNT(list) { - list[kOnTimeout](); + list._timer[kOnTimeout](); } @@ -212,9 +217,9 @@ function reuse(item) { // if empty - reuse the watcher if (list && L.isEmpty(list)) { debug('reuse hit'); - list.stop(); + list._timer.stop(); delete refedLists[item._idleTimeout]; - return list; + return list._timer; } return null; @@ -223,10 +228,10 @@ function reuse(item) { // Remove a timer. Cancels the timeout and resets the relevant timer properties. const unenroll = exports.unenroll = function(item) { - var list = reuse(item); - if (list) { + var handle = reuse(item); + if (handle) { debug('unenroll: list empty'); - list.close(); + handle.close(); } // if active is called later, then we want to make sure not to insert again item._idleTimeout = -1; From 93e29e1a8db0d7e713c82930aea9ead6bb4ff830 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 8 Feb 2016 13:17:32 -0500 Subject: [PATCH 14/29] timers: (fixup) Update code comments. I am going to hate squashing this. --- lib/timers.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index eaf1f0b4609c45..a506bd2cd91126 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -27,21 +27,22 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // // Object maps are kept which contain linked lists keyed by their duration in // milliseconds. -// The linked lists within are TimerWrap C++ handles with a linked list appended -// to their JavaScript representation. +// The linked lists within also have some meta-properties, one of which is a +// TimerWrap C++ handle, which makes the call after the duration to proceess the +// list it is attached to. // -// With this, constant-time insertion (append), removal, and timeout is possible -// in the JavaScript layer. Any one list of timers is able to be sorted by just -// appending to it because all timers share the same duration. Therefore, any -// timer added later will always have been scheduled to timeout later, thus -// only needing to be appended. Any less-than constant overhead is contained -// within the TimerWrap's inherently faster libuv bindings. +// With this, virtually constant-time insertion (append), removal, and timeout +// is possible in the JavaScript layer. Any one list of timers is able to be +// sorted by just appending to it because all timers within share the same +// duration. Therefore, any timer added later will always have been scheduled to +// timeout later, thus only needing to be appended. Any less-than constant +// overhead is contained within the TimerWrap's inherently faster libuv binding. // There is, however, a minor overhead from accessing the specific list from the // map, or creating a new one, but it is trivial in comparison. // Removal from an object-property linked list is also constant-time as can be // seen in the lib/internal/linkedlist.js implementation. // Timeouts only need to process any timers due to currently timeout, which will -// always be at the beginning of the list for reasons stated above. Any timers +// always be at the beginning of the list for reasons stated above. Any timers // after the first one encountered that does not yet need to timeout will also // always be due to timeout at a later time. @@ -88,9 +89,8 @@ function insert(item, unrefed) { var list = lists[msecs]; if (!list) { debug('no %d list was found in insert, creating a new one', msecs); - // Make a new linked list of timers. - // The list object is a TimerWrap, a C++ handle backing the timers for - // efficiency, and the linked list is appended onto it. + // Make a new linked list of timers, and create a TimerWrap to schedule + // processing for the list. list = { _idleNext: null, // Create the list object with the linkedlist properties _idlePrev: null, // to prevent any unecessary hidden class changes. @@ -149,7 +149,7 @@ function listOnTimeout() { // domain or uncaughtException handler ignore the exception, // other timers that expire on this tick should still run. // - // https://github.com/joyent/node/issues/2631 + // https://github.com/nodejs/node-v0.x-archive/issues/2631 if (domain._disposed) continue; @@ -164,7 +164,7 @@ function listOnTimeout() { // If `L.peek(list)` returned nothing, the list was either empty or we have // called all of the timer timeouts. - // As such, we can remove and clean up the timer list. (The TimerWrap) + // As such, we can remove the list and clean up the TimerWrap C++ handle. debug('%d list empty', msecs); assert(L.isEmpty(list)); this.close(); @@ -204,7 +204,7 @@ function listOnTimeoutNT(list) { } -// A convenience function for re-using timer handles (lists) more easily. +// A convenience function for re-using TimerWrap handles more easily. // // This mostly exists to fix https://github.com/nodejs/node/issues/1264. // Handles in libuv take at least one `uv_run` to be registered as unreferenced. From aa31fef40714a9cc01d98cb0b67cc580c154e56b Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 8 Feb 2016 13:20:33 -0500 Subject: [PATCH 15/29] timers: (fixup) rename Timer to TimerWrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminate this confusion once and for all. Only ever refer to the C++ handles as “TimerWrap(s)”. --- lib/timers.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index a506bd2cd91126..683df26a719d81 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -1,11 +1,11 @@ 'use strict'; -const Timer = process.binding('timer_wrap').Timer; +const TimerWrap = process.binding('timer_wrap').Timer; const L = require('internal/linkedlist'); const assert = require('assert'); const util = require('util'); const debug = util.debuglog('timer'); -const kOnTimeout = Timer.kOnTimeout | 0; +const kOnTimeout = TimerWrap.kOnTimeout | 0; // Timeout values > TIMEOUT_MAX are set to 1. const TIMEOUT_MAX = 2147483647; // 2^31-1 @@ -81,7 +81,7 @@ function insert(item, unrefed) { const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) return; - item._idleStart = Timer.now(); + item._idleStart = TimerWrap.now(); const lists = unrefed === true ? unrefedLists : refedLists; @@ -94,7 +94,7 @@ function insert(item, unrefed) { list = { _idleNext: null, // Create the list object with the linkedlist properties _idlePrev: null, // to prevent any unecessary hidden class changes. - _timer: new Timer(), + _timer: new TimerWrap(), _unrefed: unrefed, msecs: msecs }; @@ -118,7 +118,7 @@ function listOnTimeout() { debug('timeout callback %d', msecs); - var now = Timer.now(); + var now = TimerWrap.now(); debug('now: %s', now); var diff, timer, threw; @@ -422,7 +422,7 @@ Timeout.prototype.unref = function() { if (this._handle) { this._handle.unref(); } else if (typeof this._onTimeout === 'function') { - var now = Timer.now(); + var now = TimerWrap.now(); if (!this._idleStart) this._idleStart = now; var delay = this._idleStart + this._idleTimeout - now; if (delay < 0) delay = 0; @@ -435,7 +435,7 @@ Timeout.prototype.unref = function() { var handle = reuse(this); - this._handle = handle || new Timer(); + this._handle = handle || new TimerWrap(); this._handle.owner = this; this._handle[kOnTimeout] = unrefdHandle; this._handle.start(delay, 0); From 61dd4abe5fef6b3612aa5ddc498559bd03a20f93 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 10 Feb 2016 10:36:50 -0500 Subject: [PATCH 16/29] timers: (fixup) spelling nits --- lib/timers.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 683df26a719d81..b1bfa2a43eeda0 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -22,13 +22,13 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // linked list implementation. How it works is somewhat counter-intuitive to // most JavaScript code, and timers depend on it extensively. // -// In order to be as efficient as possible, the implementation is does as little -// as possible, and only when required. +// In order to be as efficient as possible, the implementation does as little as +// possible, and only when required. // // Object maps are kept which contain linked lists keyed by their duration in // milliseconds. // The linked lists within also have some meta-properties, one of which is a -// TimerWrap C++ handle, which makes the call after the duration to proceess the +// TimerWrap C++ handle, which makes the call after the duration to process the // list it is attached to. // // With this, virtually constant-time insertion (append), removal, and timeout @@ -93,7 +93,7 @@ function insert(item, unrefed) { // processing for the list. list = { _idleNext: null, // Create the list object with the linkedlist properties - _idlePrev: null, // to prevent any unecessary hidden class changes. + _idlePrev: null, // to prevent any unnecessary hidden class changes. _timer: new TimerWrap(), _unrefed: unrefed, msecs: msecs From 06ab42e0353cc6ef6d5ce18ea866c16bc0ed520e Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 19 Feb 2016 16:17:46 -0500 Subject: [PATCH 17/29] timers: (fixup) remove "active" comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I dunno how to make this comment useful… --- lib/timers.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/timers.js b/lib/timers.js index b1bfa2a43eeda0..d07f962d9399fa 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -59,7 +59,6 @@ const unrefedLists = {}; // Schedule or re-schedule a timer. -// Call this whenever the item is active (not idle). // The item must have been enroll()'d first. exports.active = function(item) { insert(item, false); From aaf7e264ea390dc781aeb23c71518753ab323a70 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 19 Feb 2016 22:16:47 -0500 Subject: [PATCH 18/29] timers: (fixup) fix comment --- lib/timers.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index d07f962d9399fa..2ea936d938d4e6 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -124,10 +124,8 @@ function listOnTimeout() { while (timer = L.peek(list)) { diff = now - timer._idleStart; - // Check if this loop iteration is too early for the timer. - // This happens if one of two things is true: - // - There are more timers scheduled for later in the list. (Most common) - // - The earliest timer was canceled (unenrolled). (Less common) + // Check if this loop iteration is too early for the next timer. + // This happens if there are more timers scheduled for later in the list. if (diff < msecs) { this.start(msecs - diff, 0); debug('%d list wait because diff is %d', msecs, diff); From e28efc9b53e74e66fd32aed20ca2fbd368b4646f Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 23 Feb 2016 16:55:25 -0500 Subject: [PATCH 19/29] timers: (fixup) move a code comment --- lib/timers.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 2ea936d938d4e6..fb5d74be4886ee 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -49,13 +49,15 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // Object maps containing linked lists of timers, keyed and sorted by their // duration in milliseconds. +// +// The difference between these two objects is that the former contains timers +// that will keep the process open if they are the only thing left, while the +// latter will not. +// // - key = time in milliseconds // - value = linked list const refedLists = {}; const unrefedLists = {}; -// The difference between these two objects is that the former contains timers -// that will keep the process open if they are the only thing left, while the -// latter will not. // Schedule or re-schedule a timer. From 739f9ea4464f934f23064b82862040c7842d2111 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 24 Feb 2016 13:29:22 -0500 Subject: [PATCH 20/29] timers: (fixup) address most of Julien's comments --- lib/timers.js | 56 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index fb5d74be4886ee..f581bfaea1b292 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -19,11 +19,21 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // and efficient. // // Note: It is suggested you first read though the lib/internal/linkedlist.js -// linked list implementation. How it works is somewhat counter-intuitive to -// most JavaScript code, and timers depend on it extensively. +// linked list implementation, since timers depend on it extensively. It can be +// somewhat counter-intuitive at first, as it is not actually a class. Instead, +// it is a set of helpers that operate on an existing object. // -// In order to be as efficient as possible, the implementation does as little as -// possible, and only when required. +// In order to be as performant as possible, the architecture and data +// structures are designed so that they are optimized to handle the following +// use cases as efficiently as possible: + +// - Adding a new timer. (insert) +// - Removing an existing timer. (remove) +// - Handling a timer timing out. (timeout) +// +// Whenever possible, the implementation tries to make the complexity of these +// operations as close to constant-time as possible. +// (So that performance is not impacted by the number of scheduled timers.) // // Object maps are kept which contain linked lists keyed by their duration in // milliseconds. @@ -31,20 +41,46 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // TimerWrap C++ handle, which makes the call after the duration to process the // list it is attached to. // +// +// ┌──── → Object Map +// │ +// ├── +// │ refedLists: { '40': { }, '320': { etc } } (keys of millisecond duration) +// └── ┌─────────────────┘ +// │ +// ┌── ↓ +// │ { _idleNext: { }, _idlePrev: (self), _timer: (TimerWrap handle) } +// │ ┌────────┘ +// │ ┌── ↓ ↑ +// │ │ { _idleNext: { }, _idlePrev: { }, _onTimeout: (callback) } +// │ │ ┌───────────┘ +// │ │ ↓ ↑ +// │ │ { _idleNext: { etc }, _idlePrev: { }, _onTimeout: (callback) } +// ├── ├── +// │ │ +// │ └──── → Actual JavaScript timeouts +// │ +// └──── → Linked List +// +// // With this, virtually constant-time insertion (append), removal, and timeout // is possible in the JavaScript layer. Any one list of timers is able to be // sorted by just appending to it because all timers within share the same // duration. Therefore, any timer added later will always have been scheduled to -// timeout later, thus only needing to be appended. Any less-than constant -// overhead is contained within the TimerWrap's inherently faster libuv binding. -// There is, however, a minor overhead from accessing the specific list from the -// map, or creating a new one, but it is trivial in comparison. -// Removal from an object-property linked list is also constant-time as can be -// seen in the lib/internal/linkedlist.js implementation. +// timeout later, thus only needing to be appended. +// Removal from an object-property linked list is also virtually constant-time +// as can be seen in the lib/internal/linkedlist.js implementation. // Timeouts only need to process any timers due to currently timeout, which will // always be at the beginning of the list for reasons stated above. Any timers // after the first one encountered that does not yet need to timeout will also // always be due to timeout at a later time. +// +// Less-than constant time operations are thus contianed in two places: +// TimerWrap's backing libuv timers implementation (a performant heap-based +// queue), and the object map lookup of a specific list by the duration of +// timers within (or creation of a new list). +// However, these operations combined have shown to be trivial in comparison to +// other alternative timers architectures. // Object maps containing linked lists of timers, keyed and sorted by their From d86edc9652abcc0c07174a1676f7aa4222e30acc Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 24 Feb 2016 13:35:22 -0500 Subject: [PATCH 21/29] timers: (fixup) nit --- lib/timers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/timers.js b/lib/timers.js index f581bfaea1b292..4436c6ab810941 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -130,7 +130,7 @@ function insert(item, unrefed) { // processing for the list. list = { _idleNext: null, // Create the list object with the linkedlist properties - _idlePrev: null, // to prevent any unnecessary hidden class changes. + _idlePrev: null, // to prevent any unnecessary hidden class changes. _timer: new TimerWrap(), _unrefed: unrefed, msecs: msecs From 5d9cc356676dd0ec5341c216410914057fe44407 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 24 Feb 2016 13:49:08 -0500 Subject: [PATCH 22/29] timers: (fixup) remove ascii drawing :( node refuses to compile with it --- lib/timers.js | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 4436c6ab810941..b8472899ee9565 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -41,28 +41,6 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // TimerWrap C++ handle, which makes the call after the duration to process the // list it is attached to. // -// -// ┌──── → Object Map -// │ -// ├── -// │ refedLists: { '40': { }, '320': { etc } } (keys of millisecond duration) -// └── ┌─────────────────┘ -// │ -// ┌── ↓ -// │ { _idleNext: { }, _idlePrev: (self), _timer: (TimerWrap handle) } -// │ ┌────────┘ -// │ ┌── ↓ ↑ -// │ │ { _idleNext: { }, _idlePrev: { }, _onTimeout: (callback) } -// │ │ ┌───────────┘ -// │ │ ↓ ↑ -// │ │ { _idleNext: { etc }, _idlePrev: { }, _onTimeout: (callback) } -// ├── ├── -// │ │ -// │ └──── → Actual JavaScript timeouts -// │ -// └──── → Linked List -// -// // With this, virtually constant-time insertion (append), removal, and timeout // is possible in the JavaScript layer. Any one list of timers is able to be // sorted by just appending to it because all timers within share the same From 4120c5efc907fc94899af42bc3546ecc2c221e4e Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 24 Feb 2016 13:51:30 -0500 Subject: [PATCH 23/29] timers: (fixup) remove _ in tryOnTimeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rename to match Brian’s setImmediate() refactor in https://github.com/nodejs/node/pull/4169 --- lib/timers.js | 4 ++-- test/message/timeout_throw.out | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index b8472899ee9565..e2e5dea4cb8bbd 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -169,7 +169,7 @@ function listOnTimeout() { domain.enter(); } - _tryOnTimeout(timer, list); + tryOnTimeout(timer, list); if (domain) domain.exit(); @@ -191,7 +191,7 @@ function listOnTimeout() { // An optimization so that the try/finally only de-optimizes what is in this // smaller function. -function _tryOnTimeout(timer, list) { +function tryOnTimeout(timer, list) { timer._called = true; var threw = true; try { diff --git a/test/message/timeout_throw.out b/test/message/timeout_throw.out index eed7ec273f1ea9..dcff9b695bb41d 100644 --- a/test/message/timeout_throw.out +++ b/test/message/timeout_throw.out @@ -3,5 +3,5 @@ ^ ReferenceError: undefined_reference_error_maker is not defined at null._onTimeout (*test*message*timeout_throw.js:*:*) - at _tryOnTimeout (timers.js:*:*) + at tryOnTimeout (timers.js:*:*) at Timer.listOnTimeout (timers.js:*:*) From 11a96ad121f5c688c13304b59c2941eee6b4c546 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 25 Feb 2016 10:52:01 -0500 Subject: [PATCH 24/29] timers: (fixup) re-add awesome diagram --- lib/timers.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/timers.js b/lib/timers.js index e2e5dea4cb8bbd..38e4d19e3852ed 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -41,6 +41,28 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // TimerWrap C++ handle, which makes the call after the duration to process the // list it is attached to. // +// +// ╔════ > Object Map +// ║ +// ╠══ +// ║ refedLists: { '40': { }, '320': { etc } } (keys of millisecond duration) +// ╚══ ┌─────────────────┘ +// │ +// ╔══ │ +// ║ { _idleNext: { }, _idlePrev: (self), _timer: (TimerWrap handle) } +// ║ ┌────────┘ +// ║ ╔══ │ ^ +// ║ ║ { _idleNext: { }, _idlePrev: { }, _onTimeout: (callback) } +// ║ ║ ┌───────────┘ +// ║ ║ │ ^ +// ║ ║ { _idleNext: { etc }, _idlePrev: { }, _onTimeout: (callback) } +// ╠══ ╠══ +// ║ ║ +// ║ ╚════ > Actual JavaScript timeouts +// ║ +// ╚════ > Linked List +// +// // With this, virtually constant-time insertion (append), removal, and timeout // is possible in the JavaScript layer. Any one list of timers is able to be // sorted by just appending to it because all timers within share the same From 13b2e0c298b98db99dbdd3f9dc3c4a507a7dbf3a Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 25 Feb 2016 11:08:40 -0500 Subject: [PATCH 25/29] timers: (fixup) update comment preface --- lib/timers.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 38e4d19e3852ed..7f86e7f895cba9 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -13,9 +13,11 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // HOW and WHY the timers implementation works the way it does. // -// Since many parts of Node.js and user applications rely on timeouts, there may -// be a very large amount of timeouts scheduled at any given time. -// Therefore, it is very important that the timers implementation is very fast +// Timers are cruitial to Node.js. Internally, any TCP I/O connection creates a +// timer so that we can time out of connections. Additionally, many user +// user libraries and applications also use timers. As such there may be a +// significantly large amount of timeouts scheduled at any given time. +// Therefore, it is very important that the timers implementation is performant // and efficient. // // Note: It is suggested you first read though the lib/internal/linkedlist.js From 1acab4db772ab4800783465c81191cb141407773 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 25 Feb 2016 11:12:41 -0500 Subject: [PATCH 26/29] timers: (fixup) final? spelling nits --- lib/timers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 7f86e7f895cba9..54b7331a998fb5 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -13,7 +13,7 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // HOW and WHY the timers implementation works the way it does. // -// Timers are cruitial to Node.js. Internally, any TCP I/O connection creates a +// Timers are crucial to Node.js. Internally, any TCP I/O connection creates a // timer so that we can time out of connections. Additionally, many user // user libraries and applications also use timers. As such there may be a // significantly large amount of timeouts scheduled at any given time. @@ -77,7 +77,7 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // after the first one encountered that does not yet need to timeout will also // always be due to timeout at a later time. // -// Less-than constant time operations are thus contianed in two places: +// Less-than constant time operations are thus contained in two places: // TimerWrap's backing libuv timers implementation (a performant heap-based // queue), and the object map lookup of a specific list by the duration of // timers within (or creation of a new list). From dc0961ed11309580f0d6cf677ca0548ace43afc3 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 25 Feb 2016 16:00:48 -0500 Subject: [PATCH 27/29] timers: (fixup) fix linting --- lib/timers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 54b7331a998fb5..0246c4bde4f401 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -149,7 +149,7 @@ function insert(item, unrefed) { L.append(list, item); assert(!L.isEmpty(list)); // list is not empty -}; +} function listOnTimeout() { var list = this._list; @@ -160,7 +160,7 @@ function listOnTimeout() { var now = TimerWrap.now(); debug('now: %s', now); - var diff, timer, threw; + var diff, timer; while (timer = L.peek(list)) { diff = now - timer._idleStart; From a7d3be5cd2db66c3e44b79715337960863b9be6e Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 26 Feb 2016 12:31:34 -0500 Subject: [PATCH 28/29] timers: (fixup) use a constructor function for lists --- lib/timers.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 0246c4bde4f401..93a5286621053d 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -130,13 +130,7 @@ function insert(item, unrefed) { debug('no %d list was found in insert, creating a new one', msecs); // Make a new linked list of timers, and create a TimerWrap to schedule // processing for the list. - list = { - _idleNext: null, // Create the list object with the linkedlist properties - _idlePrev: null, // to prevent any unnecessary hidden class changes. - _timer: new TimerWrap(), - _unrefed: unrefed, - msecs: msecs - }; + list = new TimersList(msecs, unrefed); L.init(list); list._timer._list = list; @@ -151,6 +145,14 @@ function insert(item, unrefed) { assert(!L.isEmpty(list)); // list is not empty } +function TimersList(msecs, unrefed) { + this._idleNext = null; // Create the list with the linkedlist properties to + this._idlePrev = null; // prevent any unnecessary hidden class changes. + this._timer = new TimerWrap(); + this._unrefed = unrefed; + this.msecs = msecs; +} + function listOnTimeout() { var list = this._list; var msecs = list.msecs; From c87e455817a2416179105296fed914efec53ff79 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 26 Feb 2016 12:33:17 -0500 Subject: [PATCH 29/29] timers: (fixup) update ascii graph for TimersList --- lib/timers.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 93a5286621053d..ee7c45aaed36d6 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -48,11 +48,11 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // ║ // ╠══ // ║ refedLists: { '40': { }, '320': { etc } } (keys of millisecond duration) -// ╚══ ┌─────────────────┘ -// │ -// ╔══ │ -// ║ { _idleNext: { }, _idlePrev: (self), _timer: (TimerWrap handle) } -// ║ ┌────────┘ +// ╚══ ┌─────────┘ +// │ +// ╔══ │ +// ║ TimersList { _idleNext: { }, _idlePrev: (self), _timer: (TimerWrap) } +// ║ ┌────────────────┘ // ║ ╔══ │ ^ // ║ ║ { _idleNext: { }, _idlePrev: { }, _onTimeout: (callback) } // ║ ║ ┌───────────┘