From 7227f64ea8d9cc1698eb81fe393ddd32d48ab7c8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Mar 2016 19:32:06 -0400 Subject: [PATCH] Don't assume <10sec build times in watch.isUpToDate. If you have a lot of packages and you change something in a package that is used by lots of other packages, such as "meteor" or "modules", then the rebuild can take a lot longer than ten seconds. --- tools/fs/watch.js | 13 +++++++++---- tools/isobuild/isopack-cache.js | 32 +++++++++++++++++++++++--------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/tools/fs/watch.js b/tools/fs/watch.js index b89fd1701f..499841b952 100644 --- a/tools/fs/watch.js +++ b/tools/fs/watch.js @@ -698,10 +698,10 @@ export class Watcher { } // Given a WatchSet, returns true if it currently describes the state of -// the disk. If mtimeWindowMs is a number, return true if none of the +// the disk. If buildStartTime is a number, return true if none of the // watched files were touched since that many milliseconds ago. -export function isUpToDate(watchSet, mtimeWindowMs) { - if (typeof mtimeWindowMs === "number") { +export function isUpToDate(watchSet, buildStartTime) { + if (typeof buildStartTime === "number") { const now = +new Date; let recentlyChanged = false; @@ -719,7 +719,12 @@ export function isUpToDate(watchSet, mtimeWindowMs) { break; } - if (stat && (now - stat.mtime < mtimeWindowMs)) { + // Add a ten-second buffer to account for changes that happened just + // before the build started. This is perfectly safe because using a + // more distant time in the past only increases the chances that we + // will fall back to the full isUpToDate check. + const tenSecondsBeforeBuildStartTime = buildStartTime - 10 * 1000; + if (stat && stat.mtime > tenSecondsBeforeBuildStartTime) { recentlyChanged = true; break; } diff --git a/tools/isobuild/isopack-cache.js b/tools/isobuild/isopack-cache.js index 832f2ddd6a..05d551046f 100644 --- a/tools/isobuild/isopack-cache.js +++ b/tools/isobuild/isopack-cache.js @@ -61,6 +61,8 @@ _.extend(exports.IsopackCache.prototype, { var self = this; buildmessage.assertInCapture(); + const buildStartTime = Date.now(); + if (self.cacheDir) { files.mkdir_p(self.cacheDir); } @@ -68,11 +70,11 @@ _.extend(exports.IsopackCache.prototype, { var onStack = {}; if (rootPackageNames) { _.each(rootPackageNames, function (name) { - self._ensurePackageLoaded(name, onStack); + self._ensurePackageLoaded(name, {onStack, buildStartTime}); }); } else { self._packageMap.eachPackage(function (name, packageInfo) { - self._ensurePackageLoaded(name, onStack); + self._ensurePackageLoaded(name, {onStack, buildStartTime}); }); } }, @@ -140,7 +142,10 @@ _.extend(exports.IsopackCache.prototype, { return null; }, - _ensurePackageLoaded: function (name, onStack) { + _ensurePackageLoaded: function (name, { + onStack, + buildStartTime, + }) { var self = this; buildmessage.assertInCapture(); if (_.has(self._isopacks, name)) { @@ -155,7 +160,7 @@ _.extend(exports.IsopackCache.prototype, { return; } onStack[depName] = true; - self._ensurePackageLoaded(depName, onStack); + self._ensurePackageLoaded(depName, {onStack, buildStartTime}); delete onStack[depName]; }; @@ -191,7 +196,11 @@ _.extend(exports.IsopackCache.prototype, { return; } Profile.time('IsopackCache Build local isopack', () => { - self._loadLocalPackage(name, packageInfo, previousIsopack); + self._loadLocalPackage(name, { + packageInfo, + previousIsopack, + buildStartTime, + }); }); }); } else if (packageInfo.kind === 'versioned') { @@ -254,12 +263,17 @@ _.extend(exports.IsopackCache.prototype, { } }, - _loadLocalPackage: function (name, packageInfo, previousIsopack) { + _loadLocalPackage: function (name, { + packageInfo, + previousIsopack, + buildStartTime, + }) { var self = this; buildmessage.assertInCapture(); buildmessage.enterJob("building package " + name, function () { var isopack; - if (previousIsopack && self._checkUpToDatePreloaded(previousIsopack)) { + if (previousIsopack && + self._checkUpToDatePreloaded(previousIsopack, buildStartTime)) { isopack = previousIsopack; // We don't need to call self._lintLocalPackage here, because // lintingMessages is saved on the isopack. @@ -389,7 +403,7 @@ _.extend(exports.IsopackCache.prototype, { return watch.isUpToDate(watchSet); }, - _checkUpToDatePreloaded: function (previousIsopack) { + _checkUpToDatePreloaded: function (previousIsopack, buildStartTime) { var self = this; // If we include Cordova but this Isopack doesn't, or via versa, then we're @@ -414,7 +428,7 @@ _.extend(exports.IsopackCache.prototype, { // Since we've checked this isopack previously, take a shortcut by not // considering it out of date unless any of its files have mtimes // within the last 30 seconds. - return watch.isUpToDate(watchSet, 30 * 1000); + return watch.isUpToDate(watchSet, buildStartTime); }, _isopackDir: function (packageName) {