From 3cafbc72ac83dfbbae007181de6a1164ae17c7ce Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 1 Jul 2015 12:05:48 -0400 Subject: [PATCH] Use async functions to eliminate the need for fiberHelpers.inBareFiber. The comments for inBareFiber claim that it's for times when you don't want to inherit the dynamic environment variables of the current Fiber, but none of the call sites actually relied on this behavior. Still, it may be worth noting that async functions automatically (and cheaply) inherit the calling Fiber's dynamics, which is virtually always what you want. --- tools/runners/run-app.js | 22 +++++++++++++--------- tools/runners/run-updater.js | 10 +++++----- tools/utils/fiber-helpers.js | 14 -------------- tools/utils/func-utils.js | 16 ++++++++-------- 4 files changed, 26 insertions(+), 36 deletions(-) diff --git a/tools/runners/run-app.js b/tools/runners/run-app.js index ffc0144a96..8d8a8605c2 100644 --- a/tools/runners/run-app.js +++ b/tools/runners/run-app.js @@ -85,8 +85,12 @@ _.extend(AppProcess.prototype, { self.proc = self._spawn(); // Send stdout and stderr to the runLog - var eachline = require('eachline'); - eachline(self.proc.stdout, 'utf8', fiberHelpers.inBareFiber(function (line) { + var realEachline = require('eachline'); + function eachline(stream, encoding, callback) { + realEachline(stream, encoding, (...args) => void(callback(...args))); + } + + eachline(self.proc.stdout, 'utf8', async function (line) { if (line.match(/^LISTENING\s*$/)) { // This is the child process telling us that it's ready to receive // connections. (It does this because we told it to with @@ -96,9 +100,9 @@ _.extend(AppProcess.prototype, { } else { runLog.logAppOutput(line); } - })); + }); - eachline(self.proc.stderr, 'utf8', fiberHelpers.inBareFiber(function (line) { + eachline(self.proc.stderr, 'utf8', async function (line) { if (self.debugPort && line.indexOf("debugger listening on port ") >= 0) { Console.enableProgressDisplay(false); @@ -106,15 +110,15 @@ _.extend(AppProcess.prototype, { } runLog.logAppOutput(line, true); - })); + }); // Watch for exit and for stdio to be fully closed (so that we don't miss // log lines). - self.proc.on('close', fiberHelpers.inBareFiber(function (code, signal) { + self.proc.on('close', async function (code, signal) { self._maybeCallOnExit(code, signal); - })); + }); - self.proc.on('error', fiberHelpers.inBareFiber(function (err) { + self.proc.on('error', async function (err) { // if the error is the result of .send command over ipc pipe, ignore it if (self._refreshing) { return; @@ -126,7 +130,7 @@ _.extend(AppProcess.prototype, { // 'close' callback, so we use a guard to make sure we only call // onExit once. self._maybeCallOnExit(); - })); + }); // This happens sometimes when we write a keepalive after the app // is dead. If we don't register a handler, we get a top level diff --git a/tools/runners/run-updater.js b/tools/runners/run-updater.js index c2864e4d76..f01c16e137 100644 --- a/tools/runners/run-updater.js +++ b/tools/runners/run-updater.js @@ -21,15 +21,15 @@ _.extend(Updater.prototype, { // Check every 3 hours. (Should not share buildmessage state with // the main fiber.) - self.timer = setInterval(fiberHelpers.inBareFiber(function () { + async function check() { self._check(); - }), 3 * 60 * 60 * 1000); + } + + self.timer = setInterval(check, 3 * 60 * 60 * 1000); // Also start a check now, but don't block on it. (This should // not share buildmessage state with the main fiber.) - new Fiber(function () { - self._check(); - }).run(); + check(); }, _check: function () { diff --git a/tools/utils/fiber-helpers.js b/tools/utils/fiber-helpers.js index 4b1ad7e5a2..a2d743e240 100644 --- a/tools/utils/fiber-helpers.js +++ b/tools/utils/fiber-helpers.js @@ -126,20 +126,6 @@ exports.bindEnvironment = function (func) { }; }; -// An alternative to bindEnvironment for the rare case where you -// want the callback you're passing to some Node function to start -// a new fiber but *NOT* to inherit the current environment. -// Eg, if you are trying to do the equivalent of start a background -// thread. -exports.inBareFiber = function (func) { - return function (...args) { - var self = this; - new Fiber(function () { - func.apply(self, args); - }).run(); - }; -}; - // Returns a Promise that supports .resolve(result) and .reject(error). exports.makeFulfillablePromise = function () { var resolve, reject; diff --git a/tools/utils/func-utils.js b/tools/utils/func-utils.js index c5ddb2e86b..4509f06c8d 100644 --- a/tools/utils/func-utils.js +++ b/tools/utils/func-utils.js @@ -2,7 +2,7 @@ // milliseconds of each other, and prevents overlapping invocations of fn // by postponing the next invocation until after fn's fiber finishes. exports.coalesce = function(delayMs, callback, context) { - var pendingTimer = null; + var pending = false; var inProgress = 0; delayMs = delayMs || 100; @@ -17,32 +17,32 @@ exports.coalesce = function(delayMs, callback, context) { return; } - if (pendingTimer !== null) { + if (pending) { // Defer to the already-pending timer. return; } - var fiberCallback = require('./fiber-helpers.js').inBareFiber(function() { + new Promise( + resolve => setTimeout(resolve, delayMs) + ).then(function thenCallback() { // Now that the timeout has fired, set inProgress to 1 so that // (until the callback is complete and we set inProgress to 0 again) // any calls to coalescingWrapper will increment inProgress to // indicate that at least one other caller wants fiberCallback to be // called again when the original callback is complete. - pendingTimer = null; + pending = false; inProgress = 1; try { callback.call(self); } finally { if (inProgress > 1) { - process.nextTick(fiberCallback); - pendingTimer = true; + Promise.resolve().then(thenCallback); + pending = true; } inProgress = 0; } }); - - pendingTimer = setTimeout(fiberCallback, delayMs); } return wrap(coalescingWrapper, callback);