Skip to content

Commit

Permalink
Merge pull request #5632 from Polymer/legacy-undefined-noBatch-5631
Browse files Browse the repository at this point in the history 
Fixes for several related dom-repeat chunking issues. Fixes #5631.
  • Loading branch information
@kevinpschaaf
kevinpschaaf authored Feb 22, 2020
2 parents 537da37 + 0797488 commit 5421b5b
Show file tree
Hide file tree
Showing 3 changed files with 326 additions and 255 deletions.
195 changes: 117 additions & 78 deletions lib/elements/dom-repeat.js
View file
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 @@ -299,11 +318,13 @@ export class DomRepeat extends domRepeatBase {
super();
this.__instances = [];
this.__limit = Infinity;
this.__pool = [];
this.__renderDebouncer = null;
this.__itemsIdxToInstIdx = {};
this.__chunkCount = null;
this.__lastChunkTime = null;
this.__renderStartTime = null;
this.__itemsArrayChanged = false;
this.__shouldMeasureChunk = false;
this.__shouldContinueChunking = false;
this.__sortFn = null;
this.__filterFn = null;
this.__observePaths = null;
Expand Down Expand Up @@ -445,57 +466,11 @@ 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();
}
}

__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);
}
}

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

__renderChunk() {
// 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);
}

__observeChanged() {
this.__observePaths = this.observe &&
this.observe.replace('.*', '.').split(' ');
}

__itemsChanged(change) {
if (this.items && !Array.isArray(this.items)) {
console.warn('dom-repeat expected array for `items`, found', this.items);
}
// If path was to an item (e.g. 'items.3' or 'items.3.foo'), forward the
// path to that instance synchronously (returns false for non-item paths)
if (!this.__handleItemPath(change.path, change.value)) {
// Otherwise, the array was reset ('items') or spliced ('items.splices'),
// so queue a full refresh
this.__initializeChunking();
this.__debounceRender(this.__render);
}
}

__handleObservedPaths(path) {
// Handle cases where path changes should cause a re-sort/filter
if (this.__sortFn || this.__filterFn) {
Expand All @@ -514,6 +489,23 @@ export class DomRepeat extends domRepeatBase {
}
}

__itemsChanged(change) {
if (this.items && !Array.isArray(this.items)) {
console.warn('dom-repeat expected array for `items`, found', this.items);
}
// If path was to an item (e.g. 'items.3' or 'items.3.foo'), forward the
// path to that instance synchronously (returns false for non-item paths)
if (!this.__handleItemPath(change.path, change.value)) {
// Otherwise, the array was reset ('items') or spliced ('items.splices'),
// so queue a render. Restart chunking when the items changed (for
// backward compatibility), unless `reuseChunkedInstances` option is set
if (change.path === 'items') {
this.__itemsArrayChanged = true;
}
this.__debounceRender(this.__render);
}
}

/**
* @param {function(this:DomRepeat)} fn Function to debounce.
* @param {number=} delay Delay in ms to debounce by.
Expand Down Expand Up @@ -545,28 +537,34 @@ export class DomRepeat extends domRepeatBase {
// No template found yet
return;
}
this.__applyFullRefresh();
// Reset the pool
// TODO(kschaaf): Reuse pool across turns and nested templates
// Now that objects/arrays are re-evaluated when set, we can safely
// reuse pooled instances across turns, however we still need to decide
// semantics regarding how long to hold, how many to hold, etc.
this.__pool.length = 0;
let items = this.items || [];
// Sort and filter the items into a mapping array from instance->item
const isntIdxToItemsIdx = this.__sortAndFilterItems(items);
// If we're chunking, increase the limit if there are new instances to
// create and schedule the next chunk
if (this.initialCount) {
this.__updateLimit(isntIdxToItemsIdx.length);
}
// Create, update, and/or remove instances
this.__updateInstances(items, isntIdxToItemsIdx);
// Set rendered item count
this._setRenderedItemCount(this.__instances.length);
// If we're chunking, schedule a rAF task to measure/continue chunking
if (this.initialCount &&
(this.__shouldMeasureChunk || this.__shouldContinueChunking)) {
this.__debounceRender(this.__continueChunkingAfterRaf);
}
// Notify users
if (!suppressTemplateNotifications || this.notifyDomChange) {
this.dispatchEvent(new CustomEvent('dom-change', {
bubbles: true,
composed: true
}));
}
// Check to see if we need to render more items
this.__tryRenderChunk();
}

__applyFullRefresh() {
let items = this.items || [];
__sortAndFilterItems(items) {
// Generate array maping the instance index to the items array index
let isntIdxToItemsIdx = new Array(items.length);
for (let i=0; i<items.length; i++) {
isntIdxToItemsIdx[i] = i;
Expand All @@ -580,6 +578,60 @@ export class DomRepeat extends domRepeatBase {
if (this.__sortFn) {
isntIdxToItemsIdx.sort((a, b) => this.__sortFn(items[a], items[b]));
}
return isntIdxToItemsIdx;
}

__updateLimit(filteredItemCount) {
let newCount;
if (!this.__chunkCount ||
(this.__itemsArrayChanged && !this.reuseChunkedInstances)) {
// 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);
// Update the limit based on how many new items we're making, limited
// buy the total size of the list
this.__limit = Math.min(this.__limit + newCount, filteredItemCount);
}
this.__itemsArrayChanged = false;
// Record some state about chunking for use in `__continueChunking`
this.__shouldMeasureChunk = newCount === this.__chunkCount;
this.__shouldContinueChunking = this.__limit < filteredItemCount;
this.__renderStartTime = performance.now();
}

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

__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. 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);
}
}

__updateInstances(items, isntIdxToItemsIdx) {
// items->inst map kept for item path forwarding
const itemsIdxToInstIdx = this.__itemsIdxToInstIdx = {};
let instIdx = 0;
Expand Down Expand Up @@ -622,10 +674,7 @@ export class DomRepeat extends domRepeatBase {
}

__detachAndRemoveInstance(idx) {
let inst = this.__detachInstance(idx);
if (inst) {
this.__pool.push(inst);
}
this.__detachInstance(idx);
this.__instances.splice(idx, 1);
}

Expand All @@ -638,17 +687,7 @@ export class DomRepeat extends domRepeatBase {
}

__insertInstance(item, instIdx, itemIdx) {
let inst = this.__pool.pop();
if (inst) {
// TODO(kschaaf): If the pool is shared across turns, hostProps
// need to be re-set to reused instances in addition to item
inst._setPendingProperty(this.as, item);
inst._setPendingProperty(this.indexAs, instIdx);
inst._setPendingProperty(this.itemsIndexAs, itemIdx);
inst._flushProperties();
} else {
inst = this.__stampInstance(item, instIdx, itemIdx);
}
const inst = this.__stampInstance(item, instIdx, itemIdx);
let beforeRow = this.__instances[instIdx + 1];
let beforeNode = beforeRow ? beforeRow.children[0] : this;
wrap(wrap(this).parentNode).insertBefore(inst.root, beforeNode);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/dom-repeat-elements.js
View file
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 5421b5b

Please sign in to comment.