Skip to content

Commit

Permalink
Applies micro-optimizations and removes obsolete settings
Browse files Browse the repository at this point in the history 
Applies micro-optimizations that were found to improve element creation benchmarks by 5-10%, and removes obsolete settings:

* Removed `legacyNoBatch` and `legacyNotifyOrder` settings.
* dom-if/repeat: `dom-change` and `renderedCount` no longer fire with `legacyOptimizations` set.
* legacy-element-mixin: `isAttached` now set before calling `connectedCallback` so it is batched with initial rendering.
* `PropertiesChanged`: property accessor code now inlined for efficiency rather than calling `_get/_setProperty`. The `__dataCounter` tracking flag has been moved here to avoid the need to override `_flushProperties` in `PropertyEffects`.
* `PropertyEffects`: inlined `runEffectsForProperty` into `runEffects` for efficiency. Removed wrapping around sending data events.
* `async`: In the microtask scheduler, now only provoke a DOM mutation if needed.
  • Loading branch information
Steven Orvell committed Sep 12, 2019
1 parent 27779a3 commit 280f4f0
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 174 deletions.
4 changes: 2 additions & 2 deletions lib/elements/dom-if.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { microTask } from '../utils/async.js';
import { root } from '../utils/path.js';
import { wrap } from '../utils/wrap.js';
import { hideElementsGlobally } from '../utils/hide-template-controls.js';
import { fastDomIf, strictTemplatePolicy } from '../utils/settings.js';
import { fastDomIf, strictTemplatePolicy, legacyOptimizations } from '../utils/settings.js';
import { showHideChildren, templatize } from '../utils/templatize.js';

/**
Expand Down Expand Up @@ -244,7 +244,7 @@ class DomIfBase extends PolymerElement {
this.__teardownInstance();
}
this._showHideChildren();
if (this.if != this._lastIf) {
if (!legacyOptimizations && this.if != this._lastIf) {
this.dispatchEvent(new CustomEvent('dom-change', {
bubbles: true,
composed: true
Expand Down
13 changes: 8 additions & 5 deletions lib/elements/dom-repeat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { matches, translate } from '../utils/path.js';
import { timeOut, microTask } from '../utils/async.js';
import { wrap } from '../utils/wrap.js';
import { hideElementsGlobally } from '../utils/hide-template-controls.js';
import { legacyOptimizations } from '../utils/settings.js';

/**
* @constructor
Expand Down Expand Up @@ -239,7 +240,7 @@ export class DomRepeat extends domRepeatBase {
*/
renderedItemCount: {
type: Number,
notify: true,
notify: !legacyOptimizations,
readOnly: true
},

Expand Down Expand Up @@ -545,10 +546,12 @@ export class DomRepeat extends domRepeatBase {
// Set rendered item count
this._setRenderedItemCount(this.__instances.length);
// Notify users
this.dispatchEvent(new CustomEvent('dom-change', {
bubbles: true,
composed: true
}));
if (!legacyOptimizations) {
this.dispatchEvent(new CustomEvent('dom-change', {
bubbles: true,
composed: true
}));
}
// Check to see if we need to render more items
this.__tryRenderChunk();
}
Expand Down
2 changes: 1 addition & 1 deletion lib/legacy/legacy-element-mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ export const LegacyElementMixin = dedupingMixin((base) => {
* @override
*/
connectedCallback() {
super.connectedCallback();
this.isAttached = true;
super.connectedCallback();
this.attached();
}

Expand Down
13 changes: 11 additions & 2 deletions lib/mixins/properties-changed.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,15 @@ export const PropertiesChanged = dedupingMixin(
/* eslint-disable valid-jsdoc */
/** @this {PropertiesChanged} */
get() {
return this._getProperty(property);
// Inline for perf instead of using `_getProperty`
return this.__data[property];
},
/** @this {PropertiesChanged} */
set: readOnly ? function () {} : function (value) {
this._setProperty(property, value);
// Inline for perf instead of using `_setProperty`
if (this._setPendingProperty(property, value, true)) {
this._invalidateProperties();
}
}
/* eslint-enable */
});
Expand All @@ -180,6 +184,9 @@ export const PropertiesChanged = dedupingMixin(
this.__dataPending = null;
this.__dataOld = null;
this.__dataInstanceProps = null;
/** @type {number} */
// NOTE: used to track re-entrant calls to `_flushProperties`
this.__dataCounter = 0;
this.__serializing = false;
this._initializeProperties();
}
Expand Down Expand Up @@ -367,6 +374,7 @@ export const PropertiesChanged = dedupingMixin(
* @override
*/
_flushProperties() {
this.__dataCounter++;
const props = this.__data;
const changedProps = this.__dataPending;
const old = this.__dataOld;
Expand All @@ -375,6 +383,7 @@ export const PropertiesChanged = dedupingMixin(
this.__dataOld = null;
this._propertiesChanged(props, changedProps, old);
}
this.__dataCounter--;
}

/**
Expand Down
11 changes: 1 addition & 10 deletions lib/mixins/property-accessors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import '../utils/boot.js';
import { dedupingMixin } from '../utils/mixin.js';
import { camelToDashCase, dashToCamelCase } from '../utils/case-map.js';
import { PropertiesChanged } from './properties-changed.js';
import { legacyNoBatch } from '../utils/settings.js';

// Save map of native properties; this forms a blacklist or properties
// that won't have their values "saved" by `saveAccessorValue`, since
Expand Down Expand Up @@ -295,15 +294,7 @@ export const PropertyAccessors = dedupingMixin(superClass => {
* @override
*/
_definePropertyAccessor(property, readOnly) {
if (legacyNoBatch) {
// Ensure properties are not flushed when adding instance effects
const ready = this.__dataReady;
this.__dataReady = false;
saveAccessorValue(this, property);
this.__dataReady = ready;
} else {
saveAccessorValue(this, property);
}
saveAccessorValue(this, property);
super._definePropertyAccessor(property, readOnly);
}

Expand Down
115 changes: 31 additions & 84 deletions lib/mixins/property-effects.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { camelToDashCase, dashToCamelCase } from '../utils/case-map.js';
import { PropertyAccessors } from './property-accessors.js';
/* for annotated effects */
import { TemplateStamp } from './template-stamp.js';
import { sanitizeDOMValue, legacyUndefined, legacyNoBatch, legacyNotifyOrder, orderedComputed, removeNestedTemplates, fastDomIf } from '../utils/settings.js';
import { sanitizeDOMValue, legacyUndefined, orderedComputed, removeNestedTemplates, fastDomIf } from '../utils/settings.js';

// Monotonically increasing unique ID used for de-duping effects triggered
// from multiple properties in the same turn
Expand Down Expand Up @@ -125,12 +125,22 @@ function ensureOwnEffectMap(model, type, cloneArrays) {
function runEffects(inst, effects, props, oldProps, hasPaths, extraArgs) {
if (effects) {
let ran = false;
let id = dedupeId++;
const id = dedupeId++;
for (let prop in props) {
if (runEffectsForProperty(
inst, /** @type {!Object} */ (effects), id, prop, props, oldProps,
hasPaths, extraArgs)) {
ran = true;
// Inline `runEffectsForProperty` for perf.
let rootProperty = hasPaths ? root(prop) : prop;
let fxs = effects[rootProperty];
if (fxs) {
for (let i=0, l=fxs.length, fx; (i<l) && (fx=fxs[i]); i++) {
if ((!fx.info || fx.info.lastRun !== id) &&
(!hasPaths || pathMatchesTrigger(prop, fx.trigger))) {
if (fx.info) {
fx.info.lastRun = id;
}
fx.fn(inst, prop, props, oldProps, fx.info, hasPaths, extraArgs);
ran = true;
}
}
}
}
return ran;
Expand Down Expand Up @@ -310,7 +320,8 @@ function dispatchNotifyEvent(inst, eventName, value, path) {
if (path) {
detail.path = path;
}
wrap(/** @type {!HTMLElement} */(inst)).dispatchEvent(new CustomEvent(eventName, { detail }));
// Note, since this does not bubble, it should not need to be wrapped.
inst.dispatchEvent(new CustomEvent(eventName, { detail }));
}

/**
Expand Down Expand Up @@ -806,7 +817,7 @@ function applyBindingValue(inst, node, binding, part, value) {
inst._enqueueClient(node);
}
}
} else if (!legacyNoBatch || inst.__dataReady) {
} else {
// In legacy no-batching mode, bindings applied before dataReady are
// equivalent to the "apply config" phase, which only set managed props
inst._setUnmanagedPropertyToNode(node, prop, value);
Expand Down Expand Up @@ -1204,13 +1215,13 @@ function notifySplices(inst, array, path, splices) {
const splicesData = { indexSplices: splices };
// Legacy behavior stored splices in `__data__` so it was *not* ephemeral.
// To match this behavior, we store splices directly on the array.
if (legacyUndefined) {
if (legacyUndefined && !inst._overrideLegacyUndefined) {
array.splices = splicesData;
}
inst.notifyPath(path + '.splices', splicesData);
inst.notifyPath(path + '.length', array.length);
// Clear splice data only when it's stored on the array.
if (legacyUndefined) {
if (legacyUndefined && !inst._overrideLegacyUndefined) {
splicesData.indexSplices = [];
}
}
Expand Down Expand Up @@ -1311,11 +1322,6 @@ export const PropertyEffects = dedupingMixin(superClass => {
/** @type {boolean} */
// Used to identify users of this mixin, ala instanceof
this.__isPropertyEffectsClient = true;
/** @type {number} */
// NOTE: used to track re-entrant calls to `_flushProperties`
// path changes dirty check against `__dataTemp` only during one "turn"
// and are cleared when `__dataCounter` returns to 0.
this.__dataCounter = 0;
/** @type {boolean} */
this.__dataClientsReady;
/** @type {Array} */
Expand Down Expand Up @@ -1690,12 +1696,6 @@ export const PropertyEffects = dedupingMixin(superClass => {
this.__dataToNotify = this.__dataToNotify || {};
this.__dataToNotify[property] = shouldNotify;
}
if (legacyNoBatch) {
this._invalidateProperties();
// Returning false here means "the change was already handled, no need
// to invalidate"
return false;
}
return true;
}
return false;
Expand Down Expand Up @@ -1748,45 +1748,6 @@ export const PropertyEffects = dedupingMixin(superClass => {
}
}

/**
* Overrides superclass implementation.
*
* @override
* @return {void}
* @protected
*/
_flushProperties() {
this.__dataCounter++;
if (legacyNoBatch && !this.__dataReady) {
// The first time this element is being flushed, propagate pending
// data down the tree first; this approximates 1.x's distributeConfig
this._propagatePropertyChanges(this.__dataPending, {}, false);
// Flushing clients recurses, running the initial call to
// `_flushProperties` down the tree. It also causes this element to
// become __dataReady, enabling effects to run.
this._flushClients();
// Capture element data and flush properties one-by one to defeat
// Polymer 2.x's batched _propertiesChanged scheme; this approximates
// 1.x's applyConfig. Data is reset to mimic 1.x behavior of
// properties incrementally being initialized
const changedProps = this.__data;
const notify = this.__dataToNotify;
this.__data = {};
this.__dataPending = this.__dataToNotify = null;
for (let prop in changedProps) {
const value = changedProps[prop];
this.__data[prop] = value;
this.__dataPending = {[prop]: value};
this.__dataOld = {};
this.__dataToNotify = {[prop]: notify && notify[prop]};
super._flushProperties();
}
} else {
super._flushProperties();
}
this.__dataCounter--;
}

/**
* Flushes any clients previously enqueued via `_enqueueClient`, causing
* their `_flushProperties` method to run.
Expand Down Expand Up @@ -1928,35 +1889,21 @@ export const PropertyEffects = dedupingMixin(superClass => {
this.__dataHasPaths = false;
let notifyProps;
// Compute properties
if (legacyNoBatch) {
// When not batching, computed effects may re-enter, so capture
// notifying properties early. Use simple runEffects rather than
// runComputedEffects since computed effects will run immediately
// rather than being batched.
notifyProps = this.__dataToNotify;
this.__dataToNotify = null;
runEffects(this, this[TYPES.COMPUTE], changedProps, oldProps, hasPaths);
} else {
runComputedEffects(this, changedProps, oldProps, hasPaths);
// Clear notify properties prior to possible reentry (propagate, observe),
// but after computing effects have a chance to add to them
notifyProps = this.__dataToNotify;
this.__dataToNotify = null;
}
runComputedEffects(this, changedProps, oldProps, hasPaths);
// Clear notify properties prior to possible reentry (propagate, observe),
// but after computing effects have a chance to add to them
notifyProps = this.__dataToNotify;
this.__dataToNotify = null;
// Propagate properties to clients
this._propagatePropertyChanges(changedProps, oldProps, hasPaths);
// Flush clients
this._flushClients();
// Reflect properties
runEffects(this, this[TYPES.REFLECT], changedProps, oldProps, hasPaths);
// Notify properties to host (1.x order)
if (notifyProps && legacyNotifyOrder) {
runNotifyEffects(this, notifyProps, changedProps, oldProps, hasPaths);
}
// Observe properties
runEffects(this, this[TYPES.OBSERVE], changedProps, oldProps, hasPaths);
// Notify properties to host (2.x order)
if (notifyProps && !legacyNotifyOrder) {
// Notify properties to host
if (notifyProps) {
runNotifyEffects(this, notifyProps, changedProps, oldProps, hasPaths);
}
// Clear temporary cache at end of turn
Expand Down Expand Up @@ -2497,7 +2444,7 @@ export const PropertyEffects = dedupingMixin(superClass => {
}
// When the `legacyUndefined` flag is enabled, pass a no-op value
// so that the observer, computed property, or compound binding is aborted.
if (legacyUndefined && value === undefined && args.length > 1) {
if (legacyUndefined && !this._overrideLegacyUndefined && value === undefined && args.length > 1) {
return NOOP;
}
values[i] = value;
Expand Down Expand Up @@ -2710,7 +2657,7 @@ export const PropertyEffects = dedupingMixin(superClass => {
* create and link an instance of the template metadata associated with a
* particular stamping.
*
* @override
* @override
* @param {!HTMLTemplateElement} template Template containing binding
* bindings
* @param {boolean=} instanceBinding When false (default), performs
Expand All @@ -2720,7 +2667,7 @@ export const PropertyEffects = dedupingMixin(superClass => {
* list of bound templates.
* @return {!TemplateInfo} Template metadata object; for `runtimeBinding`,
* this is an instance of the prototypical template info
* @protected
* @protected
* @suppress {missingProperties} go/missingfnprops
*/
_bindTemplate(template, instanceBinding) {
Expand Down
7 changes: 6 additions & 1 deletion lib/utils/async.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ let microtaskCurrHandle = 0;
let microtaskLastHandle = 0;
let microtaskCallbacks = [];
let microtaskNodeContent = 0;
let microtaskScheduled = false;
let microtaskNode = document.createTextNode('');
new window.MutationObserver(microtaskFlush).observe(microtaskNode, {characterData: true});

function microtaskFlush() {
microtaskScheduled = false;
const len = microtaskCallbacks.length;
for (let i = 0; i < len; i++) {
let cb = microtaskCallbacks[i];
Expand Down Expand Up @@ -181,7 +183,10 @@ const microTask = {
* @return {number} Handle used for canceling task
*/
run(callback) {
microtaskNode.textContent = microtaskNodeContent++;
if (!microtaskScheduled) {
microtaskScheduled = true;
microtaskNode.textContent = microtaskNodeContent++;
}
microtaskCallbacks.push(callback);
return microtaskCurrHandle++;
},
Expand Down
Loading

0 comments on commit 280f4f0

Please sign in to comment.