Skip to content

Commit

Permalink
Fixes for several related dom-repeat chunking issues. Fixes #5631.
Browse files Browse the repository at this point in the history
* Only restart chunking (resetting the list to the initialCount) if the `items` array itself changed (and not splices to the array), to match Polymer 1 behavior.
* Add `reuseChunkedInstances` option to allow reusing instances even when `items` changes; this is likely the more common optimal case when using immutable data, but making it optional for backward compatibility.
* Only measure render time and throttle the chunk size if we rendered a full chunk of new items. Ensures that fast re-renders of existing items don't cause the chunk size to scale up dramatically, subsequently causing too many new items to be created in one chunk.
* Increase the limit by the chunk size as part of any render if there are new items to render, rather than only as a result of rendering.
* Continue chunking by comparing the filtered item count to the limit (not the unfiltered item count).
  • Loading branch information
kevinpschaaf committed Feb 21, 2020
1 parent c5c7eec commit b40840b
Show file tree
Hide file tree
Showing 3 changed files with 289 additions and 208 deletions.
111 changes: 80 additions & 31 deletions lib/elements/dom-repeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,13 @@ export class DomRepeat extends domRepeatBase {
/**
* 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 instances have
* into "chunking mode". The remaining items (and any future items as a
* result of pushing onto the array) will be created and rendered
* incrementally at each animation frame thereof until all instances have
* been rendered.
*/
initialCount: {
type: Number,
observer: '__initializeChunking'
type: Number
},

/**
Expand Down Expand Up @@ -285,6 +285,25 @@ export class DomRepeat extends domRepeatBase {
*/
notifyDomChange: {
type: Boolean
},

/**
* When chunking is enabled via `initialCount` and the `items` array is
* set to a new array, this flag controls whether the previously rendered
* instances are reused or not.
*
* When `true`, any previously rendered template instances are updated in
* place to their new item values synchronously in one shot, and then any
* further items (if any) are chunked out. When `false`, the list is
* returned back to its `initialCount` (any instances over the initial
* count are discarded) and the remainder of the list is chunked back in.
* Set this to `true` to avoid re-creating the list and losing scroll
* position, although note that when changing the list to completely
* different data the render thread will be blocked until all existing
* instances are updated to their new data.
*/
reuseChunkedInstances: {
type: Boolean
}

};
Expand All @@ -303,7 +322,10 @@ export class DomRepeat extends domRepeatBase {
this.__renderDebouncer = null;
this.__itemsIdxToInstIdx = {};
this.__chunkCount = null;
this.__lastChunkTime = null;
this.__renderStartTime = null;
this.__restartChunking = true;
this.__shouldMeasureChunk = false;
this.__shouldContinueChunking = false;
this.__sortFn = null;
this.__filterFn = null;
this.__observePaths = null;
Expand Down Expand Up @@ -445,36 +467,58 @@ export class DomRepeat extends domRepeatBase {
return Math.ceil(1000/rate);
}

__initializeChunking() {
if (this.initialCount) {
this.__limit = this.initialCount;
this.__chunkCount = this.initialCount;
this.__lastChunkTime = performance.now();
__updateLimitAndScheduleNextChunk(filteredItemCount) {
let newCount;
if (this.__restartChunking) {
this.__restartChunking = false;
// Limit next render to the initial count
this.__limit = Math.min(filteredItemCount, this.initialCount);
// Subtract off any existing instances to determine the number of
// instances that will be created
newCount = Math.max(this.__limit - this.__instances.length, 0);
// Initialize the chunk size with how many items we're creating
this.__chunkCount = newCount || 1;
} else {
// The number of new instances that will be created is based on the
// existing instances, the new list size, and the maximum chunk size
newCount = Math.min(
Math.max(filteredItemCount - this.__instances.length, 0),
this.__chunkCount);
// Increase the limit based on how many new items we're making
this.__limit = Math.min(this.__limit + newCount, filteredItemCount);
}
}

__tryRenderChunk() {
// Debounced so that multiple calls through `_render` between animation
// frames only queue one new rAF (e.g. array mutation & chunked render)
if (this.items && this.__limit < this.items.length) {
this.__debounceRender(this.__requestRenderChunk);
// Schedule a rAF task to measure the total time to render the chunk
// (including layout/paint plus any other thread contention), throttle the
// chunk size accordingly, and schedule another render if there is more to
// chunk
this.__shouldMeasureChunk = newCount === this.__chunkCount;
this.__shouldContinueChunking = this.__limit < filteredItemCount;
this.__renderStartTime = performance.now();
if (this.__shouldMeasureChunk || this.__shouldContinueChunking) {
this.__debounceRender(this.__continueChunkingAfterRaf);
}
}

__requestRenderChunk() {
requestAnimationFrame(()=>this.__renderChunk());
__continueChunkingAfterRaf() {
requestAnimationFrame(() => this.__continueChunking());
}

__renderChunk() {
__continueChunking() {
// 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
let currChunkTime = performance.now();
let ratio = this._targetFrameTime / (currChunkTime - this.__lastChunkTime);
this.__chunkCount = Math.round(this.__chunkCount * ratio) || 1;
this.__limit += this.__chunkCount;
this.__lastChunkTime = currChunkTime;
this.__debounceRender(this.__render);
// measure actual time between frames and scale chunk count by ratio of
// target/actual frame time. Only modify chunk size if our measurement
// reflects the cost of a creating a full chunk's worth of instances; this
// avoids scaling up the chunk size if we e.g. quickly re-rendered instances
// in place
if (this.__shouldMeasureChunk) {
const renderTime = performance.now() - this.__renderStartTime;
const ratio = this._targetFrameTime / renderTime;
this.__chunkCount = Math.round(this.__chunkCount * ratio) || 1;
}
// Enqueue a new render if we haven't reached the full size of the list
if (this.__shouldContinueChunking) {
this.__debounceRender(this.__render);
}
}

__observeChanged() {
Expand All @@ -491,7 +535,9 @@ export class DomRepeat extends domRepeatBase {
if (!this.__handleItemPath(change.path, change.value)) {
// Otherwise, the array was reset ('items') or spliced ('items.splices'),
// so queue a full refresh
this.__initializeChunking();
if (change.path === 'items' && !this.reuseChunkedInstances) {
this.__restartChunking = true;
}
this.__debounceRender(this.__render);
}
}
Expand Down Expand Up @@ -561,8 +607,6 @@ export class DomRepeat extends domRepeatBase {
composed: true
}));
}
// Check to see if we need to render more items
this.__tryRenderChunk();
}

__applyFullRefresh() {
Expand All @@ -580,6 +624,11 @@ export class DomRepeat extends domRepeatBase {
if (this.__sortFn) {
isntIdxToItemsIdx.sort((a, b) => this.__sortFn(items[a], items[b]));
}
// If we're chunking, increase the limit if there are new instances to
// create and schedule the next chunk
if (this.initialCount) {
this.__updateLimitAndScheduleNextChunk(isntIdxToItemsIdx.length);
}
// items->inst map kept for item path forwarding
const itemsIdxToInstIdx = this.__itemsIdxToInstIdx = {};
let instIdx = 0;
Expand Down
2 changes: 1 addition & 1 deletion test/unit/dom-repeat-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ Polymer({
});
Polymer({
_template: html`
<template id="repeater" is="dom-repeat" items="{{items}}" initial-count="10">
<template id="repeater" is="dom-repeat" items="{{items}}" initial-count="10" target-framerate="25">
<x-wait>{{item.prop}}</x-wait>
</template>
`,
Expand Down
Loading

0 comments on commit b40840b

Please sign in to comment.