diff --git a/lib/_linklist.js b/lib/_linklist.js deleted file mode 100644 index 02186cfedcb9f6..00000000000000 --- a/lib/_linklist.js +++ /dev/null @@ -1,57 +0,0 @@ -'use strict'; - -function init(list) { - list._idleNext = list; - list._idlePrev = list; -} -exports.init = init; - - -// show the most idle item -function peek(list) { - if (list._idlePrev == list) return null; - return list._idlePrev; -} -exports.peek = peek; - - -// remove the most idle item from the list -function shift(list) { - var first = list._idlePrev; - remove(first); - return first; -} -exports.shift = shift; - - -// remove a item from its list -function remove(item) { - if (item._idleNext) { - item._idleNext._idlePrev = item._idlePrev; - } - - if (item._idlePrev) { - item._idlePrev._idleNext = item._idleNext; - } - - item._idleNext = null; - item._idlePrev = null; -} -exports.remove = remove; - - -// remove a item from its list and place at the end. -function append(list, item) { - remove(item); - item._idleNext = list._idleNext; - list._idleNext._idlePrev = item; - item._idlePrev = list; - list._idleNext = item; -} -exports.append = append; - - -function isEmpty(list) { - return list._idleNext === list; -} -exports.isEmpty = isEmpty; diff --git a/lib/timers.js b/lib/timers.js index aa9f316253e692..ec5b4d1f5c243a 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -1,10 +1,6 @@ 'use strict'; const Timer = process.binding('timer_wrap').Timer; -const L = require('_linklist'); -const assert = require('assert').ok; -const util = require('util'); -const debug = util.debuglog('timer'); const kOnTimeout = Timer.kOnTimeout | 0; // Timeout values > TIMEOUT_MAX are set to 1. @@ -15,62 +11,94 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // 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: +// and a set. This technique is described in the libev manual but using linked +// lists instead of sets: // 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 = {}; +// Array containing all timers ordered by when they are set to fire +var timers = []; -// the main function - creates lists on demand and the watchers associated -// with them. -function insert(item, msecs) { - item._idleStart = Timer.now(); - item._idleTimeout = msecs; +var triggerTimers = []; - if (msecs < 0) return; - - var list; +function addToTimers(item) { + let maxIndex = timers.length - 1; + // hot path + if (maxIndex === -1 || item._triggerAt >= timers[maxIndex]._triggerAt) { + timers[maxIndex + 1] = item; + return maxIndex + 1; + } - if (lists[msecs]) { - list = lists[msecs]; - } else { - list = new Timer(); - list.start(msecs, 0); + let minIndex = 0; + let currentIndex; + let currentElement; - L.init(list); + while (minIndex <= maxIndex) { + currentIndex = (minIndex + maxIndex) / 2 | 0; + currentElement = timers[currentIndex]; - lists[msecs] = list; - list.msecs = msecs; - list[kOnTimeout] = listOnTimeout; + if (currentElement._triggerAt < item._triggerAt) { + minIndex = currentIndex + 1; + } else if (currentElement._triggerAt > item._triggerAt) { + maxIndex = currentIndex - 1; + } + else { + break; + } } + timers.splice(currentIndex, 0, item); - L.append(list, item); - assert(!L.isEmpty(list)); // list is not empty + return currentIndex; +} + +function setTriggerTimer() { + let triggerTimer = new Timer(); + let msecs = timers[0]._triggerAt - Timer.now(); + if (msecs < 1) msecs = 1; + triggerTimer.start(msecs, 0); + triggerTimer.msecs = timers[0]._idleTimeout; + triggerTimer[kOnTimeout] = listOnTimeout; + triggerTimers.push(triggerTimer); +} + +// the main function - inserts timers into the right place in the timers array +// and creates watchers associated with them when necessary +function insert(item, msecs) { + if (msecs < 0) return; + + item._idleStart = Timer.now(); + item._idleTimeout = msecs; + item._triggerAt = item._idleStart + msecs; + + const indexInserted = addToTimers(item); + if (indexInserted === 0) + setTriggerTimer(); +} + +function removeFiredTimers() { + timers = timers.filter(function(val) {return ! val._called;}); } function listOnTimeout() { - var msecs = this.msecs; var list = this; - debug('timeout callback %d', msecs); - var now = Timer.now(); - debug('now: %s', now); var diff, first, threw; - while (first = L.peek(list)) { - diff = now - first._idleStart; - if (diff < msecs) { - list.start(msecs - diff, 0); - debug('%d list wait because diff is %d', msecs, diff); + let i = 0; + let timersCopy = timers.slice(0); + while (i < timersCopy.length) { + first = timersCopy[i]; + diff = first._triggerAt - now; + if (diff > 0) { + removeFiredTimers(); + list.start(diff, 0); return; } else { - L.remove(first); - assert(first !== L.peek(list)); - - if (!first._onTimeout) continue; + i++; + if (!first._onTimeout) { + first._called = true; + continue; + } // v0.4 compatibility: if the timer callback throws and the // domain or uncaughtException handler ignore the exception, @@ -78,23 +106,29 @@ function listOnTimeout() { // // https://github.com/joyent/node/issues/2631 var domain = first.domain; - if (domain && domain._disposed) + if (domain && domain._disposed) { + first._called = true; 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. + removeFiredTimers(); var oldDomain = process.domain; process.domain = null; process.nextTick(listOnTimeoutNT, list); @@ -104,10 +138,11 @@ function listOnTimeout() { } } - debug('%d list empty', msecs); - assert(L.isEmpty(list)); list.close(); - delete lists[msecs]; + removeFiredTimers(); + if (timers.length !== 0) { + setTriggerTimer(); + } } @@ -117,15 +152,18 @@ function listOnTimeoutNT(list) { const unenroll = exports.unenroll = function(item) { - L.remove(item); - var list = lists[item._idleTimeout]; + const myIndex = timers.indexOf(item); + if (myIndex !== -1) { + timers.splice(myIndex, 1); + } + // if empty then stop the watcher - debug('unenroll'); - if (list && L.isEmpty(list)) { - debug('unenroll: list empty'); - list.close(); - delete lists[item._idleTimeout]; + if (timers.length === 0 && triggerTimers.length !== 0) { + for (let i = 0; i < triggerTimers.length; i++) { + triggerTimers[i].close(); + } + triggerTimers = []; } // if active is called later, then we want to make sure not to insert again item._idleTimeout = -1; @@ -144,7 +182,7 @@ exports.enroll = function(item, msecs) { // if this item was already in a list somewhere // then we should unenroll it from that - if (item._idleNext) unenroll(item); + unenroll(item); // Ensure that msecs fits into signed int32 if (msecs > TIMEOUT_MAX) { @@ -152,7 +190,7 @@ exports.enroll = function(item, msecs) { } item._idleTimeout = msecs; - L.init(item); + addToTimers(item); }; @@ -278,6 +316,8 @@ exports.setInterval = function(callback, repeat) { this._handle.start(repeat, 0); } else { timer._idleTimeout = repeat; + removeFiredTimers(); + timer._called = false; exports.active(timer); } } @@ -295,8 +335,6 @@ exports.clearInterval = function(timer) { const Timeout = function(after) { this._called = false; this._idleTimeout = after; - this._idlePrev = this; - this._idleNext = this; this._idleStart = null; this._onTimeout = null; this._repeat = null; @@ -351,19 +389,17 @@ Timeout.prototype.close = function() { }; -var immediateQueue = {}; -L.init(immediateQueue); +var immediateQueue = []; function processImmediate() { var queue = immediateQueue; var domain, immediate; - immediateQueue = {}; - L.init(immediateQueue); + immediateQueue = []; - while (L.isEmpty(queue) === false) { - immediate = L.shift(queue); + for (let i = 0; i < queue.length; i++) { + immediate = queue[i]; domain = immediate.domain; if (domain) @@ -371,19 +407,16 @@ function processImmediate() { var threw = true; try { - immediate._onImmediate(); + if (immediate._onImmediate) { + immediate._onImmediate(); + } threw = false; } finally { if (threw) { - if (!L.isEmpty(queue)) { - // Handle any remaining on next tick, assuming we're still - // alive to do so. - while (!L.isEmpty(immediateQueue)) { - L.append(queue, L.shift(immediateQueue)); - } - immediateQueue = queue; - process.nextTick(processImmediate); - } + // Handle any remaining on next tick, assuming we're still + // alive to do so. + immediateQueue = queue.slice(i + 1).concat(immediateQueue); + process.nextTick(processImmediate); } } @@ -394,7 +427,7 @@ function processImmediate() { // Only round-trip to C++ land if we have to. Calling clearImmediate() on an // immediate that's in |queue| is okay. Worst case is we make a superfluous // call to NeedImmediateCallbackSetter(). - if (L.isEmpty(immediateQueue)) { + if (immediateQueue.length === 0) { process._needImmediateCallback = false; } } @@ -404,8 +437,6 @@ function Immediate() { } Immediate.prototype.domain = undefined; Immediate.prototype._onImmediate = undefined; -Immediate.prototype._idleNext = undefined; -Immediate.prototype._idlePrev = undefined; exports.setImmediate = function(callback, arg1, arg2, arg3) { @@ -413,8 +444,6 @@ exports.setImmediate = function(callback, arg1, arg2, arg3) { var len = arguments.length; var immediate = new Immediate(); - L.init(immediate); - switch (len) { // fast cases case 0: @@ -456,7 +485,7 @@ exports.setImmediate = function(callback, arg1, arg2, arg3) { if (process.domain) immediate.domain = process.domain; - L.append(immediateQueue, immediate); + immediateQueue.push(immediate); return immediate; }; @@ -467,9 +496,12 @@ exports.clearImmediate = function(immediate) { immediate._onImmediate = undefined; - L.remove(immediate); + const myIndex = immediateQueue.indexOf(immediate); + if (myIndex !== -1) { + immediateQueue.splice(myIndex, 1); + } - if (L.isEmpty(immediateQueue)) { + if (immediateQueue.length === 0) { process._needImmediateCallback = false; } }; @@ -478,13 +510,23 @@ exports.clearImmediate = function(immediate) { // 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; +var unrefArray = []; +var unrefTimer; function _makeTimerTimeout(timer) { var domain = timer.domain; var msecs = timer._idleTimeout; + let myIndex; - L.remove(timer); + myIndex = timers.indexOf(timer); + if (myIndex !== -1) { + timers.splice(myIndex, 1); + } + + myIndex = unrefArray.indexOf(timer); + if (myIndex !== -1) { + unrefArray.splice(myIndex, 1); + } // Timer has been unenrolled by another timer that fired at the same time, // so don't make it timeout. @@ -501,7 +543,6 @@ function _makeTimerTimeout(timer) { domain.enter(); } - debug('unreftimer firing timeout'); timer._called = true; _runOnTimeout(timer); @@ -522,8 +563,6 @@ function _runOnTimeout(timer) { function unrefTimeout() { var now = Timer.now(); - debug('unrefTimer fired'); - var timeSinceLastActive; var nextTimeoutTime; var nextTimeoutDuration; @@ -541,8 +580,8 @@ function unrefTimeout() { // 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) { + for (let i = 0; i < unrefArray.length; i++) { + let cur = unrefArray[i]; timeSinceLastActive = now - cur._idleStart; if (timeSinceLastActive < cur._idleTimeout) { @@ -566,8 +605,6 @@ function unrefTimeout() { // of current timers has been fully traversed. timersToTimeout.push(cur); } - - cur = cur._idlePrev; } var nbTimersToTimeout = timersToTimeout.length; @@ -580,9 +617,6 @@ function unrefTimeout() { if (minNextTimeoutTime !== TIMEOUT_MAX) { unrefTimer.start(minNextTimeoutTime - now, 0); unrefTimer.when = minNextTimeoutTime; - debug('unrefTimer rescheduled'); - } else if (L.isEmpty(unrefList)) { - debug('unrefList is empty'); } } @@ -590,22 +624,17 @@ function unrefTimeout() { 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; + const myIndex = timers.indexOf(item); + if (myIndex !== -1) { + timers.splice(myIndex, 1); } + unrefTimer = new Timer(); + unrefTimer.unref(); + unrefTimer.when = -1; + unrefTimer[kOnTimeout] = unrefTimeout; + var now = Timer.now(); item._idleStart = now; @@ -616,9 +645,7 @@ exports._unrefActive = function(item) { 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); + unrefArray.push(item); }; diff --git a/node.gyp b/node.gyp index 0e2fd3ae443449..61632cccab8357 100644 --- a/node.gyp +++ b/node.gyp @@ -17,7 +17,6 @@ 'src/node.js', 'lib/_debug_agent.js', 'lib/_debugger.js', - 'lib/_linklist.js', 'lib/assert.js', 'lib/buffer.js', 'lib/child_process.js', diff --git a/test/parallel/test-timers-linked-list.js b/test/parallel/test-timers-linked-list.js deleted file mode 100644 index 00b2129d126c4a..00000000000000 --- a/test/parallel/test-timers-linked-list.js +++ /dev/null @@ -1,100 +0,0 @@ -'use strict'; -var common = require('../common'); -var assert = require('assert'); -var L = require('_linklist'); - - -var list = { name: 'list' }; -var A = { name: 'A' }; -var B = { name: 'B' }; -var C = { name: 'C' }; -var D = { name: 'D' }; - - -L.init(list); -L.init(A); -L.init(B); -L.init(C); -L.init(D); - -assert.ok(L.isEmpty(list)); -assert.equal(null, L.peek(list)); - -L.append(list, A); -// list -> A -assert.equal(A, L.peek(list)); - -L.append(list, B); -// list -> A -> B -assert.equal(A, L.peek(list)); - -L.append(list, C); -// list -> A -> B -> C -assert.equal(A, L.peek(list)); - -L.append(list, D); -// list -> A -> B -> C -> D -assert.equal(A, L.peek(list)); - -var x = L.shift(list); -assert.equal(A, x); -// list -> B -> C -> D -assert.equal(B, L.peek(list)); - -x = L.shift(list); -assert.equal(B, x); -// list -> C -> D -assert.equal(C, L.peek(list)); - -// B is already removed, so removing it again shouldn't hurt. -L.remove(B); -// list -> C -> D -assert.equal(C, L.peek(list)); - -// Put B back on the list -L.append(list, B); -// list -> C -> D -> B -assert.equal(C, L.peek(list)); - -L.remove(C); -// list -> D -> B -assert.equal(D, L.peek(list)); - -L.remove(B); -// list -> D -assert.equal(D, L.peek(list)); - -L.remove(D); -// list -assert.equal(null, L.peek(list)); - - -assert.ok(L.isEmpty(list)); - - -L.append(list, D); -// list -> D -assert.equal(D, L.peek(list)); - -L.append(list, C); -L.append(list, B); -L.append(list, A); -// list -> D -> C -> B -> A - -// Append should REMOVE C from the list and append it to the end. -L.append(list, C); - -// list -> D -> B -> A -> C -assert.equal(D, L.shift(list)); -// list -> B -> A -> C -assert.equal(B, L.peek(list)); -assert.equal(B, L.shift(list)); -// list -> A -> C -assert.equal(A, L.peek(list)); -assert.equal(A, L.shift(list)); -// list -> C -assert.equal(C, L.peek(list)); -assert.equal(C, L.shift(list)); -// list -assert.ok(L.isEmpty(list)); - diff --git a/test/parallel/test-timers-unref-active-unenrolled-disposed.js b/test/parallel/test-timers-unref-active-unenrolled-disposed.js index ac22cd6e6a7144..42895b1567e794 100644 --- a/test/parallel/test-timers-unref-active-unenrolled-disposed.js +++ b/test/parallel/test-timers-unref-active-unenrolled-disposed.js @@ -24,8 +24,6 @@ someTimer._onTimeout = function() { }; endTest._onTimeout = common.mustCall(function() { - assert.strictEqual(someTimer._idlePrev, null); - assert.strictEqual(someTimer._idleNext, null); clearTimeout(keepOpen); }); diff --git a/test/parallel/test-timers-unref.js b/test/parallel/test-timers-unref.js index c0b24a4275be86..acb5050221d7a9 100644 --- a/test/parallel/test-timers-unref.js +++ b/test/parallel/test-timers-unref.js @@ -71,5 +71,5 @@ process.on('exit', function() { assert.strictEqual(unref_interval, true, 'An unrefd interval should still fire'); assert.strictEqual(unref_callbacks, 1, - 'Callback should only run once'); + 'Callback should run exactly once'); });