From 979554ab554c828de788436c10facab3c03b99a7 Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Mon, 9 Mar 2015 23:44:47 -0700 Subject: [PATCH] Ban versions not matching top-level constraints in solver. This should speed things up a lot! --- .../constraint-solver-tests.js | 2 +- packages/constraint-solver/input-tests.js | 13 +++- packages/constraint-solver/solver.js | 73 +++++++++++++++---- 3 files changed, 69 insertions(+), 19 deletions(-) diff --git a/packages/constraint-solver/constraint-solver-tests.js b/packages/constraint-solver/constraint-solver-tests.js index aae763c824..7fbdbe7212 100644 --- a/packages/constraint-solver/constraint-solver-tests.js +++ b/packages/constraint-solver/constraint-solver-tests.js @@ -185,7 +185,7 @@ Tinytest.add("constraint solver - no results", function (test) { ]); testWithResolver(test, resolver, function (t, FAIL) { FAIL({foo: "w1.0.0", bar: "1.0.0"}, - /Constraints:[^]+top level/); + /No version of foo satisfies top-level constraints: @1.0.0/); }); }); diff --git a/packages/constraint-solver/input-tests.js b/packages/constraint-solver/input-tests.js index 156bd04512..b952aabd81 100644 --- a/packages/constraint-solver/input-tests.js +++ b/packages/constraint-solver/input-tests.js @@ -329,13 +329,15 @@ Tinytest.add("constraint solver - input - fake PackageConstraint", function (tes }; doFailTest(test, - new CS.Input(["foo"], [fakeConstraint], + new CS.Input(["foo", "bar"], [fakeConstraint], CS.CatalogCache.fromJSONable({ data: { - "foo 1.0.0": [] + "foo 1.0.0": [], + "foo 2.0.0": [], + "bar 1.0.0": ["foo@1.0.0"] } })), - /constraint foo@2.0.0 is not satisfied by foo 1.0.0/); + /constraint foo@1.0.0 is not satisfied by foo 2.0.0/); }); Tinytest.add("constraint solver - input - stack overflow bug", function (test) { @@ -345,8 +347,11 @@ Tinytest.add("constraint solver - input - stack overflow bug", function (test) { // before logic-solver got smarter about avoiding recursion in formula // generation, and it also tests the case where an unsatisfiable constraint is // in .meteor/packages. + // + // It's not actually a good test of logic-solver overflowing the stack anymore, + // because the constraint-solver is smarter now. doFailTest(test, STACK_OVERFLOW_BUG_INPUT, - /constraint follower-livedata@0.9.0 is not satisfied by follower-livedata/); + /No version of follower-livedata satisfies top-level constraints: @0.9.0/); }); diff --git a/packages/constraint-solver/solver.js b/packages/constraint-solver/solver.js index 608b446970..eff0abbe25 100644 --- a/packages/constraint-solver/solver.js +++ b/packages/constraint-solver/solver.js @@ -23,11 +23,57 @@ CS.Solver = function (input, options) { self.steps = []; self.stepsByName = {}; + // package -> array of version strings. If a package has an entry in + // this map, then only the versions in the array are allowed for + // consideration. + self.allowedVersions = self.calculateAllowedVersions(); + self.analysis = {}; self.logic = null; // Logic.Solver, initialized later }; +CS.Solver.prototype.calculateAllowedVersions = function () { + var self = this; + var allowedVersions = {}; + // process top-level constraints, applying them right now! + // we won't even consider versions that don't match them. + // in particular, this is great for equality constraints. + _.each(_.groupBy(self.input.constraints, 'package'), function (cs, p) { + var versions = self.input.catalogCache.getPackageVersions(p); + if (! versions.length) { + // let the main solver deal with this unknown package + return; + } + _.each(cs, function (constr) { + versions = _.filter(versions, function (v) { + return CS.isConstraintSatisfied(p, constr.versionConstraint, v); + }); + }); + // it's important to make sure we allow *some* version of every package + // in order for the rest of the solver to work + if (! versions.length) { + CS.throwConstraintSolverError( + 'No version of ' + p + ' satisfies top-level constraints: ' + + _.map(cs, function (constr) { + return '@' + constr.constraintString; + }).join(', ')); + } + allowedVersions[p] = versions; + }); + + return allowedVersions; +}; + +CS.Solver.prototype.getVersions = function (package) { + var self = this; + if (_.has(self.allowedVersions, package)) { + return self.allowedVersions[package]; + } else { + return self.input.catalogCache.getPackageVersions(package); + } +}; + // A Step consists of a name, an array of terms, and an array of weights. // Steps are optimized one by one. Optimizing a Step means to find // the minimum whole number value for the weighted sum of the terms, @@ -207,7 +253,7 @@ CS.Solver.prototype.analyzeReachability = function () { var visit = function (p) { reachablePackages[p] = true; - _.each(cache.getPackageVersions(p), function (v) { + _.each(self.getVersions(p), function (v) { _.each(cache.getDependencyMap(p, v), function (dep) { // `dep` is a CS.Dependency var p2 = dep.packageConstraint.package; @@ -248,7 +294,7 @@ CS.Solver.prototype.analyzeConstraints = function () { // constraints specified by package versions _.each(_.keys(self.analysis.reachablePackages), function (p) { - _.each(cache.getPackageVersions(p), function (v) { + _.each(self.getVersions(p), function (v) { var pv = pvVar(p, v); _.each(cache.getDependencyMap(p, v), function (dep) { // `dep` is a CS.Dependency @@ -263,9 +309,9 @@ CS.Solver.prototype.analyzeConstraints = function () { }); }; -CS.Solver.prototype.getAllVersions = function (package) { +CS.Solver.prototype.getAllVersionVars = function (package) { var self = this; - return _.map(self.input.catalogCache.getPackageVersions(package), + return _.map(self.getVersions(package), function (v) { return pvVar(package, v); }); @@ -295,7 +341,7 @@ CS.Solver.prototype.getOldnesses = function (stepBaseName, packages) { var rest = new CS.Solver.Step(stepBaseName + '_rest'); _.each(packages, function (p) { - var versions = self.input.catalogCache.getPackageVersions(p); + var versions = self.getVersions(p); var costs = self.pricer.priceVersions( versions, CS.VersionPricer.MODE_UPDATE); addCostsToSteps(p, versions, costs, @@ -313,7 +359,7 @@ CS.Solver.prototype.getGravityPotential = function (stepBaseName, packages) { var rest = new CS.Solver.Step(stepBaseName + '_rest'); _.each(packages, function (p) { - var versions = self.input.catalogCache.getPackageVersions(p); + var versions = self.getVersions(p); var costs = self.pricer.priceVersions( versions, CS.VersionPricer.MODE_GRAVITY_WITH_PATCHES); addCostsToSteps(p, versions, costs, @@ -335,7 +381,7 @@ CS.Solver.prototype.getDistances = function (stepBaseName, packageAndVersions) { _.each(packageAndVersions, function (pvArg) { var package = pvArg.package; var previousVersion = pvArg.version; - var versions = self.input.catalogCache.getPackageVersions(package); + var versions = self.getVersions(package); var costs = self.pricer.priceVersionsWithPrevious( versions, previousVersion); addCostsToSteps(package, versions, costs, @@ -401,7 +447,7 @@ CS.Solver.prototype.getSolution = function (options) { // generate package version variables for known, reachable packages _.each(_.keys(analysis.reachablePackages), function (p) { - var versionVars = self.getAllVersions(p); + var versionVars = self.getAllVersionVars(p); // At most one of ["foo 1.0.0", "foo 1.0.1", ...] is true. logic.require(Logic.atMostOne(versionVars)); // The variable "foo" is true if and only if at least one of the @@ -411,7 +457,7 @@ CS.Solver.prototype.getSolution = function (options) { // generate strong dependency requirements _.each(_.keys(analysis.reachablePackages), function (p) { - _.each(cache.getPackageVersions(p), function (v) { + _.each(self.getVersions(p), function (v) { _.each(cache.getDependencyMap(p, v), function (dep) { // `dep` is a CS.Dependency if (! dep.isWeak) { @@ -465,7 +511,7 @@ CS.Solver.prototype.getSolution = function (options) { var unanticipatedPrereleases = []; _.each(_.keys(analysis.reachablePackages), function (p) { var anticipatedPrereleases = input.anticipatedPrereleases[p]; - _.each(cache.getPackageVersions(p), function (v) { + _.each(self.getVersions(p), function (v) { if (/-/.test(v) && ! (anticipatedPrereleases && _.has(anticipatedPrereleases, v))) { unanticipatedPrereleases.push(pvVar(p, v)); @@ -493,7 +539,7 @@ CS.Solver.prototype.getSolution = function (options) { _.each(toUpdate, function (p) { if (input.isRootDependency(p) && input.isInPreviousSolution(p)) { var parts = self.pricer.partitionVersions( - cache.getPackageVersions(p), input.previousSolution[p]); + self.getVersions(p), input.previousSolution[p]); _.each(parts.older.concat(parts.higherMajor), function (v) { previousRootIncompat.addTerm(pvVar(p, v), 1); }); @@ -664,7 +710,7 @@ var getOkVersions = function (toPackage, vConstraint, targetVersions) { var _getConstraintFormula = function (toPackage, vConstraint) { var self = this; - var targetVersions = self.input.catalogCache.getPackageVersions(toPackage); + var targetVersions = self.getVersions(toPackage); var okVersions = getOkVersions(toPackage, vConstraint, targetVersions); if (okVersions.length === targetVersions.length) { @@ -709,8 +755,7 @@ CS.Solver.prototype.throwConflicts = function () { // c is a CS.Solver.Constraint if (solution.evaluate(c.conflictVar)) { // skipped this constraint - var possibleVersions = - self.input.catalogCache.getPackageVersions(c.toPackage); + var possibleVersions = self.getVersions(c.toPackage); var chosenVersion = _.find(possibleVersions, function (v) { return solution.evaluate(pvVar(c.toPackage, v)); });