From e9aebd72ad6d2b8ae4f52b548353cc7c52475e99 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 23 Oct 2015 18:13:11 -0700 Subject: [PATCH 01/10] dom-repeat chunked/throttled render API --- src/lib/template/dom-repeat.html | 243 ++++++++++++++++++++++++------- 1 file changed, 189 insertions(+), 54 deletions(-) diff --git a/src/lib/template/dom-repeat.html b/src/lib/template/dom-repeat.html index 0a56ed7891..b75f8ed496 100644 --- a/src/lib/template/dom-repeat.html +++ b/src/lib/template/dom-repeat.html @@ -188,7 +188,75 @@ * This is useful in rate-limiting shuffing of the view when * item changes may be frequent. */ - delay: Number + delay: Number, + + /** + * When `limit` is defined, the number of actually rendered template + * instances will be limited to this count. + * + * Note that if `initialCount` is used, the `limit` property will be + * automatically controlled and should not be set by the user. + */ + limit: { + value: Infinity, + type: Number, + observer: '_limitChanged' + }, + + /** + * Defines an initial count of template instances to render after setting + * the `items` array, before the next paint, and puts the `dom-repeat` + * into "chunking mode". The remaining items will be created and rendered + * incrementally at each animation frame therof until all instnaces have + * been rendered. The number of instances created each animation frame + * can be controlled via the `chunkCount` property. + */ + initialCount: { + type: Number, + value: 0 + }, + + /** + * When `initialCount` is used, defines the number of instances to be + * created at each animation frame after rendering the `initialCount`. + * When left to the default `'auto'` value, the chunk count will be + * throttled automatically using a best effort scheme to maintain the + * value of the `targetFramerate` property. + */ + chunkCount: { + type: Number, + value: 'auto' + }, + + /** + * When `initialCount` is used and `chunkCount` is set to `'auto'`, this + * property defines a frame rate to target by throttling the number of + * instances rendered each frame to not exceed the budget for the target + * frame rate. Setting this to a higher number will allow lower latency + * and higher throughput for things like event handlers, but will result + * in a longer time for the remaining items to complete rendering. + */ + targetFramerate: { + type: Number, + value: 20 + }, + + /** + * Maximum number of removed instances to pool for reuse when rows are + * added in a future turn. By default, pooling is enabled. + * + * Set to 0 to disable pooling, which will allow all removed instances to + * be garbage collected. + */ + poolSize: { + type: Number, + value: 1000 + }, + + _targetFrameTime: { + computed: '_computeFrameTime(targetFramerate)' + } + }, behaviors: [ @@ -196,23 +264,32 @@ ], observers: [ - '_itemsChanged(items.*)' + '_itemsChanged(items.*)', + '_initializeChunkCount(initialCount, chunkCount)' ], created: function() { this._instances = []; + this._pool = []; + this._boundRenderChunk = this._renderChunk.bind(this); }, detached: function() { for (var i=0; i= 0 && this._chunkCount && + Math.min(this.limit, this._instances.length) < this.items.length) { + this.debounce('renderChunk', this._requestRenderChunk); + } + }, + + _requestRenderChunk: function() { + requestAnimationFrame(this._boundRenderChunk); + }, + + _renderChunk: function() { + if (this.chunkCount == 'auto') { + // Simple auto chunkSize throttling algorithm based on feedback loop: + // measure actual time between frames and scale chunk count by ratio + // of target/actual frame time + var prevChunkTime = this._currChunkTime; + this._currChunkTime = performance.now(); + var chunkTime = this._currChunkTime - prevChunkTime; + if (chunkTime) { + var ratio = this._targetFrameTime / chunkTime; + this._chunkCount = Math.round(this._chunkCount * ratio) || 1; + } + } + this.limit += this._chunkCount; + }, + _observeChanged: function() { this._observePaths = this.observe && this.observe.replace('.*', '.').split(' '); @@ -324,7 +443,7 @@ if (this._needFullRefresh) { this._applyFullRefresh(); this._needFullRefresh = false; - } else { + } else if (this._keySplices.length) { if (this._sortFn) { this._applySplicesUserSort(this._keySplices); } else { @@ -338,14 +457,28 @@ } this._keySplices = []; this._indexSplices = []; - // Update final _keyToInstIdx and instance indices + // Update final _keyToInstIdx, instance indices, and replace placeholders var keyToIdx = this._keyToInstIdx = {}; - for (var i=0; i=0; i--) { var inst = this._instances[i]; + if (inst.isPlaceholder && i=this.limit) { + inst = this._insertRow(i, inst.__key__, true, true); + } keyToIdx[inst.__key__] = i; - inst.__setProperty(this.indexAs, i, true); + if (!inst.isPlaceholder) { + inst.__setProperty(this.indexAs, i, true); + } } + // Reset the pool + // TODO(kschaaf): Allow pool to be reused across turns & between nested + // peer repeats (requires updating parentProps when reusing from pool) + this._pool.length = 0; + // Notify users this.fire('dom-change'); + // Check to see if we need to render more items + this._tryRenderChunk(); }, // Render method 1: full refesh @@ -385,17 +518,20 @@ var key = keys[i]; var inst = this._instances[i]; if (inst) { - inst.__setProperty('__key__', key, true); - inst.__setProperty(this.as, c.getItem(key), true); + if (inst.isPlaceholder) { + inst.__key__ = key; + } else { + inst.__setProperty('__key__', key, true); + inst.__setProperty(this.as, c.getItem(key), true); + } } else { - this._instances.push(this._insertRow(i, key)); + this._insertRow(i, key); } } // Remove any extra instances from previous state - for (; i=i; j--) { + this._detachRow(j); } - this._instances.splice(keys.length, this._instances.length-keys.length); }, _keySort: function(a, b) { @@ -414,7 +550,6 @@ var c = this.collection; var instances = this._instances; var keyMap = {}; - var pool = []; var sortFn = this._sortFn || this._keySort.bind(this); // Dedupe added and removed keys to a final added/removed map splices.forEach(function(s) { @@ -448,8 +583,7 @@ var idx = removedIdxs[i]; // Removed idx may be undefined if item was previously filtered out if (idx !== undefined) { - pool.push(this._detachRow(idx)); - instances.splice(idx, 1); + this._detachRow(idx); } } } @@ -468,12 +602,12 @@ // Insertion-sort new instances into place (from pool or newly created) var start = 0; for (var i=0; i=0; i--) { - var inst = this._instances[i]; - if (inst.isPlaceholder) { - this._instances[i] = this._insertRow(i, inst.key, pool, true); - } - } }, - _detachRow: function(idx) { + _detachRow: function(idx, keepInstance) { var inst = this._instances[idx]; if (!inst.isPlaceholder) { var parentNode = Polymer.dom(this).parentNode; @@ -545,22 +661,41 @@ var el = inst._children[i]; Polymer.dom(inst.root).appendChild(el); } + this._pool.push(inst); + } + if (!keepInstance) { + this._instances.splice(idx, 1); } return inst; }, - _insertRow: function(idx, key, pool, replace) { + _insertRow: function(idx, key, replace, makePlaceholder) { var inst; - if (inst = pool && pool.pop()) { - inst.__setProperty(this.as, this.collection.getItem(key), true); - inst.__setProperty('__key__', key, true); + if (makePlaceholder || idx >= this.limit) { + inst = { + isPlaceholder: true, + __key__: key + }; } else { - inst = this._generateRow(idx, key); + if (inst = this._pool.pop()) { + inst.__setProperty(this.as, this.collection.getItem(key), true); + inst.__setProperty('__key__', key, true); + } else { + inst = this._generateRow(idx, key); + } + var beforeRow = this._instances[replace ? idx + 1 : idx]; + var beforeNode = beforeRow && !beforeRow.isPlaceholder ? beforeRow._children[0] : this; + var parentNode = Polymer.dom(this).parentNode; + Polymer.dom(parentNode).insertBefore(inst.root, beforeNode); + } + if (replace) { + if (makePlaceholder) { + this._detachRow(idx, true); + } + this._instances[idx] = inst; + } else { + this._instances.splice(idx, 0, inst); } - var beforeRow = this._instances[replace ? idx + 1 : idx]; - var beforeNode = beforeRow ? beforeRow._children[0] : this; - var parentNode = Polymer.dom(this).parentNode; - Polymer.dom(parentNode).insertBefore(inst.root, beforeNode); return inst; }, From e99e5faa89579209705fbb04ba5f4f6c8a49654f Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 30 Oct 2015 19:56:43 -0700 Subject: [PATCH 02/10] Add tests and fix issues. --- src/lib/template/dom-repeat.html | 70 ++-- test/unit/dom-repeat-elements.html | 61 ++++ test/unit/dom-repeat.html | 502 +++++++++++++++++++++++++++++ 3 files changed, 603 insertions(+), 30 deletions(-) diff --git a/src/lib/template/dom-repeat.html b/src/lib/template/dom-repeat.html index b75f8ed496..f39144cee4 100644 --- a/src/lib/template/dom-repeat.html +++ b/src/lib/template/dom-repeat.html @@ -241,18 +241,6 @@ value: 20 }, - /** - * Maximum number of removed instances to pool for reuse when rows are - * added in a future turn. By default, pooling is enabled. - * - * Set to 0 to disable pooling, which will allow all removed instances to - * be garbage collected. - */ - poolSize: { - type: Number, - value: 1000 - }, - _targetFrameTime: { computed: '_computeFrameTime(targetFramerate)' } @@ -338,10 +326,10 @@ return Math.ceil(1000/rate); }, - _initializeChunkCount: function(initialCount, chunkCount) { - if (initialCount) { - this.limit = initialCount; - this._chunkCount = parseInt(chunkCount, 10) || initialCount; + _initializeChunkCount: function() { + if (this.initialCount) { + this.limit = this.initialCount; + this._chunkCount = parseInt(this.chunkCount, 10) || this.initialCount; } }, @@ -387,6 +375,9 @@ this._error(this._logf('dom-repeat', 'expected array for `items`,' + ' found', this.items)); } + if (this._instances.length && this.initialCount) { + this._initializeChunkCount(); + } this._keySplices = []; this._indexSplices = []; this._needFullRefresh = true; @@ -472,8 +463,12 @@ } } // Reset the pool - // TODO(kschaaf): Allow pool to be reused across turns & between nested - // peer repeats (requires updating parentProps when reusing from pool) + // TODO(kschaaf): Reuse pool across turns and nested templates + // Requires updating parentProps and dealing with the fact that path + // notifications won't reach instances sitting in the pool, which + // could result in out-of-sync instances since simply re-setting + // `item` may not be sufficient if the pooled instance happens to be + // the same item. this._pool.length = 0; // Notify users this.fire('dom-change'); @@ -518,10 +513,8 @@ var key = keys[i]; var inst = this._instances[i]; if (inst) { - if (inst.isPlaceholder) { - inst.__key__ = key; - } else { - inst.__setProperty('__key__', key, true); + inst.__key__ = key; + if (!inst.isPlaceholder && i < this.limit) { inst.__setProperty(this.as, c.getItem(key), true); } } else { @@ -661,7 +654,9 @@ var el = inst._children[i]; Polymer.dom(inst.root).appendChild(el); } - this._pool.push(inst); + if (!keepInstance) { + this._pool.push(inst); + } } if (!keepInstance) { this._instances.splice(idx, 1); @@ -669,6 +664,13 @@ return inst; }, + _detachAllRows: function() { + for (var i=0; i= this.limit) { @@ -678,6 +680,8 @@ }; } else { if (inst = this._pool.pop()) { + // TODO(kschaaf): If the pool is shared across turns, parentProps + // need to be re-set to reused instances in addition to item/key inst.__setProperty(this.as, this.collection.getItem(key), true); inst.__setProperty('__key__', key, true); } else { @@ -749,18 +753,24 @@ // Called as side-effect of a host property change, responsible for // notifying parent path change on each inst _forwardParentProp: function(prop, value) { - this._instances.forEach(function(inst) { - inst.__setProperty(prop, value, true); - }, this); + for (var i=0, i$=this._instances, il=i$.length; i. path change, @@ -771,7 +781,7 @@ var key = path.substring(0, dot < 0 ? path.length : dot); var idx = this._keyToInstIdx[key]; var inst = this._instances[idx]; - if (inst) { + if (inst && !inst.isPlaceholder) { if (dot >= 0) { path = this.as + '.' + path.substring(dot+1); inst._notifyPath(path, value, true); diff --git a/test/unit/dom-repeat-elements.html b/test/unit/dom-repeat-elements.html index 274294dae0..0ef268b41e 100644 --- a/test/unit/dom-repeat-elements.html +++ b/test/unit/dom-repeat-elements.html @@ -411,3 +411,64 @@ }); + + + + + + + + + + diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html index 65de283c22..45eff0fe18 100644 --- a/test/unit/dom-repeat.html +++ b/test/unit/dom-repeat.html @@ -71,6 +71,12 @@

inDocumentRepeater

x-primitive-large

+

x-repeat-limit

+ + +

x-repeat-chunked

+ +
@@ -93,6 +99,7 @@

x-primitive-large

stamped[38] .. 3-3-3 */ + suite('errors', function() { test('items must be array', function() { @@ -3348,6 +3355,501 @@

x-primitive-large

}); + + suite('limit', function() { + + var checkItemOrder = function(stamped) { + for (var i=0; i lastLength); + } + if (stamped.length < 100) { + lastLength = stamped.length; + checkUntilComplete(); + } else { + // Final rendering at exact item count + assert.equal(stamped.length, 100); + done(); + } + }; + var checkUntilComplete = function() { + // On polyfilled MO, need to wait one setTimeout before rAF + if (MutationObserver._isPolyfilled) { + setTimeout(function() { + requestAnimationFrame(checkCount); + }); + } else { + requestAnimationFrame(checkCount); + } + }; + + chunked.items = chunked.preppedItems.slice(); + checkUntilComplete(); + + }); + + test('mutations during chunked rendering', function(done) { + + var checkItemOrder = function(stamped) { + var last = -1; + for (var i=0; i last); + last = curr; + } + }; + + var mutateArray = function(repeater, renderedCount) { + // The goal here is to remove & add some, and do it over + // the threshold of where we have currently rendered items, and + // ensure that the prop values of the newly inserted items are in + // ascending order so we can do a simple check in checkItemOrder + var overlap = 2; + var remove = 4; + var add = 6; + var start = renderedCount.length - overlap; + if (start + add < repeater.items.length) { + var end = start + remove; + var args = ['items', start, remove]; + var startVal = repeater.items[start].prop; + var endVal = repeater.items[end].prop; + var delta = (endVal - startVal) / add; + for (var i=0; i lastLength); + } + if (stamped.length < chunked.items.length) { + if (mutateCount-- > 0) { + mutateArray(chunked, stamped); + } + lastLength = stamped.length; + checkUntilComplete(); + } else { + // Final rendering at exact item count + assert.equal(stamped.length, chunked.items.length); + done(); + } + }; + var checkUntilComplete = function() { + // On polyfilled MO, need to wait one setTimeout before rAF + if (MutationObserver._isPolyfilled) { + setTimeout(function() { + requestAnimationFrame(checkCount); + }); + } else { + requestAnimationFrame(checkCount); + } + }; + + chunked.items = chunked.preppedItems.slice(); + checkUntilComplete(); + + }); + + + test('mutations during chunked rendering, sort & filtered', function(done) { + + var checkItemOrder = function(stamped) { + var last = Infinity; + for (var i=0; i lastLength); + } + } + if (stamped.length < filteredLength) { + if (mutateCount-- > 0) { + mutateArray(chunked, stamped); + } + lastLength = stamped.length; + checkUntilComplete(); + } else { + assert.equal(stamped.length, filteredLength); + done(); + } + }; + var checkUntilComplete = function() { + // On polyfilled MO, need to wait one setTimeout before rAF + if (MutationObserver._isPolyfilled) { + setTimeout(function() { + requestAnimationFrame(checkCount); + }); + } else { + requestAnimationFrame(checkCount); + } + }; + + chunked.$.repeater.sort = function(a, b) { + return b.prop - a.prop; + }; + chunked.$.repeater.filter = function(a) { + return (a.prop % 2) === 0; + }; + chunked.items = chunked.preppedItems.slice(); + checkUntilComplete(); + + }); + + }); + From 59c27fa4c3c1fdbcb4480ba6600322837d9b2a5f Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 30 Oct 2015 20:14:05 -0700 Subject: [PATCH 03/10] Clean up cruft. --- src/lib/template/dom-repeat.html | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/lib/template/dom-repeat.html b/src/lib/template/dom-repeat.html index f39144cee4..78af69c0f0 100644 --- a/src/lib/template/dom-repeat.html +++ b/src/lib/template/dom-repeat.html @@ -334,7 +334,7 @@ }, _tryRenderChunk: function() { - if (this.limit >= 0 && this._chunkCount && + if (this._chunkCount && Math.min(this.limit, this._instances.length) < this.items.length) { this.debounce('renderChunk', this._requestRenderChunk); } @@ -664,13 +664,6 @@ return inst; }, - _detachAllRows: function() { - for (var i=0; i= this.limit) { From f447c0eb70b48574eb26aa5a33245e13df60347d Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 4 Nov 2015 17:39:42 -0800 Subject: [PATCH 04/10] Remove limit & chunkCount API. Refactor insert/remove. --- src/lib/template/dom-repeat.html | 222 +++++++++++----------- test/unit/dom-repeat-elements.html | 4 +- test/unit/dom-repeat.html | 284 ++++++++++++++--------------- 3 files changed, 239 insertions(+), 271 deletions(-) diff --git a/src/lib/template/dom-repeat.html b/src/lib/template/dom-repeat.html index 78af69c0f0..db74fa9635 100644 --- a/src/lib/template/dom-repeat.html +++ b/src/lib/template/dom-repeat.html @@ -190,19 +190,6 @@ */ delay: Number, - /** - * When `limit` is defined, the number of actually rendered template - * instances will be limited to this count. - * - * Note that if `initialCount` is used, the `limit` property will be - * automatically controlled and should not be set by the user. - */ - limit: { - value: Infinity, - type: Number, - observer: '_limitChanged' - }, - /** * Defines an initial count of template instances to render after setting * the `items` array, before the next paint, and puts the `dom-repeat` @@ -213,28 +200,16 @@ */ initialCount: { type: Number, - value: 0 - }, - - /** - * When `initialCount` is used, defines the number of instances to be - * created at each animation frame after rendering the `initialCount`. - * When left to the default `'auto'` value, the chunk count will be - * throttled automatically using a best effort scheme to maintain the - * value of the `targetFramerate` property. - */ - chunkCount: { - type: Number, - value: 'auto' + observer: '_initializeChunking' }, /** - * When `initialCount` is used and `chunkCount` is set to `'auto'`, this - * property defines a frame rate to target by throttling the number of - * instances rendered each frame to not exceed the budget for the target - * frame rate. Setting this to a higher number will allow lower latency - * and higher throughput for things like event handlers, but will result - * in a longer time for the remaining items to complete rendering. + * When `initialCount` is used, this property defines a frame rate to + * target by throttling the number of instances rendered each frame to + * not exceed the budget for the target frame rate. Setting this to a + * higher number will allow lower latency and higher throughput for + * things like event handlers, but will result in a longer time for the + * remaining items to complete rendering. */ targetFramerate: { type: Number, @@ -252,32 +227,29 @@ ], observers: [ - '_itemsChanged(items.*)', - '_initializeChunkCount(initialCount, chunkCount)' + '_itemsChanged(items.*)' ], created: function() { this._instances = []; this._pool = []; - this._boundRenderChunk = this._renderChunk.bind(this); + this._limit = Infinity; + var self = this; + this._boundRenderChunk = function() { + self._renderChunk(); + }; }, detached: function() { for (var i=0; i=0; i--) { var inst = this._instances[i]; - if (inst.isPlaceholder && i=this.limit) { - inst = this._insertRow(i, inst.__key__, true, true); + if (inst.isPlaceholder && i=this._limit) { + inst = this._downgradeInstance(i, inst.__key__); } keyToIdx[inst.__key__] = i; if (!inst.isPlaceholder) { @@ -514,16 +482,16 @@ var inst = this._instances[i]; if (inst) { inst.__key__ = key; - if (!inst.isPlaceholder && i < this.limit) { + if (!inst.isPlaceholder && i < this._limit) { inst.__setProperty(this.as, c.getItem(key), true); } } else { - this._insertRow(i, key); + this._insertPlaceholder(i, key); } } // Remove any extra instances from previous state for (var j=this._instances.length-1; j>=i; j--) { - this._detachRow(j); + this._removeInstance(j); } }, @@ -576,7 +544,7 @@ var idx = removedIdxs[i]; // Removed idx may be undefined if item was previously filtered out if (idx !== undefined) { - this._detachRow(idx); + this._removeInstance(idx); } } } @@ -624,7 +592,7 @@ idx = end + 1; } // Insert instance at insertion point - this._insertRow(idx, key); + this._insertPlaceholder(idx, key); return idx; }, @@ -638,65 +606,48 @@ splices.forEach(function(s) { // Detach & pool removed instances for (var i=0; i= this.limit) { - inst = { - isPlaceholder: true, - __key__: key - }; - } else { - if (inst = this._pool.pop()) { - // TODO(kschaaf): If the pool is shared across turns, parentProps - // need to be re-set to reused instances in addition to item/key - inst.__setProperty(this.as, this.collection.getItem(key), true); - inst.__setProperty('__key__', key, true); - } else { - inst = this._generateRow(idx, key); - } - var beforeRow = this._instances[replace ? idx + 1 : idx]; - var beforeNode = beforeRow && !beforeRow.isPlaceholder ? beforeRow._children[0] : this; - var parentNode = Polymer.dom(this).parentNode; - Polymer.dom(parentNode).insertBefore(inst.root, beforeNode); - } - if (replace) { - if (makePlaceholder) { - this._detachRow(idx, true); - } - this._instances[idx] = inst; - } else { - this._instances.splice(idx, 0, inst); + _removeInstance: function(idx) { + var inst = this._detachInstance(idx); + if (inst) { + this._pool.push(inst); } - return inst; + this._instances.splice(idx, 1); }, - _generateRow: function(idx, key) { + _insertPlaceholder: function(idx, key) { + this._instances.splice(idx, 0, { + isPlaceholder: true, + __key__: key + }); + }, + + _generateInstance: function(idx, key) { var model = { __key__: key }; @@ -706,6 +657,37 @@ return inst; }, + _upgradePlaceholder: function(idx, key) { + var inst = this._pool.pop(); + if (inst) { + // TODO(kschaaf): If the pool is shared across turns, parentProps + // need to be re-set to reused instances in addition to item/key + inst.__setProperty(this.as, this.collection.getItem(key), true); + inst.__setProperty('__key__', key, true); + } else { + inst = this._generateInstance(idx, key); + } + var beforeRow = this._instances[idx + 1 ]; + var beforeNode = beforeRow && !beforeRow.isPlaceholder ? beforeRow._children[0] : this; + var parentNode = Polymer.dom(this).parentNode; + Polymer.dom(parentNode).insertBefore(inst.root, beforeNode); + this._instances[idx] = inst; + return inst; + }, + + _downgradeInstance: function(idx, key) { + var inst = this._detachInstance(idx); + if (inst) { + this._pool.push(inst); + } + inst = { + isPlaceholder: true, + __key__: key + }; + this._instances[idx] = inst; + return inst; + }, + // Implements extension point from Templatizer mixin _showHideChildren: function(hidden) { for (var i=0; i