From b065d1450a4795d14633ece90510292229f6bc38 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 11 Apr 2019 10:45:50 -0700 Subject: [PATCH] Cleanup, add tests. * remove old implementation * add API docs * rename some API * add dynamicFn to dep count * add test for method as dependency --- lib/mixins/property-effects.js | 226 ++++++++++--------------- test/runner.html | 2 + test/unit/path-effects.html | 3 + test/unit/property-effects-elements.js | 23 ++- test/unit/property-effects.html | 167 +++++++++--------- 5 files changed, 207 insertions(+), 214 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index fc82d767db..82bb81eb14 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -415,45 +415,59 @@ function runReflectEffect(inst, property, props, oldProps, info) { function runComputedEffects(inst, changedProps, oldProps, hasPaths) { let computeEffects = inst[TYPES.COMPUTE]; if (computeEffects) { - if (orderedComputed == 2) { + if (orderedComputed) { + // Runs computed effects in efficient order by keeping a topologically- + // sorted queue of compute effects to run, and inserting subsequently + // invalidated effects as they are run dedupeId++; - const order = orderedComputedDeps(inst); + const order = getComputedOrder(inst); const queue = []; for (let p in changedProps) { - addEffectsFor(p, computeEffects, queue, order, hasPaths); + enqueueEffectsFor(p, computeEffects, queue, order, hasPaths); } let info; while ((info = queue.shift())) { if (runComputedEffect(inst, '', changedProps, oldProps, info, hasPaths)) { - addEffectsFor(info.methodInfo, computeEffects, queue, order, hasPaths); + enqueueEffectsFor(info.methodInfo, computeEffects, queue, order, hasPaths); } } Object.assign(/** @type {!Object} */ (oldProps), inst.__dataOld); Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); inst.__dataPending = null; } else { + // Original Polymer 2.x computed effects order, which continues running + // effects until no further computed properties have been invalidated let inputProps = changedProps; while (runEffects(inst, computeEffects, inputProps, oldProps, hasPaths)) { Object.assign(/** @type {!Object} */ (oldProps), inst.__dataOld); Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); inputProps = inst.__dataPending; inst.__dataPending = null; - if (orderedComputed) { - // Ensure all computed properties are de-duped against the same turn - dedupeId--; - } } } } } -const insertEffect = (info, effects, order) => { +/** + * Inserts a computed effect into a queue, given the specified order. Performs + * the insert using a binary search. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {Object} info Property effects metadata + * @param {Array} queue Ordered queue of effects + * @param {Map} order Map of computed property name->topological + * sort order + */ +const insertEffect = (info, queue, order) => { let start = 0; - let end = effects.length - 1; + let end = queue.length - 1; let idx = -1; while (start <= end) { const mid = (start + end) >> 1; - const cmp = order.get(effects[mid].methodInfo) - order.get(info.methodInfo); + // Note `methodInfo` is where the computed property name is stored in + // the effect metadata + const cmp = order.get(queue[mid].methodInfo) - order.get(info.methodInfo); if (cmp < 0) { start = mid + 1; } else if (cmp > 0) { @@ -466,10 +480,24 @@ const insertEffect = (info, effects, order) => { if (idx < 0) { idx = end + 1; } - effects.splice(idx, 0, info); + queue.splice(idx, 0, info); }; -const addEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { +/** + * Inserts all downstream computed effects invalidated by the specified property + * into the topologically-sorted queue of effects to be run. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {string} prop Property name + * @param {Object} computeEffects Computed effects for this element + * @param {Array} queue Topologically-sorted queue of computed effects + * to be run + * @param {Map} order Map of computed property name->topological + * sort order + * @param {boolean} hasPaths True with `changedProps` contains one or more paths + */ +const enqueueEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { const rootProperty = hasPaths ? root(prop) : prop; const fxs = computeEffects[rootProperty]; if (fxs) { @@ -484,21 +512,43 @@ const addEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { } }; -function orderedComputedDeps(inst) { +/** + * Generates and retrieves a memoized map of computed property name to its + * topologically-sorted order. + * + * The map is generated by first assigning a "dependency count" to each property + * (defined as number properties it depends on, including its method for + * "dynamic functions"). Any properties that have no dependencies are added to + * the `ready` queue, which are properties whose order can be added to the final + * order map. Properties are popped off the `ready` one by one and a.) added as + * the next property in the order map, and b.) each property that it is a + * dependency for has its dep count decremented (and if that property's dep + * count goes to zero, it is added to the `ready` queue), until all properties + * have been visited and ordered. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {!Polymer_PropertyEffects} inst The instance to retrieve the computed + * effect order for. + * @return {Map} Map of computed property name->topological sort + * order + */ +function getComputedOrder(inst) { let ordered = inst.constructor.__orderedComputedDeps; if (!ordered) { ordered = new Map(); const effects = inst[TYPES.COMPUTE]; - const {counts, next} = edgeCounts(inst); + const {counts, ready} = dependencyCounts(inst); let curr; - while ((curr = next.pop())) { + while ((curr = ready.shift())) { ordered.set(curr, ordered.size); const computedByCurr = effects[curr]; if (computedByCurr) { computedByCurr.forEach(fx => { - const p = fx.info.methodInfo; - if (--counts[p] === 0) { - next.push(p); + // Note `methodInfo` is where the computed property name is stored + const computedProp = fx.info.methodInfo; + if (--counts[computedProp] === 0) { + ready.push(computedProp); } }); } @@ -508,24 +558,37 @@ function orderedComputedDeps(inst) { return ordered; } -function edgeCounts(inst) { +/** + * Generates a map of property-to-dependency count (`counts`, where "dependency + * count" is the number of dependencies a given property has assuming it is a + * computed property, otherwise 0). It also returns a pre-populated list of + * `ready` properties that have no dependencies. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {!Polymer_PropertyEffects} inst The instance to generate dependency + * counts for. + * @return {Object} Object containing `counts` map (property-to-dependency + * count) and pre-populated `ready` array of properties that had zero + * dependencies. + */ +function dependencyCounts(inst) { const props = inst.constructor._properties; - const depsForComputed = inst[COMPUTE_INFO]; + const infoForComputed = inst[COMPUTE_INFO]; const counts = {}; - const next = []; + const ready = []; for (let p in props) { - const deps = depsForComputed[p]; - if (deps) { - counts[p] = deps.args.length; + const info = infoForComputed[p]; + if (info) { + // Be sure to add the method name itself in case of "dynamic functions" + counts[p] = info.args.length + (info.dynamicFn ? 1 : 0); } else { - next.push(p); + ready.push(p); } } - return {counts, next}; + return {counts, ready}; } -const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; - /** * Implements the "computed property" effect by running the method with the * values of the arguments specified in the `info` object and setting the @@ -540,120 +603,19 @@ const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; * @return {boolean} True when the property being computed changed * @private */ -function runComputedEffect(inst, property, changedProps, oldProps, info, hasPaths) { - if (orderedComputed == 1) { - // Compute any computed dependencies first; this recurses through the - // dependency graph to ensure computed properties are never computed with - // dependencies that may become invalidated later in this turn - computeDependencies(inst, info, changedProps, oldProps, hasPaths); - // If the dependency was not transitive, it's definitely dirty and needs to - // be computed; if it is transitive, check its arguments against the current - // changed props and only re-compute if it is dirty - if (property === TRANSITIVE_DEPENDENCY && - !computedPropertyIsDirty(inst, info, changedProps, hasPaths)) { - return; - } - } +function runComputedEffect(inst, property, changedProps, oldProps, info) { // Dirty check dependencies and run if any invalid let result = runMethodEffect(inst, property, changedProps, oldProps, info); // Abort if method returns a no-op result if (result === NOOP) { - return; + return false; } let computedProp = info.methodInfo; if (inst.__dataHasAccessor && inst.__dataHasAccessor[computedProp]) { return inst._setPendingProperty(computedProp, result, true); } else { inst[computedProp] = result; - } -} - -/** - * Returns any dependencies of a computed property that are themselves - * also computed. - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {?} info Computed effect metadata - * @return {Array} Array of computed effect metadata for depenencies - * @private - */ - function computedDependenciesFor(inst, info) { - let deps = info.computedDependencies; - if (!deps) { - deps = info.computedDependencies = []; - info.args.forEach(arg => { - const dep = arg.rootProperty || arg.name; - const depInfo = inst[COMPUTE_INFO][dep]; - if (depInfo) { - deps.push(depInfo); - } - }); - } - return deps; -} - -/** - * Runs computed effects for dependencies of the computed property effect - * described by `info`. - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {Object} info Effect metadata - * @param {Object} changedProps Bag of current property changes - * @param {Object} oldProps Bag of previous values for changed properties - * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {void} - * @private - */ - function computeDependencies(inst, info, changedProps, oldProps, hasPaths) { - let deps = computedDependenciesFor(inst, info); - if (deps.length) { - deps.forEach(depInfo => { - if (depInfo.lastRun !== info.lastRun) { - depInfo.lastRun = info.lastRun; - runComputedEffect(inst, TRANSITIVE_DEPENDENCY, changedProps, oldProps, depInfo, hasPaths); - } - }); - } -} - -/** - * Returns whether the computed effect has any changed dependencies. - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {?} info Effect metadata - * @param {Object} changedProps Bag of current property changes - * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {boolean} true when the computed effect should be re-calculated - * @private - */ -function computedPropertyIsDirty(inst, info, changedProps, hasPaths) { - return info.dynamicFn && propertyIsDirty(inst, {rootProperty: info.methodName}, changedProps, hasPaths) || - info.args.some(arg => propertyIsDirty(inst, arg, changedProps, hasPaths)); -} - -/** - * Returns whether any changed props match the effect trigger - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {DataTrigger} trigger Descriptor - * @param {Object} changedProps Bag of current property changes - * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {boolean} true when the property is dirty - * @private - */ -function propertyIsDirty(inst, trigger, changedProps, hasPaths) { - if (hasPaths) { - [changedProps, inst.__dataPending].some(props => { - for (let p in props) { - if (pathMatchesTrigger(p, trigger)) { - return true; - } - } - }); return false; - } else { - return Boolean(changedProps && trigger.rootProperty in changedProps || - inst.__dataPending && trigger.rootProperty in inst.__dataPending); } } diff --git a/test/runner.html b/test/runner.html index 341b02899d..8596a652aa 100644 --- a/test/runner.html +++ b/test/runner.html @@ -33,8 +33,10 @@ 'unit/property-effects.html', 'unit/property-effects.html?legacyUndefined=true', 'unit/property-effects.html?legacyUndefined=true&legacyNoBatch=true', + 'unit/property-effects.html?orderedComputed=true', 'unit/property-effects-template.html', 'unit/path-effects.html', + 'unit/path-effects.html?orderedComputed=true', 'unit/shady.html', 'unit/shady-events.html', 'unit/shady-content.html', diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html index feebb9c901..973d9a5d3c 100644 --- a/test/unit/path-effects.html +++ b/test/unit/path-effects.html @@ -21,6 +21,9 @@