perf: round 18 — calendar-event in-place iCal edit, ResourceList incremental id-index
Two items from the ROUND17 deferred list, each benchmark-gated with a
BEFORE/AFTER equivalence gate and a rollback-on-regression check (ROUND2–17
discipline). See benches/ROUND18.md.
[C1] backend — CalendarEvent::update_ical_property / remove_ical_property
rewrote the ENTIRE ical_data body with format!("{}{}{}") on every call and
allocated two search needles per call. calendar_storage_adapter::update_event
fans a multi-field edit out into one call per changed field, so a full REST
edit paid one full-body (up to ~11 KB) allocation per property. The body is
now mutated in place (replace_range for an existing property, four insert/
insert_str for a new one, byte-identical spans) and the single "\nNAME:"
needle is built on the stack (the "\r\nNAME:" needle was redundant — the LF
form is its suffix). bench_round18_micro [C1]: 70 -> 2 allocs/op (68 fewer),
2.46x wall, emitted body byte-identical.
[F1] frontend — ResourceList itemIndexById rebuilt a fresh Map over the whole
accumulated list every infinite-scroll page (O(N)/page, O(N^2) drain) and,
being a new instance each page, re-fired the reap-stale effect (another O(N)
id Set/page). New ItemIndexBuilder extends a persistent Map with the fresh
page only and reuses the reference across appends; the reap-stale effect now
tests membership against it. round18.bench.test.ts [F1]: 40x50 drain 74.1 ->
6.4 ms (11.5x), deep-equal to the reference at every page, reference-contract
gate (same-ref append / new-ref rebuild).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FoxFtikahM1N4PE5s3ZVH3
This commit is contained in:
@@ -79,6 +79,7 @@
|
||||
import { formatDate, iconNameFromClass, fileIconKindClass } from '$lib/utils/display';
|
||||
import { gridColumns } from '$lib/utils/grid';
|
||||
import { ResourceSectionsBuilder } from '$lib/utils/resourceSections';
|
||||
import { ItemIndexBuilder } from '$lib/utils/itemIndex';
|
||||
import { fileThumbnailUrl, thumbSizeForView } from '$lib/api/endpoints/files';
|
||||
import {
|
||||
canThumbnailClientSide,
|
||||
@@ -468,13 +469,18 @@
|
||||
// stale-selection cleanup only fires when items truly leave the
|
||||
// dataset (reload, delete, etc.), not when the filter hides them.
|
||||
//
|
||||
// Index rebuilt only when `items` changes; the projection is then
|
||||
// O(k · log k) in the selection size k instead of a full O(N) re-scan
|
||||
// of the list on every toggle (O(N²)-ish across a shift-range gesture
|
||||
// once the batch toolbar is mounted — benches/ROUND11.md §S1). The
|
||||
// index sort preserves item order, so the toolbar sees the same array
|
||||
// the old filter produced.
|
||||
const itemIndexById = $derived(new Map(items.map((i, idx) => [i.id, idx])));
|
||||
// Index extended over the freshly-appended page only (never re-scanned in
|
||||
// full) via `ItemIndexBuilder`: an infinite-scroll drain with a selection
|
||||
// active collapses from Σ O(N²) Map rebuilds to O(N) total, and the Map
|
||||
// reference is reused across appends so the reap-stale effect below no
|
||||
// longer re-fires (nor re-allocates an O(N) id Set) on a page that removed
|
||||
// nothing — its reference only changes on a rebuild (reload / deletion),
|
||||
// exactly when a reap is warranted. The projection is then O(k · log k) in
|
||||
// the selection size k, not a full O(N) re-scan on every toggle
|
||||
// (benches/ROUND11.md §S1, benches/ROUND18.md §F1). The index sort preserves
|
||||
// item order, so the toolbar sees the same array the old filter produced.
|
||||
const itemIndex = new ItemIndexBuilder<FileItem | FolderItem>();
|
||||
const itemIndexById = $derived(itemIndex.sync(items));
|
||||
const selectedItems = $derived.by(() => {
|
||||
const picked: { idx: number; item: FileItem | FolderItem }[] = [];
|
||||
for (const id of selected) {
|
||||
@@ -487,15 +493,20 @@
|
||||
|
||||
// Drop selection ids that are no longer present after a reload.
|
||||
$effect(() => {
|
||||
// With nothing selected (the common case) every infinite-scroll page
|
||||
// re-fired this effect and built a throwaway O(N) id Set for a loop
|
||||
// that never runs — skip straight out. `selected.size` is reactive,
|
||||
// so the effect re-fires when a selection appears.
|
||||
// With nothing selected (the common case) the loop never runs — skip
|
||||
// straight out. `selected.size` is reactive, so the effect re-fires
|
||||
// when a selection appears.
|
||||
if (selected.size === 0) return;
|
||||
const ids = new Set(items.map((i) => i.id));
|
||||
// Test membership against the incremental `itemIndexById` rather than a
|
||||
// throwaway O(N) id Set rebuilt per page. Its reference is stable across
|
||||
// infinite-scroll appends (which never remove an id — nothing to reap)
|
||||
// so this effect no longer re-fires on every page; the reference changes
|
||||
// only on a rebuild (reload / deletion), which is exactly when a stale
|
||||
// selection must be dropped (benches/ROUND18.md §F1).
|
||||
const index = itemIndexById;
|
||||
let changed = false;
|
||||
for (const id of selected) {
|
||||
if (!ids.has(id)) {
|
||||
if (!index.has(id)) {
|
||||
selected.delete(id);
|
||||
changed = true;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
// Round-18 frontend micro-pack (benches/ROUND18.md §F1).
|
||||
//
|
||||
// Each section is BEFORE (verbatim replica of the shipped-before shape) vs
|
||||
// AFTER (the shipped incremental builder), with an equivalence gate, a
|
||||
// reference-contract gate, and a wall-time perf gate — the same discipline as
|
||||
// the Rust micro-packs: an AFTER that doesn't beat its BEFORE fails the gate.
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { buildItemIndex, ItemIndexBuilder } from '$lib/utils/itemIndex';
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
// [F1] ResourceList `itemIndexById` — rebuild-a-fresh-Map-per-page vs incremental
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
//
|
||||
// Audit finding (ROUND17 deferred list): ResourceList derived
|
||||
// `itemIndexById = new Map(items.map((i, idx) => [i.id, idx]))`. Every
|
||||
// infinite-scroll page (`items = [...items, ...page]`) rebuilt a brand-new Map
|
||||
// over the WHOLE accumulated list — O(N) per page, Σ O(N²) across a P-page
|
||||
// drain — and, being a fresh instance each page, re-fired the reap-stale
|
||||
// `$effect` that reference-diffs it (allocating another O(N) id Set for a reap
|
||||
// an append can never trigger). `ItemIndexBuilder` extends the persistent Map
|
||||
// with the fresh page only and returns the same reference on an append.
|
||||
|
||||
interface Item {
|
||||
id: string;
|
||||
}
|
||||
|
||||
/** A page of `{ id }` items (50/page, the default page size). */
|
||||
function pageOf(start: number, n: number): Item[] {
|
||||
return Array.from({ length: n }, (_, i) => ({ id: `it-${start + i}` }));
|
||||
}
|
||||
|
||||
/** BEFORE: rebuild a fresh Map over the whole accumulated list each page. */
|
||||
function rebuildPerPage(pages: Item[][]): Map<string, number> {
|
||||
let acc: Item[] = [];
|
||||
let index = new Map<string, number>();
|
||||
for (const page of pages) {
|
||||
acc = [...acc, ...page]; // the component's `items = [...items, ...page]`
|
||||
index = new Map(acc.map((i, idx) => [i.id, idx])); // new instance + O(N) rebuild
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
/** AFTER: one persistent builder, extend with only the fresh page's ids. */
|
||||
function incrementalPerPage(pages: Item[][]): Map<string, number> {
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
let acc: Item[] = [];
|
||||
let index = new Map<string, number>();
|
||||
for (const page of pages) {
|
||||
acc = [...acc, ...page];
|
||||
index = builder.sync(acc);
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
describe('round18 §F1 — ResourceList itemIndexById incremental Map', () => {
|
||||
it('final index is identical to the full rebuild (equivalence gate)', () => {
|
||||
const pages = Array.from({ length: 20 }, (_, p) => pageOf(p * 50, 50));
|
||||
const acc = pages.flat();
|
||||
const before = rebuildPerPage(pages);
|
||||
const after = incrementalPerPage(pages);
|
||||
const reference = buildItemIndex(acc);
|
||||
expect(after.size).toBe(before.size);
|
||||
for (const [id, idx] of reference) expect(after.get(id)).toBe(idx);
|
||||
for (const [id, idx] of after) expect(before.get(id)).toBe(idx);
|
||||
});
|
||||
|
||||
it('the index stays deep-equal to the reference at EVERY page (equivalence gate)', () => {
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
let acc: Item[] = [];
|
||||
for (let p = 0; p < 12; p++) {
|
||||
acc = [...acc, ...pageOf(p * 50, 50)];
|
||||
const got = builder.sync(acc);
|
||||
const want = buildItemIndex(acc);
|
||||
expect(got.size).toBe(want.size);
|
||||
for (const [id, idx] of want) expect(got.get(id)).toBe(idx);
|
||||
}
|
||||
});
|
||||
|
||||
it('a later duplicate id resolves to its highest index, matching Map (equivalence gate)', () => {
|
||||
// The old `new Map(items.map(...))` keeps the last (highest-index)
|
||||
// occurrence of a duplicate id; the incremental extend must too.
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
const dup: Item = { id: 'dup' };
|
||||
const p1 = [dup, { id: 'a' }];
|
||||
const p2 = [{ id: 'b' }, dup]; // 'dup' re-appears at index 3
|
||||
builder.sync(p1);
|
||||
const got = builder.sync([...p1, ...p2]);
|
||||
const want = buildItemIndex([...p1, ...p2]);
|
||||
expect(got.get('dup')).toBe(want.get('dup'));
|
||||
expect(got.get('dup')).toBe(3);
|
||||
});
|
||||
|
||||
it('reuses the Map reference on append, mints a new one on rebuild (reference-contract gate)', () => {
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
const p1 = pageOf(0, 50);
|
||||
const first = builder.sync(p1);
|
||||
// Append: same reference (so the reap-stale effect does NOT re-fire —
|
||||
// an append removes nothing).
|
||||
const appended = builder.sync([...p1, ...pageOf(50, 50)]);
|
||||
expect(appended).toBe(first);
|
||||
// Deletion (shorter, non-append prefix): fresh reference (so the
|
||||
// reap-stale effect DOES re-fire and drops the removed id).
|
||||
const afterDelete = builder.sync(p1.slice(0, 40));
|
||||
expect(afterDelete).not.toBe(first);
|
||||
expect(afterDelete.has('it-49')).toBe(false);
|
||||
// Reload with a different first element (non-append): fresh reference.
|
||||
const reloaded = builder.sync(pageOf(1000, 50));
|
||||
expect(reloaded).not.toBe(afterDelete);
|
||||
});
|
||||
|
||||
it('a P-page drain builds the index ≥5x faster incrementally (perf gate)', () => {
|
||||
const PAGES = 40;
|
||||
const PER = 50; // 2 000 items total
|
||||
const pages = Array.from({ length: PAGES }, (_, p) => pageOf(p * PER, PER));
|
||||
|
||||
const run = (f: (p: Item[][]) => Map<string, number>): number => {
|
||||
const t0 = performance.now();
|
||||
for (let r = 0; r < 20; r++) f(pages);
|
||||
return performance.now() - t0;
|
||||
};
|
||||
|
||||
// Warm-up (JIT) then measure.
|
||||
run(rebuildPerPage);
|
||||
run(incrementalPerPage);
|
||||
const beforeMs = run(rebuildPerPage);
|
||||
const afterMs = run(incrementalPerPage);
|
||||
|
||||
console.info(
|
||||
`§F1 ${PAGES} pages × ${PER}: rebuild-per-page ${beforeMs.toFixed(1)} ms vs incremental ${afterMs.toFixed(1)} ms (${(beforeMs / afterMs).toFixed(1)}x)`
|
||||
);
|
||||
expect(afterMs).toBeLessThan(beforeMs / 5);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,83 @@
|
||||
/**
|
||||
* Incremental `id → position` index for `ResourceList`, extracted so the O(N²)
|
||||
* accumulation of its `itemIndexById` `$derived` (and the reap-stale effect's
|
||||
* per-page `new Set(items.map(…))`) can be replaced with an append-aware
|
||||
* builder — and unit/benchmark-tested off the Svelte reactive graph.
|
||||
*
|
||||
* `ResourceList` pages its list in via infinite scroll (`items = [...items,
|
||||
* ...page]`) and rebuilt `new Map(items.map((i, idx) => [i.id, idx]))` on every
|
||||
* page — O(N) per page, Σ ≈ O(N²) across a P-page drain, and a fresh Map each
|
||||
* page (so the reap-stale effect that reference-diffs it re-ran on every append
|
||||
* too, allocating another O(N) id Set for a reap that an append can never
|
||||
* trigger). This is the same class ROUND6 fixed for the files listing, ROUND14
|
||||
* §F2 for favorites, and ROUND15/16 for the grouped/shared lanes.
|
||||
*
|
||||
* Because a fresh page only ever *appends* (server order is stable; existing
|
||||
* rows keep their index), {@link ItemIndexBuilder} extends the persistent Map
|
||||
* with just the new tail on an append and returns the SAME Map reference; any
|
||||
* other change (reload, deletion, non-append) rebuilds into a NEW Map. That
|
||||
* reference contract is load-bearing for the two `ResourceList` consumers:
|
||||
*
|
||||
* - `selectedItems` re-derives on every `items` change regardless (it indexes
|
||||
* `items[idx]`), so it always reads the freshly-extended Map — a stable ref
|
||||
* on append costs it nothing.
|
||||
* - the reap-stale `$effect` reference-diffs the Map, so a stable ref on
|
||||
* append means it does NOT re-run there (an append never removes an id, so
|
||||
* there is nothing to reap), while a rebuild (delete / reload) yields a new
|
||||
* ref and DOES re-run it — exactly when stale selections must be dropped.
|
||||
*
|
||||
* The pure {@link buildItemIndex} is the verbatim reference (what the old
|
||||
* `itemIndexById` derive produced); the benchmark gate holds the builder equal
|
||||
* to it at every page.
|
||||
*/
|
||||
|
||||
import { isAppendExtension } from './appendExtension';
|
||||
|
||||
/** Minimal shape the index needs: a stable string `id`. */
|
||||
export interface HasId {
|
||||
id: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verbatim reference: the `Map<id, index>` the old `itemIndexById` `$derived`
|
||||
* produced — `new Map(items.map((i, idx) => [i.id, idx]))`. On a duplicate id
|
||||
* the highest index wins (last insertion), matching `Map`'s own semantics.
|
||||
*/
|
||||
export function buildItemIndex<T extends HasId>(items: readonly T[]): Map<string, number> {
|
||||
const index = new Map<string, number>();
|
||||
for (let i = 0; i < items.length; i++) index.set(items[i].id, i);
|
||||
return index;
|
||||
}
|
||||
|
||||
/**
|
||||
* Append-aware `id → index` builder. Call {@link sync} with the current item
|
||||
* list on every change; it detects the common case — the list grew by appending
|
||||
* a page — and indexes only the fresh tail, reusing the persistent Map (same
|
||||
* reference). Any other change rebuilds into a new Map, so the result is always
|
||||
* deep-equal to {@link buildItemIndex} and the reference changes exactly when a
|
||||
* reap-stale pass is warranted.
|
||||
*/
|
||||
export class ItemIndexBuilder<T extends HasId> {
|
||||
/** Last synced list — the append cursor and the append-detection baseline. */
|
||||
#items: readonly T[] = [];
|
||||
/** id → index; a stable reference across appends, a fresh one on rebuild. */
|
||||
#index = new Map<string, number>();
|
||||
|
||||
sync(items: readonly T[]): Map<string, number> {
|
||||
if (isAppendExtension(this.#items, items)) {
|
||||
// Append: the prefix is unchanged (existing ids keep their index), so
|
||||
// only the fresh tail needs indexing. A duplicate id in the tail
|
||||
// overwrites to its higher index — identical to the full rebuild's
|
||||
// last-wins. Same Map reference is returned (see the class doc).
|
||||
for (let i = this.#items.length; i < items.length; i++) {
|
||||
this.#index.set(items[i].id, i);
|
||||
}
|
||||
} else {
|
||||
// Reload / deletion / non-append / first run: rebuild into a NEW Map so
|
||||
// the reap-stale effect (which reference-diffs it) re-runs.
|
||||
this.#index = buildItemIndex(items);
|
||||
}
|
||||
this.#items = items;
|
||||
return this.#index;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user