Merge pull request #102 from the-grid/targeted-vars-improvements

Drop redundant space solved checks, major perf improvement

CHANGELOG.md

@@ -2,7 +2,10 @@
 
 ## Unreleased
 
-- No changes
+- Drop support for getting the targeted indexes through a callback
+- Eliminated something internally that was redundant and causing a big perf regression; in other words the solver should run much faster now on large data
+- Internal; eliminated targetedIndexes, merged the whole thing into `space.unsolvedVarIndexes`
+- Internal; eliminated duplicate `isSolved` checks
 
 ## v2.2.1:
 

src/config.js

@@ -47,7 +47,6 @@ import {
   domain_max,
   domain_min,
   domain_numarr,
-  domain_isSolved,
   domain_isSolvedArr,
   domain_intersection,
   domain_intersectionArrArr,
@@ -75,7 +74,6 @@ function config_create() {
     next_var_func: 'naive',
     next_value_func: 'min',
     targetedVars: 'all',
-    targetedIndexes: 'all', // function, string or array. initialized by config_generateVars, will contain var index for targetedVars
     var_dist_options: {},
     timeout_callback: undefined,
 
@@ -103,7 +101,6 @@ function config_clone(config, newDomains) {
     next_var_func,
     next_value_func,
     targetedVars,
-    targetedIndexes,
     var_dist_options,
     timeout_callback,
     constant_cache,
@@ -122,7 +119,6 @@ function config_clone(config, newDomains) {
     next_var_func,
     next_value_func,
     targetedVars: targetedVars instanceof Array ? targetedVars.slice(0) : targetedVars,
-    targetedIndexes: targetedIndexes instanceof Array ? targetedIndexes.slice(0) : targetedIndexes,
     var_dist_options: JSON.parse(JSON.stringify(var_dist_options)),  // TOFIX: clone this more efficiently
     timeout_callback, // by reference because it's a function if passed on...
 
@@ -354,18 +350,6 @@ function config_generateVars(config, space) {
 
     space.vardoms[varIndex] = domain_numarr(domain);
   }
-
-  if (config.targetedVars === 'all') {
-    config.targetedIndexes = 'all';
-  } else {
-    config.targetedIndexes = [];
-    for (let i = 0; i < config.targetedVars.length; ++i) {
-      let name = config.targetedVars[i];
-      let index = allVarNames.indexOf(name);
-      if (index < 0) THROW('TARGETED_VARS_SHOULD_EXIST_NOW');
-      config.targetedIndexes.push(index);
-    }
-  }
 }
 
 /**
@@ -1002,24 +986,6 @@ function config_initForSpace(config, space) {
   config_populateVarPropHash(config);
 
   ASSERT(config._varToPropagators, 'should have generated hash');
-  let targets = getAllTargetVars(space);
-  // a var is considered unsolved if it is in fact not solved AND it either is either explicitly targeted or constrained by at least one constraint
-  space.unsolvedVarIndexes = targets.filter(varIndex => !domain_isSolved(space.vardoms[varIndex]) && (config.targetedVars !== 'all' || config._varToPropagators[varIndex] || (config._constrainedAway && config._constrainedAway.indexOf(varIndex) >= 0)));
-}
-
-function getAllTargetVars(space) {
-  let configTargetedIndexes = space.config.targetedIndexes;
-
-  if (configTargetedIndexes === 'all' || !configTargetedIndexes.length) {
-    return space.config.all_var_names.map((n, i) => i);
-  }
-
-  if (configTargetedIndexes instanceof Array) {
-    return configTargetedIndexes;
-  }
-
-  ASSERT(typeof configTargetedIndexes === 'function', 'config.targetedIndexes should be a func at this point', configTargetedIndexes);
-  return configTargetedIndexes(space);
 }
 
 // BODY_STOP

src/distribution/var.js

@@ -25,11 +25,11 @@ const WORSE = 3;
  * @returns {number}
  */
 function distribution_getNextVar(space) {
-  let targetVars = space.unsolvedVarIndexes;
+  let unsolvedVarIndexes = space.unsolvedVarIndexes;
   let configNextVarFunc = space.config.next_var_func;
   // if it's a function it should return the name of the next var to process
   if (typeof configNextVarFunc === 'function') {
-    return configNextVarFunc(space, targetVars);
+    return configNextVarFunc(space, unsolvedVarIndexes);
   }
 
   let distName = configNextVarFunc;
@@ -52,7 +52,7 @@ function distribution_getNextVar(space) {
     }
   }
 
-  return _distribution_varFindBest(space, targetVars, isBetterVar, configVarFilter, configNextVarFunc);
+  return _distribution_varFindBest(space, unsolvedVarIndexes, isBetterVar, configVarFilter, configNextVarFunc);
 }
 
 /**

src/search.js

@@ -4,7 +4,7 @@ import {
 } from './helpers';
 import {
   space_createClone,
-  space_isSolved,
+  space_updateUnsolvedVarList,
   space_propagate,
 } from './space';
 import {
@@ -78,7 +78,7 @@ function search_depthFirstLoop(space, stack, state, createNextSpaceNode) {
     return false;
   }
 
-  let solved = space_isSolved(space);
+  let solved = space_updateUnsolvedVarList(space);
   if (solved) {
     _search_onSolve(state, space, stack);
     return true;

src/solver.js

@@ -673,7 +673,6 @@ class Solver {
     clone.all_constraints = '<removed>';
     clone.initial_domains = '<removed>';
     if (targeted !== 'all') clone.targetedVars = '<removed>';
-    clone.targetedIndexes = '<removed>';
     clone._propagators = '<removed>';
     clone._varToPropagators = '<removed>';
 

src/space.js

@@ -6,6 +6,7 @@ import {
 
   ASSERT,
   ASSERT_DOMAIN,
+  THROW,
 } from './helpers';
 
 import {
@@ -67,8 +68,8 @@ function space_createFromConfig(config) {
 function space_createClone(space) {
   ASSERT(space._class === '$space', 'SPACE_SHOULD_BE_SPACE');
 
-  let unsolvedVarIndexes = space_filterUnsolvedVarIndexes(space);
   let vardomsCopy = space.vardoms.slice(0);
+  let unsolvedVarIndexes = space.unsolvedVarIndexes.slice(0); // note: the list should have been updated before cloning
 
   // only for debugging
   let _depth;
@@ -103,22 +104,12 @@ function space_toConfig(space) {
   return config_clone(space.config, newDomains);
 }
 
-function space_filterUnsolvedVarIndexes(space) {
-  let unsolvedVarIndexes = [];
-  let vardoms = space.vardoms;
-  for (let i = 0; i < space.unsolvedVarIndexes.length; ++i) {
-    let varIndex = space.unsolvedVarIndexes[i];
-    if (!domain_isSolved(vardoms[varIndex])) unsolvedVarIndexes.push(varIndex);
-  }
-  return unsolvedVarIndexes;
-}
-
 /**
  * Concept of a space that holds config, some named domains (referred to as "vars"), and some propagators
  *
  * @param {$config} config
  * @param {$domain[]} vardoms Maps 1:1 to config.all_var_names
- * @param {string[]} unsolvedVarIndexes Note: Indexes to the config.all_var_names array
+ * @param {number[]} unsolvedVarIndexes Note: Indexes to the config.all_var_names array
  * @param {number} _depth
  * @param {number} _child
  * @param {string} _path
@@ -133,7 +124,6 @@ function space_createNew(config, vardoms, unsolvedVarIndexes, _depth, _child, _p
 
     config,
 
-    // TODO: should we track all_vars all_unsolved_vars AND target_vars target_unsolved_vars? because i think so.
     vardoms,
     unsolvedVarIndexes,
 
@@ -157,8 +147,44 @@ function space_createNew(config, vardoms, unsolvedVarIndexes, _depth, _child, _p
 function space_initFromConfig(space) {
   let config = space.config;
   ASSERT(config, 'should have a config');
+  ASSERT(config._class === '$config', 'should be a config');
 
   config_initForSpace(config, space);
+  initializeUnsolvedVars(space, config);
+}
+/**
+ * Initialized space.unsolvedVarIndexes with either the explicitly targeted var indexes
+ * or any index that is currently unsolved and either constrained or marked as such
+ *
+ * @param {$space} space
+ * @param {$config} config
+ */
+function initializeUnsolvedVars(space, config) {
+  let unsolvedVarIndexes = space.unsolvedVarIndexes;
+  let targetVarNames = config.targetedVars;
+  let vardoms = space.vardoms;
+
+  ASSERT(unsolvedVarIndexes.length === 0, 'should be initialized with empty list');
+
+  if (targetVarNames === 'all') {
+    for (let varIndex = 0, n = vardoms.length; varIndex < n; ++varIndex) {
+      if (!domain_isSolved(vardoms[varIndex])) {
+        if (config._varToPropagators[varIndex] || (config._constrainedAway && config._constrainedAway.indexOf(varIndex) >= 0)) {
+          unsolvedVarIndexes.push(varIndex);
+        }
+      }
+    }
+  } else {
+    let allVarNames = space.config.all_var_names;
+    for (let i = 0, n = targetVarNames.length; i < n; ++i) {
+      let varName = targetVarNames[i];
+      let varIndex = allVarNames.indexOf(varName);
+      if (varIndex < 0) THROW('E_TARGETED_VARS_SHOULD_EXIST_NOW');
+      if (!domain_isSolved(vardoms[varIndex])) {
+        unsolvedVarIndexes.push(varIndex);
+      }
+    }
+  }
 }
 
 /**
@@ -290,42 +316,15 @@ function space_abortSearch(space) {
  * @param {$space} space
  * @returns {boolean}
  */
-function space_isSolved(space) {
-  ASSERT(space._class === '$space', 'SPACE_SHOULD_BE_SPACE');
-  let targetedIndexes = space.config.targetedIndexes;
-
-  if (targetedIndexes === 'all' || !targetedIndexes.length) return _space_isSolvedForAll(space);
-  return _space_isSolvedForSome(space, targetedIndexes);
-}
-function _space_isSolvedForSome(space, targetedIndexes) {
-  ASSERT(space._class === '$space', 'SPACE_SHOULD_BE_SPACE');
-  ASSERT(targetedIndexes !== 'all' && targetedIndexes.length, 'REQUIRES_SOME_TARGETS');
-
-  let unsolvedVarIndexes = space.unsolvedVarIndexes;
-
-  let j = 0;
-  for (let i = 0; i < unsolvedVarIndexes.length; i++) {
-    let varIndex = unsolvedVarIndexes[i];
-    if (targetedIndexes.indexOf(varIndex) >= 0) {
-      let domain = space.vardoms[varIndex];
-      ASSERT_DOMAIN(domain);
-
-      if (!domain_isSolved(domain)) {
-        unsolvedVarIndexes[j++] = varIndex;
-      }
-    }
-  }
-  unsolvedVarIndexes.length = j;
-  return j === 0;
-}
-function _space_isSolvedForAll(space) {
+function space_updateUnsolvedVarList(space) {
   ASSERT(space._class === '$space', 'SPACE_SHOULD_BE_SPACE');
   let unsolvedVarIndexes = space.unsolvedVarIndexes;
+  let vardoms = space.vardoms;
 
   let j = 0;
-  for (let i = 0; i < unsolvedVarIndexes.length; i++) {
+  for (let i = 0, n = unsolvedVarIndexes.length; i < n; i++) {
     let varIndex = unsolvedVarIndexes[i];
-    let domain = space.vardoms[varIndex];
+    let domain = vardoms[varIndex];
     ASSERT_DOMAIN(domain);
 
     if (!domain_isSolved(domain)) {
@@ -385,7 +384,7 @@ export {
   space_createRoot,
   space_getVarSolveState,
   space_initFromConfig,
-  space_isSolved,
+  space_updateUnsolvedVarList,
   space_propagate,
   space_solution,
   space_toConfig,