Fixes for several related dom-repeat chunking issues. Fixes #5631.

* 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).

Author: kevinpschaaf

lib/elements/dom-repeat.js

@@ -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
       },
 
       /**
@@ -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
       }
 
     };
@@ -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;
@@ -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() {
@@ -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);
     }
   }
@@ -561,8 +607,6 @@ export class DomRepeat extends domRepeatBase {
         composed: true
       }));
     }
-    // Check to see if we need to render more items
-    this.__tryRenderChunk();
   }
 
   __applyFullRefresh() {
@@ -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;

test/unit/dom-repeat-elements.js

@@ -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>
 `,