Skip to content

Commit

Permalink
timers: add internal [@@ refresh()] function
Browse files Browse the repository at this point in the history
Hidden via a symbol because I'm unsure exactly what the API should look
like in the end.

Removes the need to use _unrefActive for efficiently refreshing
timeouts.
It still uses it under the hood but that could be replaced with
insert() directly if it were in the same file.

PR-URL: #18065
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
  • Loading branch information
Fishrock123 committed Jan 26, 2018
1 parent 54fe0a6 commit bb5575a
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 13 deletions.
9 changes: 4 additions & 5 deletions lib/internal/http2/core.js
Expand Up @@ -57,11 +57,10 @@ const {
const {
kTimeout,
setUnrefTimeout,
validateTimerDuration
validateTimerDuration,
refreshFnSymbol
} = require('internal/timers');

const { _unrefActive } = require('timers');

const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
const { constants } = binding;

Expand Down Expand Up @@ -912,7 +911,7 @@ class Http2Session extends EventEmitter {
[kUpdateTimer]() {
if (this.destroyed)
return;
if (this[kTimeout]) _unrefActive(this[kTimeout]);
if (this[kTimeout]) this[kTimeout][refreshFnSymbol]();
}

// Sets the id of the next stream to be created by this Http2Session.
Expand Down Expand Up @@ -1478,7 +1477,7 @@ class Http2Stream extends Duplex {
if (this.destroyed)
return;
if (this[kTimeout])
_unrefActive(this[kTimeout]);
this[kTimeout][refreshFnSymbol]();
if (this[kSession])
this[kSession][kUpdateTimer]();
}
Expand Down
23 changes: 21 additions & 2 deletions lib/internal/timers.js
Expand Up @@ -19,12 +19,16 @@ const errors = require('internal/errors');
// Timeout values > TIMEOUT_MAX are set to 1.
const TIMEOUT_MAX = 2 ** 31 - 1;

const refreshFnSymbol = Symbol('refresh()');
const unrefedSymbol = Symbol('unrefed');

module.exports = {
TIMEOUT_MAX,
kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals.
async_id_symbol,
trigger_async_id_symbol,
Timeout,
refreshFnSymbol,
setUnrefTimeout,
validateTimerDuration
};
Expand All @@ -39,7 +43,7 @@ function getTimers() {

// Timer constructor function.
// The entire prototype is defined in lib/timers.js
function Timeout(callback, after, args, isRepeat) {
function Timeout(callback, after, args, isRepeat, isUnrefed) {
after *= 1; // coalesce to number or NaN
if (!(after >= 1 && after <= TIMEOUT_MAX)) {
if (after > TIMEOUT_MAX) {
Expand All @@ -64,6 +68,8 @@ function Timeout(callback, after, args, isRepeat) {
this._repeat = isRepeat ? after : null;
this._destroyed = false;

this[unrefedSymbol] = isUnrefed;

this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
this[trigger_async_id_symbol] = getDefaultTriggerAsyncId();
if (async_hook_fields[kInit] > 0) {
Expand All @@ -74,6 +80,19 @@ function Timeout(callback, after, args, isRepeat) {
}
}

Timeout.prototype[refreshFnSymbol] = function refresh() {
if (this._handle) {
// Would be more ideal with uv_timer_again(), however that API does not
// cause libuv's sorted timers data structure (a binary heap at the time
// of writing) to re-sort itself. This causes ordering inconsistencies.
this._handle.stop();
this._handle.start(this._idleTimeout);
} else if (this[unrefedSymbol]) {
getTimers()._unrefActive(this);
} else {
getTimers().active(this);
}
};

function setUnrefTimeout(callback, after, arg1, arg2, arg3) {
// Type checking identical to setTimeout()
Expand Down Expand Up @@ -102,7 +121,7 @@ function setUnrefTimeout(callback, after, arg1, arg2, arg3) {
break;
}

const timer = new Timeout(callback, after, args, false);
const timer = new Timeout(callback, after, args, false, true);
getTimers()._unrefActive(timer);

return timer;
Expand Down
6 changes: 3 additions & 3 deletions lib/net.js
Expand Up @@ -23,7 +23,6 @@

const EventEmitter = require('events');
const stream = require('stream');
const timers = require('timers');
const util = require('util');
const internalUtil = require('internal/util');
const {
Expand Down Expand Up @@ -64,7 +63,8 @@ const exceptionWithHostPort = util._exceptionWithHostPort;
const {
kTimeout,
setUnrefTimeout,
validateTimerDuration
validateTimerDuration,
refreshFnSymbol
} = require('internal/timers');

function noop() {}
Expand Down Expand Up @@ -291,7 +291,7 @@ util.inherits(Socket, stream.Duplex);
Socket.prototype._unrefTimer = function _unrefTimer() {
for (var s = this; s !== null; s = s._parent) {
if (s[kTimeout])
timers._unrefActive(s[kTimeout]);
s[kTimeout][refreshFnSymbol]();
}
};

Expand Down
6 changes: 3 additions & 3 deletions lib/timers.js
Expand Up @@ -432,15 +432,15 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
break;
}

const timeout = new Timeout(callback, after, args, false);
const timeout = new Timeout(callback, after, args, false, false);
active(timeout);

return timeout;
}

setTimeout[internalUtil.promisify.custom] = function(after, value) {
const promise = createPromise();
const timeout = new Timeout(promise, after, [value], false);
const timeout = new Timeout(promise, after, [value], false, false);
active(timeout);

return promise;
Expand Down Expand Up @@ -523,7 +523,7 @@ exports.setInterval = function(callback, repeat, arg1, arg2, arg3) {
break;
}

const timeout = new Timeout(callback, repeat, args, true);
const timeout = new Timeout(callback, repeat, args, true, false);
active(timeout);

return timeout;
Expand Down
59 changes: 59 additions & 0 deletions test/parallel/test-timers-refresh.js
@@ -0,0 +1,59 @@
// Flags: --expose-internals

'use strict';

const common = require('../common');

const { strictEqual } = require('assert');
const { setUnrefTimeout, refreshFnSymbol } = require('internal/timers');

// Schedule the unrefed cases first so that the later case keeps the event loop
// active.

// Every case in this test relies on implicit sorting within either Node's or
// libuv's timers storage data structures.

// unref()'d timer
{
let called = false;
const timer = setTimeout(common.mustCall(() => {
called = true;
}), 1);
timer.unref();

// This relies on implicit timers handle sorting withing libuv.

setTimeout(common.mustCall(() => {
strictEqual(called, false, 'unref()\'d timer returned before check');
}), 1);

timer[refreshFnSymbol]();
}

// unref pooled timer
{
let called = false;
const timer = setUnrefTimeout(common.mustCall(() => {
called = true;
}), 1);

setUnrefTimeout(common.mustCall(() => {
strictEqual(called, false, 'unref pooled timer returned before check');
}), 1);

timer[refreshFnSymbol]();
}

// regular timer
{
let called = false;
const timer = setTimeout(common.mustCall(() => {
called = true;
}), 1);

setTimeout(common.mustCall(() => {
strictEqual(called, false, 'pooled timer returned before check');
}), 1);

timer[refreshFnSymbol]();
}

0 comments on commit bb5575a

Please sign in to comment.