From 60f8c1acf4462ff546fbdda3e071694767b86f1c Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 26 Feb 2016 14:18:52 -0500 Subject: [PATCH] timers: refactor timers Consolidates the implementation of regular and internal (_unrefActive) timers. Also includes a couple optimizations: - Isolates the try/catch from listOnTimeout() in a new tryOnTimeout(). - Uses a TimersList constructor as the base for linkedlists. Additionally includes other cleanup and clarification, such as a rename of "Timer" to "TimerWrap". PR-URL: https://github.com/nodejs/node/pull/4007 Reviewed-By: Rod Vagg Reviewed-By: Trevor Norris Reviewed-By: Julien Gilli Reviewed-By: Chris Dickinson --- lib/timers.js | 323 +++++++++++---------------------- test/message/timeout_throw.out | 1 + 2 files changed, 106 insertions(+), 218 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 36fbff43dbc183..fc22bd8d0809dd 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -1,16 +1,18 @@ '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').ok; +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 // IDLE TIMEOUTS +// Object maps containing linked lists of timers, keyed and sorted by their +// duration in milliseconds. // // 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 @@ -18,117 +20,151 @@ 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 = {}; +const refedLists = {}; +const unrefedLists = {}; -// 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. +// 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(); + item._idleStart = TimerWrap.now(); - var list; - - if (lists[msecs]) { - list = lists[msecs]; - } else { - list = new Timer(); - list.start(msecs, 0); + const lists = unrefed === true ? unrefedLists : refedLists; + var list = lists[msecs]; + if (!list) { + debug('no %d list was found in insert, creating a new one', msecs); + list = new TimersList(msecs, unrefed); 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); 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 msecs = this.msecs; - var list = this; + var list = this._list; + var msecs = list.msecs; debug('timeout callback %d', msecs); - var now = Timer.now(); + var now = TimerWrap.now(); debug('now: %s', now); - var diff, first, threw; - while (first = L.peek(list)) { - diff = now - first._idleStart; + var diff, timer; + while (timer = L.peek(list)) { + diff = now - timer._idleStart; + if (diff < msecs) { - list.start(msecs - diff, 0); + this.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; + L.remove(timer); + assert(timer !== L.peek(list)); - // v0.4 compatibility: if the timer callback throws and the + if (!timer._onTimeout) continue; + + var domain = timer.domain; + if (domain) { + + // 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) + // https://github.com/nodejs/node-v0.x-archive/issues/2631 + if (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; - } - } + domain.enter(); } + + tryOnTimeout(timer, list); + + if (domain) + domain.exit(); } debug('%d list empty', msecs); assert(L.isEmpty(list)); - list.close(); - delete lists[msecs]; + this.close(); + if (list._unrefed === true) { + delete unrefedLists[msecs]; + } else { + delete refedLists[msecs]; + } +} + + +// An optimization so that the try/finally only de-optimizes what is in this +// smaller function. +function tryOnTimeout(timer, list) { + timer._called = true; + var threw = true; + try { + timer._onTimeout(); + threw = false; + } finally { + 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; + process.nextTick(listOnTimeoutNT, list); + process.domain = domain; + } } function listOnTimeoutNT(list) { - list[kOnTimeout](); + list._timer[kOnTimeout](); } 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]; - return list; + list._timer.stop(); + delete refedLists[item._idleTimeout]; + return list._timer; } return null; @@ -136,10 +172,10 @@ function reuse(item) { 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; @@ -328,7 +364,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; @@ -341,7 +377,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); @@ -495,152 +531,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..dcff9b695bb41d 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 tryOnTimeout (timers.js:*:*) at Timer.listOnTimeout (timers.js:*:*)