From 68a89fe705091b46f418396a51f12564415bbc8e Mon Sep 17 00:00:00 2001 From: Denis Hilt Date: Wed, 15 Sep 2021 16:30:22 +0300 Subject: [PATCH] issue-23 fix default size calculations on remove via Buffer.update (and Cache.update) --- src/classes/buffer.ts | 59 +++++++++++++++++++------------ src/classes/buffer/cache.ts | 27 ++++++++------ src/classes/buffer/defaultSize.ts | 12 ++++--- src/processes/adapter/update.ts | 23 ++++++------ tests/buffer.spec.ts | 2 +- tests/cache.spec.ts | 13 +++---- 6 files changed, 80 insertions(+), 56 deletions(-) diff --git a/src/classes/buffer.ts b/src/classes/buffer.ts index def27c3e..ed9264fb 100644 --- a/src/classes/buffer.ts +++ b/src/classes/buffer.ts @@ -263,36 +263,44 @@ export class Buffer { generator: (index: number, data: Data) => Item, indexToTrack: number, fixRight: boolean - ): number { + ): { trackedIndex: number, toRemove: Item[] } { if (!this.size || Number.isNaN(this.firstIndex)) { - return NaN; + return { trackedIndex: NaN, toRemove: [] }; } - let _indexToTrack = indexToTrack; + let trackedIndex = indexToTrack; let index = fixRight ? this.lastIndex : this.firstIndex; const items: Item[] = []; const diff = fixRight ? -1 : 1; - const initialIndexList = this.items.map(({ $index }) => $index); - (fixRight ? this.items.reverse() : this.items).forEach(item => { + const limit = this.size - 1; + const beforeMap = new Map(); // need to persist original $indexes + const updateArray = Array.prototype[fixRight ? 'unshift' : 'push']; + + for (let i = fixRight ? limit : 0; fixRight ? i >= 0 : i <= limit; i += diff) { + const item = this.items[i]; + beforeMap.set(item.$index, item); const result = predicate(item); + // if predicate result is falsy or empty array -> delete if (!result || (Array.isArray(result) && !result.length)) { item.toRemove = true; - _indexToTrack += item.$index >= indexToTrack ? (fixRight ? 1 : 0) : (fixRight ? 0 : -1); + trackedIndex += item.$index >= indexToTrack ? (fixRight ? 1 : 0) : (fixRight ? 0 : -1); this.shiftExtremum(-1, fixRight); - return; + continue; } + // if predicate result is truthy but not array -> leave if (!Array.isArray(result)) { item.updateIndex(index); - items.push(item); + updateArray.call(items, item); index += diff; - return; + continue; } + // if predicate result is non-empty array -> insert/replace if (item.$index < indexToTrack) { - _indexToTrack += fixRight ? 0 : result.length - 1; + trackedIndex += fixRight ? 0 : result.length - 1; } else if (item.$index > indexToTrack) { - _indexToTrack += fixRight ? 1 - result.length : 0; + trackedIndex += fixRight ? 1 - result.length : 0; } let toRemove = true; const newItems: Item[] = []; @@ -300,7 +308,7 @@ export class Buffer { let newItem: Item; if (item.data === data) { if (indexToTrack === item.$index) { - _indexToTrack = index + i * diff; + trackedIndex = index + i * diff; } item.updateIndex(index + i * diff); newItem = item; @@ -309,26 +317,31 @@ export class Buffer { newItem = generator(index + i * diff, data); newItem.toInsert = true; } - newItems.push(newItem); + updateArray.call(newItems, newItem); }); item.toRemove = toRemove; - items.push(...newItems); + updateArray.call(items, ...newItems); index += diff * result.length; if (result.length > 1) { this.shiftExtremum(result.length - 1, fixRight); } - }); - this.items = fixRight ? items.reverse() : items; - this.cache.updateSubset(initialIndexList, this.items, fixRight); + } + + const toRemove = this.items.filter(item => item.toRemove); + const itemsBefore = Array.from(beforeMap) + .map(([$index, { size, toRemove }]) => ({ $index, size, toRemove })) + .sort((a, b) => a.$index - b.$index); + this.items = items; + this.cache.updateSubset(itemsBefore, items, fixRight); if (this.finiteAbsMinIndex === this.finiteAbsMaxIndex) { - _indexToTrack = NaN; - } else if (_indexToTrack > this.finiteAbsMaxIndex) { - _indexToTrack = this.finiteAbsMaxIndex; - } else if (_indexToTrack < this.finiteAbsMinIndex) { - _indexToTrack = this.finiteAbsMinIndex; + trackedIndex = NaN; + } else if (trackedIndex > this.finiteAbsMaxIndex) { + trackedIndex = this.finiteAbsMaxIndex; + } else if (trackedIndex < this.finiteAbsMinIndex) { + trackedIndex = this.finiteAbsMinIndex; } - return _indexToTrack; + return { trackedIndex, toRemove }; } cacheItem(item: Item): void { diff --git a/src/classes/buffer/cache.ts b/src/classes/buffer/cache.ts index 651a63fc..9e159e68 100644 --- a/src/classes/buffer/cache.ts +++ b/src/classes/buffer/cache.ts @@ -4,6 +4,12 @@ import { Settings } from '../settings'; import { Logger } from '../logger'; import { SizeStrategy } from '../../inputs/index'; +interface ItemUpdate { + $index: number; + size: number; + toRemove?: boolean; +} + export class ItemCache { $index: number; nodeId: string; @@ -155,19 +161,20 @@ export class Cache { * Destructively updates Set based on subset (before-after) changes. * Replaces current Set with a new one with new regular $indexes. * Maintains min/max indexes. Maintains default item size on remove only. - * Inserted and replaced items will be taken into account on Cache.add async calls after render. * - * @param {number[]} before Initial subset of indexes to be replaced by "after". Must be incremental. - * @param {Item[]} after Transformed subset that replaces "before". Must be be $index-incremental. + * @param {ItemUpdate[]} before Initial subset of items to be replaced by "after". + * Each element is an object with { $index, size, toRemove } props. Must be $index-incremental. + * Items to be removed must have toRemove flag: before[].toRemove = true. + * @param {Item[]} after Transformed subset that replaces "before". Must be $index-incremental. * Must contain at least 1 $index from "before" or be empty. * @param {boolean} fixRight This is to fix right indexes during subset collapsing. Acts only if "after" is empty. */ - updateSubset(before: number[], after: Item[], fixRight?: boolean): void { + updateSubset(before: ItemUpdate[], after: Item[], fixRight?: boolean): void { if (!this.size || !before.length) { return; } - const minB = before[0], maxB = before[before.length - 1]; - let leftDiff: number, rightDiff: number, found; + const minB = before[0].$index, maxB = before[before.length - 1].$index; + let leftDiff: number, rightDiff: number; if (after.length) { const minA = after[0].$index, maxA = after[after.length - 1].$index; leftDiff = minA - minB; @@ -191,11 +198,9 @@ export class Cache { after.forEach(item => // subset items items.set(item.$index, new ItemCache(item, this.saveData)) ); - before.forEach(index => { // removed items immediately affect the default size - if (!after.some(({ $index }) => index === $index) && (found = this.get(index))) { - this.defaultSize.setRemoved(found); - } - }); + before // to maintain default size on remove + .filter(item => item.toRemove) + .forEach(item => this.defaultSize.setRemoved(item)); this.minIndex += leftDiff; this.maxIndex += rightDiff; this.items = items; diff --git a/src/classes/buffer/defaultSize.ts b/src/classes/buffer/defaultSize.ts index bcb747d8..7f7c9300 100644 --- a/src/classes/buffer/defaultSize.ts +++ b/src/classes/buffer/defaultSize.ts @@ -114,12 +114,16 @@ export class DefaultSize { return false; } const oldValue = this.get(); - if (this.sizeStrategy === SizeStrategy.Average) { - this.recalculateAverageSize(cacheSize); + if (!cacheSize) { + this.reset(true); } else { - this.recalculateFrequentSize(); + if (this.sizeStrategy === SizeStrategy.Average) { + this.recalculateAverageSize(cacheSize); + } else { + this.recalculateFrequentSize(); + } + this.recalculation.reset(); } - this.recalculation.reset(); return this.get() !== oldValue; } diff --git a/src/processes/adapter/update.ts b/src/processes/adapter/update.ts index c4cfb472..b6fa7035 100644 --- a/src/processes/adapter/update.ts +++ b/src/processes/adapter/update.ts @@ -26,11 +26,10 @@ export default class Update extends BaseAdapterProcessFactory(AdapterProcess.upd logger.log(() => 'no items in Buffer'); return false; } - const before = [...buffer.items]; const { item: firstItem, index: firstIndex, diff: firstItemDiff } = viewport.getEdgeVisibleItem(buffer.items, Direction.backward); - const trackedIndex = buffer.updateItems( + const { trackedIndex, toRemove } = buffer.updateItems( params.predicate, (index, data) => new Item(index, data, routines), firstIndex, @@ -43,21 +42,23 @@ export default class Update extends BaseAdapterProcessFactory(AdapterProcess.upd delta = - buffer.getSizeByIndex(trackedIndex) + firstItemDiff; } - const itemsToRemove = before.filter(({ toRemove }) => toRemove); - itemsToRemove.forEach(item => item.hide()); - logger.log(() => itemsToRemove.length - ? 'items to remove: [' + itemsToRemove.map(({ $index }) => $index).join(',') + ']' + toRemove.forEach(item => item.hide()); + logger.log(() => toRemove.length + ? 'items to remove: [' + toRemove.map(({ $index }) => $index).join(',') + ']' : 'no items to remove' ); + if (toRemove.length) { // insertions will be processed on render + buffer.checkDefaultSize(); + } - const itemsToRender = buffer.items.filter(({ toInsert }) => toInsert); - logger.log(() => itemsToRender.length - ? 'items to render: [' + itemsToRender.map(({ $index }) => $index).join(',') + ']' + const toRender = buffer.items.filter(({ toInsert }) => toInsert); + logger.log(() => toRender.length + ? 'items to render: [' + toRender.map(({ $index }) => $index).join(',') + ']' : 'no items to render' ); - fetch.update(trackedIndex, delta, itemsToRender, itemsToRemove); - return !!itemsToRemove.length || !!itemsToRender.length; + fetch.update(trackedIndex, delta, toRender, toRemove); + return !!toRemove.length || !!toRender.length; } } diff --git a/tests/buffer.spec.ts b/tests/buffer.spec.ts index 6d992360..6f0decf6 100644 --- a/tests/buffer.spec.ts +++ b/tests/buffer.spec.ts @@ -56,7 +56,7 @@ const checkIndexTrackingOnUpdate = (params: BufferUpdateTrackConfig) => () => { const absMinBefore = debug && buffer.finiteAbsMinIndex, absMaxBefore = debug && buffer.finiteAbsMaxIndex; const minBefore = debug && buffer.minIndex, maxBefore = debug && buffer.maxIndex; - const trackedIndex = buffer.updateItems(predicate, cb, index, fixRight); + const { trackedIndex } = buffer.updateItems(predicate, cb, index, fixRight); if (debug) { const trackedItem = buffer.get(trackedIndex); diff --git a/tests/cache.spec.ts b/tests/cache.spec.ts index 7cdd816c..72301918 100644 --- a/tests/cache.spec.ts +++ b/tests/cache.spec.ts @@ -99,7 +99,7 @@ describe('Cache Spec', () => { describe('Update', () => { const MIN = 1, COUNT = 7; const items = generateBufferItems(MIN, COUNT); - const subset = items.slice(2, 5).map(({ $index }) => $index); // [3, 4, 5] + const subset = items.slice(2, 5); // [3, 4, 5] let cache: Cache; const checkUpdate = (list: IndexIdList) => { @@ -248,7 +248,10 @@ describe('Cache Spec', () => { } const cache = new Cache(averageSizeSettings as never, loggerMock as never); const items = generateBufferItems(1, length); - items.forEach(item => cache.add(item)); + items.forEach(item => { + item.toRemove = toRemove && toRemove.includes(item.$index); + cache.add(item); + }); cache.recalculateDefaultSize(); return { items, cache, average: { after, before } }; }; @@ -299,7 +302,7 @@ describe('Cache Spec', () => { const toRemove = [5, 8, 10]; const { items, cache, average } = prepareAverage({ length: 10, toRemove }); const listAfter = [{ 1: 1 }, { 2: 2 }, { 3: 3 }, { 4: 4 }, { 5: 6 }, { 6: 7 }, { 7: 9 }]; - const subset = items.slice(toRemove[0], toRemove[toRemove.length - 1] + 1).map(({ $index }) => $index); + const subset = items.slice(toRemove[0], toRemove[toRemove.length - 1] + 1); expect(cache.getDefaultSize()).toBe(average.before); @@ -308,7 +311,6 @@ describe('Cache Spec', () => { expect(cache.getDefaultSize()).toBe(average.after); }); - it('should maintain average size on remove via update (explicit)', () => { const INDEX_TO_REMOVE = 10; const cache = new Cache(averageSizeSettings as never, loggerMock as never); @@ -321,8 +323,7 @@ describe('Cache Spec', () => { cache.recalculateDefaultSize(); expect(cache.getDefaultSize()).toBe(25); - const before = items.filter(({ $index }) => [10, 11, 12, 13, 14, 15].includes($index)) - .map(({ $index }) => $index); + const before = items.filter(({ $index }) => [10, 11, 12, 13, 14, 15].includes($index)); const after = make([{ 10: 11 }, { 11: 12 }, { 12: 13 }, { 13: 14 }, { 14: 15 }]); after.forEach(item => item.size = item.data.size = 20);