Skip to content

Commit

Permalink
Updates from review.
Browse files Browse the repository at this point in the history 
* Refactoring `__render` for readability
* Removing `__pool`; this was never used in v2: since we reset the pool every update and items are only ever pushed at detach time and we only detach at the end of updates (as opposed to v1 which had more sophisticated splicing)
  • Loading branch information
@kevinpschaaf
kevinpschaaf committed Feb 22, 2020
1 parent b40840b commit 0797488
Showing 1 changed file with 90 additions and 100 deletions.
190 changes: 90 additions & 100 deletions lib/elements/dom-repeat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,11 @@ export class DomRepeat extends domRepeatBase {
super();
this.__instances = [];
this.__limit = Infinity;
this.__pool = [];
this.__renderDebouncer = null;
this.__itemsIdxToInstIdx = {};
this.__chunkCount = null;
this.__renderStartTime = null;
this.__restartChunking = true;
this.__itemsArrayChanged = false;
this.__shouldMeasureChunk = false;
this.__shouldContinueChunking = false;
this.__sortFn = null;
Expand Down Expand Up @@ -467,81 +466,11 @@ export class DomRepeat extends domRepeatBase {
return Math.ceil(1000/rate);
}

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

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

__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
if (change.path === 'items' && !this.reuseChunkedInstances) {
this.__restartChunking = true;
}
this.__debounceRender(this.__render);
}
}

__handleObservedPaths(path) {
// Handle cases where path changes should cause a re-sort/filter
if (this.__sortFn || this.__filterFn) {
Expand All @@ -560,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 @@ -591,15 +537,23 @@ 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', {
Expand All @@ -609,8 +563,8 @@ export class DomRepeat extends domRepeatBase {
}
}

__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 @@ -624,11 +578,60 @@ 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);
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 @@ -671,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 @@ -687,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

0 comments on commit 0797488

Please sign in to comment.