From 312a344b8d01e0c54d5eab69205bbf5baef5eb1f Mon Sep 17 00:00:00 2001 From: Chris Watts Date: Fri, 4 Dec 2015 01:44:37 +1100 Subject: [PATCH 1/2] Skips loading of the dependency cache if nothing has changed. Excludes the cache from equality check as it shouldn't have changed. --- packages/constraint-solver/constraint-solver-input.js | 5 ++++- packages/constraint-solver/constraint-solver.js | 11 ++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-input.js b/packages/constraint-solver/constraint-solver-input.js index 7d8cd7e91d..6e855f71b1 100644 --- a/packages/constraint-solver/constraint-solver-input.js +++ b/packages/constraint-solver/constraint-solver-input.js @@ -148,7 +148,10 @@ CS.Input.prototype.isEqual = function (otherInput) { // This equality test is also overly sensitive to order, // missing opportunities to declare two inputs equal when only // the order has changed. - return _.isEqual(a.toJSONable(), b.toJSONable()); + return _.isEqual( + _.omit(a.toJSONable(), "catalogCache"), + _.omit(b.toJSONable(), "catalogCache") + ); }; CS.Input.prototype.toJSONable = function () { diff --git a/packages/constraint-solver/constraint-solver.js b/packages/constraint-solver/constraint-solver.js index 17593be080..b03aca8ac8 100644 --- a/packages/constraint-solver/constraint-solver.js +++ b/packages/constraint-solver/constraint-solver.js @@ -56,11 +56,6 @@ CS.PackagesResolver.prototype.resolve = function (dependencies, constraints, 'upgradeIndirectDepPatchVersions')); }); - Profile.time( - "Input#loadFromCatalog (sqlite)", - function () { - input.loadFromCatalog(self.catalogLoader); - }); var resultCache = self._options.resultCache; if (resultCache && resultCache.lastInput && @@ -68,6 +63,12 @@ CS.PackagesResolver.prototype.resolve = function (dependencies, constraints, return resultCache.lastOutput; } + Profile.time( + "Input#loadFromCatalog (sqlite)", + function () { + input.loadFromCatalog(self.catalogLoader); + }); + if (options.previousSolution && options.missingPreviousVersionIsError) { Profile.time("check for previous versions in catalog", function () { _.each(options.previousSolution, function (version, pkg) { From 0376b477a54f8b90f5ee7c35c8043ec238c6b8c6 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Wed, 16 Dec 2015 21:05:06 -0800 Subject: [PATCH 2/2] Add clarifying comment on improved constraint solver method Explain a recent change to CS.Input.isEqual that eliminated a very costly set of SQLite queries against the local package catalog cache on every app rebuild. --- .../constraint-solver/constraint-solver-input.js | 13 +++++++++++-- packages/constraint-solver/constraint-solver.js | 8 ++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-input.js b/packages/constraint-solver/constraint-solver-input.js index 6e855f71b1..e4fd5bf392 100644 --- a/packages/constraint-solver/constraint-solver-input.js +++ b/packages/constraint-solver/constraint-solver-input.js @@ -148,9 +148,18 @@ CS.Input.prototype.isEqual = function (otherInput) { // This equality test is also overly sensitive to order, // missing opportunities to declare two inputs equal when only // the order has changed. + + // Omit `catalogCache` -- it's not actually part of the serialized + // input object (it's only in `toJSONable()` for tests). + // + // Moreover, catalogCache is populated as-needed so their values for + // `a` and `b` will very likely be different even if they represent + // the same input. So by omitting `catalogCache` we no longer need + // to reload the entire relevant part of the catalog from SQLite on + // every rebuild! return _.isEqual( - _.omit(a.toJSONable(), "catalogCache"), - _.omit(b.toJSONable(), "catalogCache") + _.omit(a.toJSONable(), "catalogCache"), + _.omit(b.toJSONable(), "catalogCache") ); }; diff --git a/packages/constraint-solver/constraint-solver.js b/packages/constraint-solver/constraint-solver.js index b03aca8ac8..7f89d69556 100644 --- a/packages/constraint-solver/constraint-solver.js +++ b/packages/constraint-solver/constraint-solver.js @@ -64,10 +64,10 @@ CS.PackagesResolver.prototype.resolve = function (dependencies, constraints, } Profile.time( - "Input#loadFromCatalog (sqlite)", - function () { - input.loadFromCatalog(self.catalogLoader); - }); + "Input#loadFromCatalog (sqlite)", + function () { + input.loadFromCatalog(self.catalogLoader); + }); if (options.previousSolution && options.missingPreviousVersionIsError) { Profile.time("check for previous versions in catalog", function () {