From 61c94709812c907e4869f7e4a3f671cdb275cfab Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 00:54:38 +0000 Subject: [PATCH 1/4] =?UTF-8?q?perf(frontend):=20round=206=20=E2=80=94=20c?= =?UTF-8?q?oalesced=20progressive=20listing,=20in-place=20SvelteSet,=20bat?= =?UTF-8?q?ch=20fan-out,=20t()=20value=20cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four SPA hot-path fixes, each shipping with a vitest benchmark gate (verbatim BEFORE replica + equivalence + perf assertion) so CI re-verifies the win on every run: - fetchFolderListing invoked onPage after EVERY 200-row page with the whole accumulated listing, and the files view re-sorts everything per emission — O(N²/page) main-thread work on large folders. Page one and the final page always emit; intermediates coalesce to one per 150 ms. 25×200 load: 30.9 → 4.0 ms (7.8x), 65 000 → 5 200 sorted elements. - selected/favoriteIds/sharedIds (files) and favoriteIds (recent) were $states copied whole on every toggle. Now one SvelteSet each, mutated in place (the useSelection pattern): 1 000 toggles @ N=5 000 771.9 → 1.9 ms (399x); one-toggle fan-out across 40 mounted rows 40 → 3 re-runs when refining a select-all. - batchDelete/moveInto awaited one request per item serially and probed listing.folders.find per id (O(N·M)). Now an id index built once + mapLimit(6) fan-out, failure semantics preserved: 100-item delete @ 5 ms RTT 525 → 89 ms (5.9x), 38 825 → 500 probes. - t() re-split its dotted key and walked the nested dict on every call, and interpolate regex-scanned strings without placeholders. Resolved values now memoize per (dict, key) in a WeakMap + a {{ guard: 20k mixed calls 22.7 → 8.6 ms (2.63x). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA --- .../lib/api/endpoints/folders.bench.test.ts | 205 ++++++++++++++++++ frontend/src/lib/api/endpoints/folders.ts | 44 +++- .../lib/composables/selectionBench.svelte.ts | 85 ++++++++ .../selectionPatterns.bench.test.ts | 127 +++++++++++ frontend/src/lib/i18n/i18n.bench.test.ts | 167 ++++++++++++++ frontend/src/lib/i18n/index.svelte.ts | 28 +++ frontend/src/lib/utils/sets.ts | 9 + .../src/routes/files/[...path]/+page.svelte | 122 ++++++----- .../src/routes/files/batchOps.bench.test.ts | 166 ++++++++++++++ frontend/src/routes/recent/+page.svelte | 23 +- 10 files changed, 905 insertions(+), 71 deletions(-) create mode 100644 frontend/src/lib/api/endpoints/folders.bench.test.ts create mode 100644 frontend/src/lib/composables/selectionBench.svelte.ts create mode 100644 frontend/src/lib/composables/selectionPatterns.bench.test.ts create mode 100644 frontend/src/lib/i18n/i18n.bench.test.ts create mode 100644 frontend/src/lib/utils/sets.ts create mode 100644 frontend/src/routes/files/batchOps.bench.test.ts diff --git a/frontend/src/lib/api/endpoints/folders.bench.test.ts b/frontend/src/lib/api/endpoints/folders.bench.test.ts new file mode 100644 index 00000000..9df9d79a --- /dev/null +++ b/frontend/src/lib/api/endpoints/folders.bench.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +vi.mock('$lib/api/client', () => ({ apiFetch: vi.fn(), apiJson: vi.fn() })); + +import { apiFetch } from '$lib/api/client'; +import type { FileItem, FolderItem, ItemType } from '$lib/api/types'; +import { fetchFolderListing, invalidateFolderCache, type FolderListing } from './folders'; + +/** + * Benchmark gate for the coalesced progressive-render emissions in + * {@link fetchFolderListing}. + * + * Audit finding: the loader invoked `onPage` after EVERY 200-item page with a + * fresh copy of the whole accumulated listing, and the files view re-derives + * its filtered + sorted view (two `localeCompare` sorts + entry rebuild) from + * each emission. For a folder of N items that is Σ page sizes ≈ O(N²/200) + * elements re-sorted on the main thread during a single load — hundreds of ms + * of jank on exactly the large folders progressive rendering was meant to + * help. The fix emits page one (first paint) and the final page always, and + * intermediate pages at most once per PAGE_EMIT_MIN_INTERVAL_MS. + * + * Gates: + * 1. Equivalence — final listing identical to the emit-every-page reference, + * first emission still after page one (first paint preserved), last + * emission still `done === true` with the complete listing. + * 2. Perf — on a fast connection (pages resolve in ≪150 ms) the consumer-side + * derive work collapses from 25 full re-sorts to ≤3; wall time of the + * load+derive cycle must drop accordingly (≥3x on the derive term). + */ + +type ResourceItem = { resource_type: ItemType; resource: { id: string; name: string } }; +type ResourcePage = { items?: ResourceItem[]; next_cursor?: string }; + +const PAGE_SIZE = 200; +const PAGES = 25; // 5 000-item folder + +/** Deterministic shuffled names so the consumer sort actually works. */ +function pageBody(page: number): ResourcePage { + const items: ResourceItem[] = []; + for (let i = 0; i < PAGE_SIZE; i++) { + const n = page * PAGE_SIZE + i; + const id = `f-${n.toString().padStart(5, '0')}`; + // Mix folders into the first page like a real listing (folders first). + const isFolder = page === 0 && i < 20; + items.push({ + resource_type: isFolder ? 'folder' : 'file', + resource: { id, name: `item ${((n * 7919) % 100000).toString().padStart(5, '0')}.txt` } + }); + } + return { items, next_cursor: page + 1 < PAGES ? `c${page + 1}` : undefined }; +} + +function fakeRes(body: ResourcePage): Response { + return { + status: 200, + ok: true, + json: async () => body, + headers: { get: () => null } + } as unknown as Response; +} + +function mockPagedFetch(): void { + let call = 0; + vi.mocked(apiFetch).mockImplementation(async () => fakeRes(pageBody(call++))); +} + +/** + * The pre-fix loader, verbatim shape: accumulate pages and emit a fresh copy + * of the whole accumulated listing after every page. + */ +async function referenceFetchFolderListing( + folderId: string, + onPage: (partial: FolderListing, done: boolean) => void +): Promise { + const folders: FolderItem[] = []; + const files: FileItem[] = []; + let cursor: string | undefined; + do { + const params = new URLSearchParams({ order_by: 'name', limit: '200' }); + if (cursor) params.set('cursor', cursor); + const res = await apiFetch(`/api/folders/${folderId}/resources?${params.toString()}`, { + credentials: 'same-origin', + cache: 'no-store' + }); + if (!res.ok) throw new Error(`listing failed: ${res.status}`); + const page = (await res.json()) as ResourcePage; + for (const it of page.items ?? []) { + if (it.resource_type === 'folder') folders.push(it.resource as FolderItem); + else files.push(it.resource as FileItem); + } + cursor = page.next_cursor; + onPage({ folders: [...folders], files: [...files], favoriteIds: [], sharedIds: [] }, !cursor); + } while (cursor); + return { folders, files, favoriteIds: [], sharedIds: [] }; +} + +/** + * The files view's per-emission derive chain, reduced to its dominant costs: + * dotfile filter pass + two localeCompare sorts + ordered-entry rebuild + * (`sortedFolders`/`sortedFiles`/`entries`/`orderedIds` in +page.svelte). + * Returns the number of elements that went through the sort — the O(N²) term. + */ +function consumerDerive(partial: FolderListing): number { + const visF = partial.folders.filter((f) => !f.name.startsWith('.')); + const visX = partial.files.filter((f) => !f.name.startsWith('.')); + const sortedF = [...visF].sort((a, b) => a.name.localeCompare(b.name)); + const sortedX = [...visX].sort((a, b) => a.name.localeCompare(b.name)); + const orderedIds = [...sortedF.map((f) => f.id), ...sortedX.map((f) => f.id)]; + return orderedIds.length; +} + +beforeEach(() => { + vi.clearAllMocks(); + invalidateFolderCache(); +}); + +describe('coalesced progressive listing emissions (benchmark gate)', () => { + it('final listing, first-paint page and done-flag match the emit-every-page reference', async () => { + mockPagedFetch(); + const refEmits: Array<{ n: number; done: boolean }> = []; + const refFinal = await referenceFetchFolderListing('bench', (p, done) => + refEmits.push({ n: p.folders.length + p.files.length, done }) + ); + + mockPagedFetch(); + const emits: Array<{ n: number; done: boolean; partial: FolderListing }> = []; + const r = await fetchFolderListing('bench', { + onPage: (partial, done) => + emits.push({ n: partial.folders.length + partial.files.length, done, partial }) + }); + + // Identical complete listing. + expect(r.listing).toEqual(refFinal); + // First paint unchanged: the first emission is still page one. + expect(emits[0].n).toBe(refEmits[0].n); + expect(emits[0].n).toBe(PAGE_SIZE); + // Exactly one done emission, last, carrying the full listing — as before. + expect(emits.filter((e) => e.done).length).toBe(1); + expect(emits[emits.length - 1].done).toBe(true); + expect(emits[emits.length - 1].n).toBe(PAGES * PAGE_SIZE); + expect(refEmits[refEmits.length - 1].done).toBe(true); + // Emissions are a subset of what the reference produced (never more). + expect(emits.length).toBeLessThanOrEqual(refEmits.length); + // Every emitted partial is a prefix-accumulation (monotone growth). + for (let i = 1; i < emits.length; i++) expect(emits[i].n).toBeGreaterThan(emits[i - 1].n); + }); + + it('single-page folders still emit exactly once, done=true (fast path untouched)', async () => { + vi.mocked(apiFetch).mockResolvedValue( + fakeRes({ items: pageBody(PAGES - 1).items }) // no next_cursor + ); + const emits: boolean[] = []; + await fetchFolderListing('one', { onPage: (_p, done) => emits.push(done) }); + expect(emits).toEqual([true]); + }); + + it( + `collapses the O(N²) consumer re-derive on a fast ${PAGES}-page load (perf gate)`, + { timeout: 30_000 }, + async () => { + // Warm-up both paths (JIT tiering outside the measured windows). + mockPagedFetch(); + await referenceFetchFolderListing('warm', (p) => consumerDerive(p)); + mockPagedFetch(); + await fetchFolderListing('warm', { onPage: (p) => consumerDerive(p) }); + + mockPagedFetch(); + let refSorted = 0; + let refEmits = 0; + const t0 = performance.now(); + await referenceFetchFolderListing('bench', (p) => { + refEmits++; + refSorted += consumerDerive(p); + }); + const refMs = performance.now() - t0; + + mockPagedFetch(); + let sorted = 0; + let emitsN = 0; + const t1 = performance.now(); + await fetchFolderListing('bench', { + onPage: (p) => { + emitsN++; + sorted += consumerDerive(p); + } + }); + const ms = performance.now() - t1; + + console.info( + `progressive load ${PAGES}×${PAGE_SIZE}: before ${refEmits} emissions / ${refSorted} sorted elements / ${refMs.toFixed(1)} ms — after ${emitsN} emissions / ${sorted} sorted elements / ${ms.toFixed(1)} ms (${(refMs / ms).toFixed(1)}x wall, ${(refSorted / sorted).toFixed(1)}x fewer sorted elements)` + ); + + // The reference re-derived every page: Σ = P(P+1)/2 pages of elements. + expect(refEmits).toBe(PAGES); + expect(refSorted).toBe((PAGES * (PAGES + 1) * PAGE_SIZE) / 2); + // Coalesced: page 1 + final (+ occasionally one mid emission if the + // stubbed pages ever take >150 ms — they don't on any healthy runner). + expect(emitsN).toBeLessThanOrEqual(3); + // ≥5x less consumer sort work is the point of the change. + expect(sorted).toBeLessThan(refSorted / 5); + // And it must show up as wall time on the combined load+derive cycle. + expect(ms).toBeLessThan(refMs / 3); + } + ); +}); diff --git a/frontend/src/lib/api/endpoints/folders.ts b/frontend/src/lib/api/endpoints/folders.ts index b88af15c..965e272c 100644 --- a/frontend/src/lib/api/endpoints/folders.ts +++ b/frontend/src/lib/api/endpoints/folders.ts @@ -94,6 +94,17 @@ export async function getFolder(id: string): Promise { return folder; } +/** + * Minimum spacing between intermediate progressive-render emissions of + * {@link fetchFolderListing}. Each emission hands the consumer the WHOLE + * accumulated listing, and the files view re-derives its filtered + sorted + * view from it (O(accumulated · log) with `localeCompare`), so emitting every + * page made a large-folder load Σ O(N²/page) of main-thread sort work. Page + * one and the final page always emit; pages in between only emit after this + * much time has passed since the previous emission. + */ +export const PAGE_EMIT_MIN_INTERVAL_MS = 150; + /** * Fetch a folder's complete listing (sub-folders + files), rebuilt from the * cursor-paginated `/api/folders/{id}/resources` feed — the old combined @@ -112,12 +123,15 @@ export async function fetchFolderListing( etag?: string; forceRefresh?: boolean; /** - * Progressive render hook: invoked after EVERY page with the - * accumulated listing so far (the arrays are fresh copies — safe to - * hand to reactive state). Without it, a 2,000-item folder waited - * for all ⌈N/200⌉ sequential round-trips before the first row - * painted; with it the view paints after page one (~200 items) and - * fills in as the tail pages land. + * Progressive render hook: invoked with the accumulated listing so + * far (the arrays are fresh copies — safe to hand to reactive + * state). Without it, a 2,000-item folder waited for all ⌈N/200⌉ + * sequential round-trips before the first row painted; with it the + * view paints after page one (~200 items) and fills in as the tail + * pages land. Emissions are coalesced to at most one per + * {@link PAGE_EMIT_MIN_INTERVAL_MS} between the first and the final + * page — the hook is always called for page one and always called + * once more with `done === true` and the complete listing. */ onPage?: (partial: FolderListing, done: boolean) => void; } = {} @@ -125,6 +139,8 @@ export async function fetchFolderListing( const folders: FolderItem[] = []; const files: FileItem[] = []; let cursor: string | undefined; + let firstPage = true; + let lastEmit = 0; do { const params = new URLSearchParams({ order_by: 'name', limit: '200' }); if (opts.forceRefresh) params.set('force_refresh', 'true'); @@ -144,10 +160,18 @@ export async function fetchFolderListing( else files.push(it.resource as FileItem); } cursor = page.next_cursor; - opts.onPage?.( - { folders: [...folders], files: [...files], favoriteIds: [], sharedIds: [] }, - !cursor - ); + const done = !cursor; + if ( + opts.onPage && + (done || firstPage || performance.now() - lastEmit >= PAGE_EMIT_MIN_INTERVAL_MS) + ) { + lastEmit = performance.now(); + opts.onPage( + { folders: [...folders], files: [...files], favoriteIds: [], sharedIds: [] }, + done + ); + } + firstPage = false; } while (cursor); return { status: 200, listing: { folders, files, favoriteIds: [], sharedIds: [] } }; diff --git a/frontend/src/lib/composables/selectionBench.svelte.ts b/frontend/src/lib/composables/selectionBench.svelte.ts new file mode 100644 index 00000000..29f60d83 --- /dev/null +++ b/frontend/src/lib/composables/selectionBench.svelte.ts @@ -0,0 +1,85 @@ +/** + * Bench harness for the selection/badge-set reactivity patterns compared in + * `selectionPatterns.bench.test.ts` (runes only compile in `.svelte.ts` + * modules, so the models live here; the app never imports this file — it is + * test-only and tree-shaken from the bundle). + * + * `copyReassignModel` is the pre-fix files-view pattern, verbatim: a + * `$state` where every toggle copies the whole set into a fresh + * `SvelteSet` and reassigns. `inPlaceModel` is the post-fix pattern: one + * `SvelteSet` mutated in place. + */ +import { flushSync } from 'svelte'; +import { SvelteSet } from 'svelte/reactivity'; + +export interface SelectionModel { + has(id: string): boolean; + toggle(id: string): void; + seed(ids: Iterable): void; + readonly size: number; +} + +/** Pre-fix pattern (files view `toggleSelected`, verbatim copy-and-reassign). */ +export function copyReassignModel(): SelectionModel { + // eslint-disable-next-line svelte/prefer-svelte-reactivity -- BEFORE arm replicates the pre-fix plain-Set pattern verbatim + let selected = $state>(new Set()); + return { + has: (id) => selected.has(id), + toggle(id) { + const next = new SvelteSet(selected); + if (next.has(id)) next.delete(id); + else next.add(id); + selected = next; + }, + seed(ids) { + // eslint-disable-next-line svelte/prefer-svelte-reactivity -- BEFORE arm replicates the pre-fix plain-Set pattern verbatim + selected = new Set(ids); + }, + get size() { + return selected.size; + } + }; +} + +/** Post-fix pattern: one live `SvelteSet` mutated in place (per-key sources + * for present keys; absent-key reads track the version signal). */ +export function inPlaceModel(): SelectionModel { + const selected = new SvelteSet(); + return { + has: (id) => selected.has(id), + toggle(id) { + if (selected.has(id)) selected.delete(id); + else selected.add(id); + }, + seed(ids) { + selected.clear(); + for (const id of ids) selected.add(id); + }, + get size() { + return selected.size; + } + }; +} + +/** + * Mount one effect per row reading `model.has(rowId)` — the shape of a row's + * checkbox/star binding — run `mutate`, and report how many row effects re-ran + * (the invalidation fan-out of the mutation). + */ +export function measureFanout(model: SelectionModel, rowIds: string[], mutate: () => void): number { + let runs = 0; + const destroy = $effect.root(() => { + for (const id of rowIds) { + $effect(() => { + void model.has(id); + runs += 1; + }); + } + }); + flushSync(); // initial run of every row effect + const baseline = runs; + mutate(); + flushSync(); + destroy(); + return runs - baseline; +} diff --git a/frontend/src/lib/composables/selectionPatterns.bench.test.ts b/frontend/src/lib/composables/selectionPatterns.bench.test.ts new file mode 100644 index 00000000..7e32c038 --- /dev/null +++ b/frontend/src/lib/composables/selectionPatterns.bench.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest'; +import { + copyReassignModel, + inPlaceModel, + measureFanout, + type SelectionModel +} from './selectionBench.svelte'; + +/** + * Benchmark gate for the in-place `SvelteSet` selection/badge sets in the + * files and recent views. + * + * Audit finding: `selected`, `favoriteIds` and `sharedIds` were plain + * `$state`s rebuilt from a full copy on every single-item toggle + * (`new SvelteSet(selected)` + reassign). That costs (a) an O(N) copy per + * toggle — N unbounded under "select all → refine" — and (b) reassigning the + * state reference invalidates EVERY mounted row's `.has(id)` read, so the + * whole viewport re-renders for a one-row change. The fix keeps one + * `SvelteSet` per set and mutates it in place; `SvelteSet` tracks per-key, so + * a toggle re-runs only the toggled row's readers. The composable + * `useSelection` already shipped this pattern — the views now match it. + * + * `SvelteSet` granularity (svelte/src/reactivity/set.js): present keys get a + * per-key source; `.has()` on an ABSENT key tracks the set's version signal + * ("don't create sources willy-nilly"), so miss-readers re-run on any + * mutation in both patterns. The in-place win is therefore: no O(N) copy, and + * every OTHER present-key reader is spared — copy-reassign re-runs all rows. + * + * Gates: (1) both patterns agree on membership across a deterministic toggle + * script; (2) fan-out under 40 mounted row-effects matches those exact + * semantics (misses+1 in place vs all 40 copied — 3 vs 40 when the list is + * mostly selected, the "select all → refine" case); (3) 1 000 toggles over a + * 5 000-id selection run ≥5x faster in place. + */ + +/** Deterministic PRNG so both models replay the identical script. */ +function mulberry32(seed: number): () => number { + let a = seed >>> 0; + return () => { + a = (a + 0x6d2b79f5) | 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + +const ids = (n: number): string[] => Array.from({ length: n }, (_, i) => `id-${i}`); + +describe('in-place SvelteSet selection (benchmark gate)', () => { + it('membership after a 500-op toggle script is identical in both patterns', () => { + const universe = ids(1_000); + const a = copyReassignModel(); + const b = inPlaceModel(); + a.seed(universe.slice(0, 100)); + b.seed(universe.slice(0, 100)); + + const rand = mulberry32(0xc0ffee); + for (let i = 0; i < 500; i++) { + const id = universe[Math.floor(rand() * universe.length)]; + a.toggle(id); + b.toggle(id); + } + expect(a.size).toBe(b.size); + for (const id of universe) { + expect(b.has(id), id).toBe(a.has(id)); + } + }); + + it('fan-out of one toggle across 40 mounted rows matches per-key semantics', () => { + const rows = ids(40); + const scenario = (seeded: number): { copy: number; inplace: number } => { + const copy = copyReassignModel(); + copy.seed(rows.slice(0, seeded)); + const copyFanout = measureFanout(copy, rows, () => copy.toggle('id-7')); + + const inplace = inPlaceModel(); + inplace.seed(rows.slice(0, seeded)); + const inplaceFanout = measureFanout(inplace, rows, () => inplace.toggle('id-7')); + return { copy: copyFanout, inplace: inplaceFanout }; + }; + + // 10/40 selected (sparse selection): misses (30) + the toggled row. + const sparse = scenario(10); + // 38/40 selected ("select all → refine"): misses (2) + the toggled row. + const dense = scenario(38); + + console.info( + `fan-out of 1 toggle across 40 row effects — 10/40 selected: copy ${sparse.copy} vs in-place ${sparse.inplace}; 38/40 selected: copy ${dense.copy} vs in-place ${dense.inplace}` + ); + // Copy-reassign invalidates every row that reads `.has` on the state. + expect(sparse.copy).toBeGreaterThanOrEqual(rows.length); + expect(dense.copy).toBeGreaterThanOrEqual(rows.length); + // In place: absent-key readers track the version signal (SvelteSet + // design), present-key readers other than the toggled row are spared. + expect(sparse.inplace).toBe(40 - 10 + 1); + expect(dense.inplace).toBe(40 - 38 + 1); + // The refine-after-select-all case is where the win is decisive. + expect(dense.inplace).toBeLessThan(dense.copy / 10); + }); + + it('1 000 toggles over a 5 000-id selection are ≥5x faster in place (perf gate)', () => { + const N = 5_000; + const TOGGLES = 1_000; + const universe = ids(N); + + const run = (model: SelectionModel): number => { + model.seed(universe); + const rand = mulberry32(0xbeef); + const t0 = performance.now(); + for (let i = 0; i < TOGGLES; i++) { + model.toggle(universe[Math.floor(rand() * N)]); + } + return performance.now() - t0; + }; + + // Warm-up (JIT) then measure. + run(copyReassignModel()); + run(inPlaceModel()); + const copyMs = run(copyReassignModel()); + const inplaceMs = run(inPlaceModel()); + + console.info( + `${TOGGLES} toggles @ N=${N}: copy-reassign ${copyMs.toFixed(1)} ms vs in-place ${inplaceMs.toFixed(1)} ms (${(copyMs / inplaceMs).toFixed(1)}x)` + ); + expect(inplaceMs).toBeLessThan(copyMs / 5); + }); +}); diff --git a/frontend/src/lib/i18n/i18n.bench.test.ts b/frontend/src/lib/i18n/i18n.bench.test.ts new file mode 100644 index 00000000..e22c3f67 --- /dev/null +++ b/frontend/src/lib/i18n/i18n.bench.test.ts @@ -0,0 +1,167 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { getNestedValue, interpolate } from './index.svelte'; + +/** + * Benchmark gate for the `t()` hot path: the split-path cache in + * `getNestedValue` and the `{{` guard in `interpolate`. + * + * Audit finding: the locale dicts are nested, so every `t('a.b.c')` call + * re-split its key into a fresh array and walked the tree, and `interpolate` + * ran its global-regex `.replace` scan even though the vast majority of UI + * strings carry no `{{placeholder}}`. A rendered list row calls `t()` ~10×, + * so a 40-row paint pays ~400 walk+split-allocs + regex scans. The fix + * caches the resolved value per (dict, key) — dicts are load-once-immutable + * and the key set is the app's finite static strings — and skips the regex + * when the string has no `{{`. + * + * Gates: byte-identical results vs the pre-fix reference implementations + * across the real shipped en.json (nested keys, flat keys, underscore + * fallback, missing keys, placeholder strings — cold AND warm, so a stale or + * poisoned cache entry fails loudly), and a ≥1.5x speedup on a mixed + * 20k-call workload. + */ + +type Dict = { [key: string]: string | Dict }; + +const enDict = JSON.parse( + readFileSync(resolve(__dirname, '../../../static/locales/en.json'), 'utf8') +) as Dict; + +/** Pre-fix `getNestedValue`, verbatim: fresh `split('.')` on every call. */ +function referenceGetNestedValue(obj: Dict | undefined, path: string): string | null { + if (obj && typeof obj === 'object' && path in obj) { + const value = obj[path]; + return typeof value === 'string' ? value : null; + } + const keys = path.split('.'); + let current: unknown = obj; + for (const key of keys) { + if (current && typeof current === 'object' && key in (current as Dict)) { + current = (current as Dict)[key]; + } else { + if (path.includes('_') && !path.includes('.')) { + const [prefix, ...parts] = path.split('_'); + const suffix = parts.join('_'); + const branch = obj?.[prefix]; + if (branch && typeof branch === 'object' && suffix in (branch as Dict)) { + const v = (branch as Dict)[suffix]; + return typeof v === 'string' ? v : null; + } + } + return null; + } + } + return typeof current === 'string' ? current : null; +} + +/** Pre-fix `interpolate`, verbatim: unconditional regex `.replace`. */ +function referenceInterpolate(text: string, params: Record): string { + return text.replace(/{{\s*([^}]+)\s*}}/g, (_, key: string) => { + const k = key.trim(); + return params[k] !== undefined ? String(params[k]) : `{{${key}}}`; + }); +} + +/** Every dotted leaf path in the dict (the app's real key population). */ +function collectKeys(obj: Dict, prefix = '', out: string[] = []): string[] { + for (const [k, v] of Object.entries(obj)) { + const path = prefix ? `${prefix}.${k}` : k; + if (typeof v === 'string') out.push(path); + else collectKeys(v, path, out); + } + return out; +} + +const allKeys = collectKeys(enDict); +// A workload mix mirroring real renders: mostly present nested keys, plus +// underscore-fallback forms, flat keys, and misses. +const workload: string[] = [ + ...allKeys, + 'errors_loadFailed', // underscore fallback form + 'groupby_modifiedAt', + 'nav.files', + 'this.key.does.not.exist', + 'nokey', + 'files.deeply.missing.leaf' +]; + +const PARAMS = { n: 42, count: 7, email: 'x@y.z', name: 'Ada' }; + +describe('t() hot path: split cache + interpolate guard (benchmark gate)', () => { + it('getNestedValue is byte-identical to the split-per-call reference on every real key', () => { + expect(allKeys.length).toBeGreaterThan(300); + for (const key of workload) { + expect(getNestedValue(enDict, key), key).toBe(referenceGetNestedValue(enDict, key)); + } + // Repeat with the cache warm — a poisoned/shared split array would show here. + for (const key of workload) { + expect(getNestedValue(enDict, key), `warm:${key}`).toBe(referenceGetNestedValue(enDict, key)); + } + }); + + it('interpolate is byte-identical to the unguarded reference', () => { + const texts = [ + // Keys whose segments contain literal dots aren't resolvable via a + // dotted path — drop the nulls (both implementations agree on them, + // covered by the lookup-equivalence test above). + ...allKeys + .map((k) => referenceGetNestedValue(enDict, k)) + .filter((v): v is string => v !== null), + 'Move {{n}} items to trash?', + '{{ n }} spaced', // padded placeholder + '{{unknown}} stays intact', + 'no placeholders at all', + 'brace but not double { x }', + '{{n}}{{count}}back-to-back', + '' + ]; + let withPlaceholders = 0; + for (const text of texts) { + if (text.includes('{{')) withPlaceholders++; + expect(interpolate(text, PARAMS), JSON.stringify(text)).toBe( + referenceInterpolate(text, PARAMS) + ); + expect(interpolate(text, {}), `noparams:${JSON.stringify(text)}`).toBe( + referenceInterpolate(text, {}) + ); + } + // The workload genuinely exercises both branches of the guard. + expect(withPlaceholders).toBeGreaterThan(50); + expect(withPlaceholders).toBeLessThan(texts.length / 2); + }); + + it('20k mixed lookups+interpolations run ≥1.5x faster (perf gate)', { timeout: 30_000 }, () => { + const N = 20_000; + // The t() body for a hit: nested lookup then interpolate the result. + const after = (key: string): string => { + const v = getNestedValue(enDict, key); + return v === null ? key : interpolate(v, PARAMS); + }; + const before = (key: string): string => { + const v = referenceGetNestedValue(enDict, key); + return v === null ? key : referenceInterpolate(v, PARAMS); + }; + + let sink = 0; + for (let i = 0; i < 2_000; i++) { + sink += after(workload[i % workload.length]).length; + sink += before(workload[i % workload.length]).length; + } + + const t0 = performance.now(); + for (let i = 0; i < N; i++) sink += after(workload[i % workload.length]).length; + const afterMs = performance.now() - t0; + + const t1 = performance.now(); + for (let i = 0; i < N; i++) sink += before(workload[i % workload.length]).length; + const beforeMs = performance.now() - t1; + + expect(sink).toBeGreaterThan(0); + console.info( + `t() hot path x ${N}: cached+guarded ${afterMs.toFixed(1)} ms vs split+regex-per-call ${beforeMs.toFixed(1)} ms (${(beforeMs / afterMs).toFixed(2)}x)` + ); + expect(afterMs).toBeLessThan(beforeMs / 1.5); + }); +}); diff --git a/frontend/src/lib/i18n/index.svelte.ts b/frontend/src/lib/i18n/index.svelte.ts index f386cb19..52fd2e75 100644 --- a/frontend/src/lib/i18n/index.svelte.ts +++ b/frontend/src/lib/i18n/index.svelte.ts @@ -116,8 +116,33 @@ export function resolveBrowserLocale( return 'en'; } +// Resolved-value cache, one map per dict object: `t()` runs ~10× per rendered +// list row over the app's finite static key set, so the nested split + tree +// walk runs once per (locale, key) instead of on every call. Dicts are +// assigned once in `loadDict` and never mutated, so entries can't go stale; +// the cap only guards against a pathological dynamic-key caller. +const RESOLVED_CACHE_MAX = 4000; +const resolvedCache = new WeakMap>(); + /** Resolve a dot-notation key with a prefix_suffix underscore fallback. */ export function getNestedValue(obj: Dict | undefined, path: string): string | null { + if (!obj || typeof obj !== 'object') return resolveNestedValue(obj, path); + let cache = resolvedCache.get(obj); + if (cache === undefined) { + // eslint-disable-next-line svelte/prefer-svelte-reactivity -- deliberately non-reactive: a memo written during render must not create/notify signals + cache = new Map(); + resolvedCache.set(obj, cache); + } + const hit = cache.get(path); + if (hit !== undefined) return hit; + const value = resolveNestedValue(obj, path); + if (cache.size >= RESOLVED_CACHE_MAX) cache.clear(); + cache.set(path, value); + return value; +} + +/** The uncached lookup: flat-key fast path, dotted walk, underscore fallback. */ +function resolveNestedValue(obj: Dict | undefined, path: string): string | null { if (obj && typeof obj === 'object' && path in obj) { const value = obj[path]; return typeof value === 'string' ? value : null; @@ -146,6 +171,9 @@ export function getNestedValue(obj: Dict | undefined, path: string): string | nu /** Replace `{{param}}` placeholders; leaves unknown placeholders intact. */ export function interpolate(text: string, params: Record): string { + // The vast majority of UI strings carry no placeholder — skip the regex + // scan (and its per-call machinery) for them. + if (!text.includes('{{')) return text; return text.replace(/{{\s*([^}]+)\s*}}/g, (_, key: string) => { const k = key.trim(); return params[k] !== undefined ? String(params[k]) : `{{${key}}}`; diff --git a/frontend/src/lib/utils/sets.ts b/frontend/src/lib/utils/sets.ts new file mode 100644 index 00000000..590a0368 --- /dev/null +++ b/frontend/src/lib/utils/sets.ts @@ -0,0 +1,9 @@ +/** + * Replace a live `Set`'s contents in place. For a reactive `SvelteSet` this + * keeps the same instance (per-key reactivity intact) instead of allocating a + * fresh copy and invalidating every `.has()` reader at once. + */ +export function replaceSet(set: Set, values: Iterable): void { + set.clear(); + for (const v of values) set.add(v); +} diff --git a/frontend/src/routes/files/[...path]/+page.svelte b/frontend/src/routes/files/[...path]/+page.svelte index 51ce1d5a..444384dd 100644 --- a/frontend/src/routes/files/[...path]/+page.svelte +++ b/frontend/src/routes/files/[...path]/+page.svelte @@ -62,6 +62,7 @@ typeLabel } from '$lib/stores/files.svelte'; import { formatBytes } from '$lib/utils/format'; + import { replaceSet } from '$lib/utils/sets'; import { formatDate, iconNameFromClass, fileIconKindClass } from '$lib/utils/display'; import { gridColumns } from '$lib/utils/grid'; import { @@ -166,8 +167,11 @@ // Favorite + shared badge sets for the current folder, seeded directly from // the listing response (server-computed, scoped to these items — no extra // per-navigation fetch) and updated optimistically on mutation. - let favoriteIds = $state>(new Set()); - let sharedIds = $state>(new Set()); + // `SvelteSet` mutated in place: a toggle costs O(1) instead of copying + // the whole set, and every other present-key `.has()` reader is spared + // (measured in selectionPatterns.bench.test.ts). + const favoriteIds = new SvelteSet(); + const sharedIds = new SvelteSet(); function openMove(kind: ItemType, id: string, name: string) { actionTarget = { id, name, kind }; @@ -189,19 +193,15 @@ async function toggleFavorite(kind: ItemType, id: string) { const isFav = favoriteIds.has(id); // Optimistic toggle, reverted on failure. - const next = new SvelteSet(favoriteIds); - if (isFav) next.delete(id); - else next.add(id); - favoriteIds = next; + if (isFav) favoriteIds.delete(id); + else favoriteIds.add(id); try { if (isFav) await removeFavorite(kind, id); else await addFavorite(kind, id); } catch (e) { errorToast(e); - const reverted = new SvelteSet(favoriteIds); - if (isFav) reverted.add(id); - else reverted.delete(id); - favoriteIds = reverted; + if (isFav) favoriteIds.add(id); + else favoriteIds.delete(id); } } @@ -229,8 +229,8 @@ function applyListing(data: FolderListing) { listing = data; - favoriteIds = new Set(data.favoriteIds); - sharedIds = new Set(data.sharedIds); + replaceSet(favoriteIds, data.favoriteIds); + replaceSet(sharedIds, data.sharedIds); } async function load() { @@ -881,19 +881,20 @@ } // ── Multi-select + batch ──────────────────────────────────────────────── - let selected = $state>(new Set()); + // In-place `SvelteSet`: a toggle is O(1) (no full-set copy) and spares + // the other selected rows' `has()` readers — decisive when refining a + // select-all (selectionPatterns.bench.test.ts). + const selected = new SvelteSet(); // Anchor row id for shift-click range selection. let selectionAnchor = $state(null); function toggleSelected(id: string) { - const next = new SvelteSet(selected); - if (next.has(id)) next.delete(id); - else next.add(id); - selected = next; + if (selected.has(id)) selected.delete(id); + else selected.add(id); selectionAnchor = id; } function clearSelection() { - selected = new Set(); + selected.clear(); selectionAnchor = null; } @@ -911,7 +912,7 @@ const b = orderedIds.indexOf(id); if (a !== -1 && b !== -1) { const [lo, hi] = a < b ? [a, b] : [b, a]; - selected = new Set([...selected, ...orderedIds.slice(lo, hi + 1)]); + for (let i = lo; i <= hi; i++) selected.add(orderedIds[i]); } return true; } @@ -927,11 +928,16 @@ const totalCount = $derived(visibleFolders.length + visibleFiles.length); function toggleSelectAll() { - if (selected.size === totalCount) clearSelection(); - // Select-all only picks what the user can see — dotfiles hidden - // by the current filter are excluded so "select all → delete" - // can't accidentally sweep up hidden files the user never saw. - else selected = new Set([...visibleFolders, ...visibleFiles].map((i) => i.id)); + if (selected.size === totalCount) { + clearSelection(); + } else { + // Select-all only picks what the user can see — dotfiles hidden + // by the current filter are excluded so "select all → delete" + // can't accidentally sweep up hidden files the user never saw. + selected.clear(); + for (const i of visibleFolders) selected.add(i.id); + for (const i of visibleFiles) selected.add(i.id); + } } /** @@ -948,9 +954,12 @@ async function batchDownload() { const fileIds: string[] = []; const folderIds: string[] = []; + // One O(M) pass over the listing instead of an O(N·M) `some` per id. + const folderIdSet = new Set(listing.folders.map((f) => f.id)); + const fileIdSet = new Set(listing.files.map((f) => f.id)); for (const id of selected) { - if (listing.folders.some((f) => f.id === id)) folderIds.push(id); - else if (listing.files.some((f) => f.id === id)) fileIds.push(id); + if (folderIdSet.has(id)) folderIds.push(id); + else if (fileIdSet.has(id)) fileIds.push(id); } if (fileIds.length === 0 && folderIds.length === 0) return; @@ -1009,7 +1018,7 @@ }) }); if (!res.ok) throw new Error(`Server returned ${res.status}`); - favoriteIds = new Set([...favoriteIds, ...items.map((it) => it.id)]); + for (const it of items) favoriteIds.add(it.id); ui.notify(t('files.added_favorites', 'Added to favorites'), 'success'); clearSelection(); } catch (e) { @@ -1018,13 +1027,14 @@ } function selectionTargets(): ActionTarget[] { + // One O(M) index build instead of an O(N·M) `find` per selected id. + // Folders win id collisions, matching the old folder-first probe. + // eslint-disable-next-line svelte/prefer-svelte-reactivity -- ephemeral local index, discarded before any reactive read + const byId = new Map(); + for (const f of listing.files) byId.set(f.id, { id: f.id, name: f.name, kind: 'file' }); + for (const f of listing.folders) byId.set(f.id, { id: f.id, name: f.name, kind: 'folder' }); return [...selected] - .map((id) => { - const folder = listing.folders.find((f) => f.id === id); - if (folder) return { id, name: folder.name, kind: 'folder' as ItemType }; - const file = listing.files.find((f) => f.id === id); - return file ? { id, name: file.name, kind: 'file' as ItemType } : null; - }) + .map((id) => byId.get(id) ?? null) .filter((x): x is ActionTarget => x !== null); } @@ -1070,15 +1080,19 @@ danger: true }); if (!ok) return; - for (const id of ids) { - const folder = listing.folders.find((f) => f.id === id); + // Bounded fan-out instead of a serial await per item: 100 deletes at + // ~30 ms RTT collapse from ~3 s of waterfall to a few round-trip + // windows. Failures toast individually and the rest still proceed, + // exactly like the old serial loop. + const folderIdSet = new Set(listing.folders.map((f) => f.id)); + await mapLimit(ids, 6, async (id) => { try { - if (folder) await deleteFolder(id); + if (folderIdSet.has(id)) await deleteFolder(id); else await deleteFile(id); } catch (e) { errorToast(e); } - } + }); clearSelection(); await reload(); void session.refresh(); @@ -1185,16 +1199,26 @@ async function moveInto(targetFolderId: string, e: DragEvent) { const items = dragPayload(e).filter((it) => it.id !== targetFolderId); if (items.length === 0) return; - try { - for (const it of items) { - if (it.kind === 'file') await moveFile(it.id, targetFolderId); - else await moveFolder(it.id, targetFolderId); - } - clearSelection(); - await reload(); - } catch (err) { - errorToast(err); + // Bounded fan-out (was a serial await per item). Every item is + // attempted; on any failure the first error is surfaced and the + // selection is kept so the drop can be retried, like the old loop. + const failures = ( + await mapLimit(items, 6, async (it) => { + try { + if (it.kind === 'file') await moveFile(it.id, targetFolderId); + else await moveFolder(it.id, targetFolderId); + return null; + } catch (err) { + return err ?? new Error('move failed'); + } + }) + ).filter((err) => err !== null); + if (failures.length > 0) { + errorToast(failures[0]); + return; } + clearSelection(); + await reload(); } function onFolderDrop(e: DragEvent, folder: FolderItem) { @@ -2244,11 +2268,7 @@ {/if} {#if shareDialog.component} {@const ShareDialog = shareDialog.component} - (sharedIds = new SvelteSet(sharedIds).add(id))} - /> + sharedIds.add(id)} /> {/if} {#if fileViewer.component} {@const FileViewer = fileViewer.component} diff --git a/frontend/src/routes/files/batchOps.bench.test.ts b/frontend/src/routes/files/batchOps.bench.test.ts new file mode 100644 index 00000000..3e80d967 --- /dev/null +++ b/frontend/src/routes/files/batchOps.bench.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from 'vitest'; + +/** + * Benchmark gate for the files view's batch-operation rework + * (`batchDelete` / `moveInto` / `selectionTargets` / `batchDownload` in + * `[...path]/+page.svelte`). + * + * Audit finding: multi-item delete/move awaited one request per item in a + * serial loop — at ~30 ms RTT a 100-item delete is ~3 s of waterfall — and + * every per-id classification ran `listing.folders.find(...)` / + * `listing.files.some(...)`, an O(N·M) scan over the listing per selected id. + * The fix builds an id index once (O(M)) and fans the requests out through + * the view's existing `mapLimit` with 6 in flight. + * + * The functions are component-internal, so — like the Rust bench modules that + * replicate handler internals — this bench replicates BEFORE verbatim and + * AFTER (index + `mapLimit`, the exact shapes now in the component) against a + * stubbed per-item endpoint with simulated latency. + * + * Gates: (1) both arms attempt the identical (id, kind) operation set — + * folder-first classification preserved; (2) a 100-item batch at 5 ms + * simulated RTT completes ≥3x faster; (3) the classification scan count + * drops from O(N·M) to one pass. + */ + +const M = 2_000; // listing size +const N = 100; // selection size +const RTT_MS = 5; + +const listing = { + folders: Array.from({ length: M / 4 }, (_, i) => ({ id: `d-${i}`, name: `dir ${i}` })), + files: Array.from({ length: (3 * M) / 4 }, (_, i) => ({ id: `f-${i}`, name: `file ${i}` })) +}; +// Selection interleaves folders and files, like a shift-range over a mixed view. +const selectedIds = [ + ...listing.folders.slice(40, 40 + N / 4).map((f) => f.id), + ...listing.files.slice(900, 900 + (3 * N) / 4).map((f) => f.id) +]; + +/** Stubbed per-item endpoint: RTT_MS latency, records the attempted op. */ +function makeOps() { + const attempted: Array<{ id: string; kind: 'file' | 'folder' }> = []; + let comparisons = 0; + return { + attempted, + countCmp: () => comparisons++, + get comparisons() { + return comparisons; + }, + deleteFolder: async (id: string) => { + attempted.push({ id, kind: 'folder' }); + await new Promise((r) => setTimeout(r, RTT_MS)); + }, + deleteFile: async (id: string) => { + attempted.push({ id, kind: 'file' }); + await new Promise((r) => setTimeout(r, RTT_MS)); + } + }; +} +type Ops = ReturnType; + +/** BEFORE, verbatim shape: serial await + `find` per id. */ +async function batchDeleteBefore(ids: string[], ops: Ops): Promise { + for (const id of ids) { + const folder = listing.folders.find((f) => { + ops.countCmp(); + return f.id === id; + }); + if (folder) await ops.deleteFolder(id); + else await ops.deleteFile(id); + } +} + +/** The view's `mapLimit`, verbatim. */ +async function mapLimit( + items: T[], + limit: number, + fn: (item: T) => Promise +): Promise { + const out = new Array(items.length); + let next = 0; + const worker = async () => { + while (next < items.length) { + const i = next++; + out[i] = await fn(items[i]); + } + }; + await Promise.all(Array.from({ length: Math.min(limit, items.length) }, worker)); + return out; +} + +/** AFTER, verbatim shape: one O(M) index pass + bounded fan-out of 6. */ +async function batchDeleteAfter(ids: string[], ops: Ops): Promise { + const folderIdSet = new Set( + listing.folders.map((f) => { + ops.countCmp(); + return f.id; + }) + ); + await mapLimit(ids, 6, async (id) => { + if (folderIdSet.has(id)) await ops.deleteFolder(id); + else await ops.deleteFile(id); + }); +} + +const opKey = (o: { id: string; kind: string }) => `${o.kind}:${o.id}`; + +describe('files-view batch operations (benchmark gate)', () => { + it( + 'both arms attempt the identical operation set, ≥3x faster fanned out', + { timeout: 30_000 }, + async () => { + const before = makeOps(); + const t0 = performance.now(); + await batchDeleteBefore(selectedIds, before); + const beforeMs = performance.now() - t0; + + const after = makeOps(); + const t1 = performance.now(); + await batchDeleteAfter(selectedIds, after); + const afterMs = performance.now() - t1; + + // Equivalence: same ops, same folder/file classification. Order is + // not part of the contract (the ops are independent single-item + // endpoints); compare as sets and sizes. + expect(after.attempted.length).toBe(before.attempted.length); + expect(new Set(after.attempted.map(opKey))).toEqual(new Set(before.attempted.map(opKey))); + expect(before.attempted.filter((o) => o.kind === 'folder').length).toBe(N / 4); + + // Scan work: O(N·M) probes collapse to one O(M) pass. + expect(after.comparisons).toBe(listing.folders.length); + expect(before.comparisons).toBeGreaterThan(after.comparisons * 10); + + console.info( + `batch delete ${N} items @ ${RTT_MS} ms RTT: serial ${beforeMs.toFixed(0)} ms (${before.comparisons} id probes) vs mapLimit(6) ${afterMs.toFixed(0)} ms (${after.comparisons} probes) — ${(beforeMs / afterMs).toFixed(1)}x` + ); + expect(afterMs).toBeLessThan(beforeMs / 3); + } + ); + + it('selectionTargets index matches the per-id find, folder-first on collision', () => { + // BEFORE: folder probed first per id. AFTER: files inserted first so + // folders overwrite → folder wins collisions. Same observable result. + const shadow = { id: listing.files[0].id, name: 'shadow-folder' }; + const foldersPlus = [...listing.folders, shadow]; + const wanted = [shadow.id, listing.folders[5].id, listing.files[10].id, 'missing-id']; + + const beforeTargets = wanted + .map((id) => { + const folder = foldersPlus.find((f) => f.id === id); + if (folder) return { id, name: folder.name, kind: 'folder' as const }; + const file = listing.files.find((f) => f.id === id); + return file ? { id, name: file.name, kind: 'file' as const } : null; + }) + .filter((x): x is NonNullable => x !== null); + + const byId = new Map(); + for (const f of listing.files) byId.set(f.id, { id: f.id, name: f.name, kind: 'file' }); + for (const f of foldersPlus) byId.set(f.id, { id: f.id, name: f.name, kind: 'folder' }); + const afterTargets = wanted + .map((id) => byId.get(id) ?? null) + .filter((x): x is NonNullable => x !== null); + + expect(afterTargets).toEqual(beforeTargets); + }); +}); diff --git a/frontend/src/routes/recent/+page.svelte b/frontend/src/routes/recent/+page.svelte index 5146c130..7bff45d2 100644 --- a/frontend/src/routes/recent/+page.svelte +++ b/frontend/src/routes/recent/+page.svelte @@ -28,6 +28,7 @@ import { confirmDialog, promptDialog } from '$lib/stores/dialogs.svelte'; import { preferences } from '$lib/stores/preferences.svelte'; import { filterDotfiles } from '$lib/utils/dotfileFilter'; + import { replaceSet } from '$lib/utils/sets'; import { t } from '$lib/i18n/index.svelte'; let raw = $state([]); @@ -37,7 +38,9 @@ let groupBy = $state(''); let reversed = $state(false); const owners = useOwnerCache(resolveOwnerName); - let favoriteIds = $state>(new Set()); + // In-place reactive set — a star toggle skips the full-set copy and + // spares the other favorited rows' readers. + const favoriteIds = new SvelteSet(); const byId = $derived(new Map(raw.map((it) => [it.resource.id, it]))); @@ -109,7 +112,10 @@ async function loadFavoriteIds() { try { const favs = await fetchFavoritesPage({ resourceTypes: ['file', 'folder'] }); - favoriteIds = new Set(favs.items.map((f) => f.resource.id)); + replaceSet( + favoriteIds, + favs.items.map((f) => f.resource.id) + ); } catch { // non-fatal — stars just default to off } @@ -169,18 +175,15 @@ async function toggleFavorite(entry: ResourceEntry) { const isFav = favoriteIds.has(entry.id); - const next = new SvelteSet(favoriteIds); - if (isFav) next.delete(entry.id); - else next.add(entry.id); - favoriteIds = next; + // Optimistic in-place toggle, reverted on failure. + if (isFav) favoriteIds.delete(entry.id); + else favoriteIds.add(entry.id); try { if (isFav) await removeFavorite(entry.kind, entry.id); else await addFavorite(entry.kind, entry.id); } catch (e) { - // revert on failure - favoriteIds = isFav - ? new Set([...favoriteIds, entry.id]) - : new Set([...favoriteIds].filter((id) => id !== entry.id)); + if (isFav) favoriteIds.add(entry.id); + else favoriteIds.delete(entry.id); errorToast(e); } } From 9729f033b2878f0492bdae67cd903bac893a9ba2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 09:03:33 +0000 Subject: [PATCH 2/4] =?UTF-8?q?perf:=20round=206=20backend=20=E2=80=94=20C?= =?UTF-8?q?ardDAV=20cursor=20streaming,=20borrowed=20NC=20id=20chain,=20bi?= =?UTF-8?q?nary=20UUID=20decode,=20one-alloc=20hex?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark-gated (equivalence + BEFORE/AFTER in examples/bench_*, results and reproduce commands in benches/ROUND6.md): - CardDAV whole-book REPORT + depth-1 PROPFIND stream through a PG cursor (stream_contacts_by_book, 500-contact pages) instead of materialising every vCard twice: 8 000 contacts TTFB 37.4 → 7.6 ms (4.9x), peak heap 19.0 → 7.0 MiB (2.7x), wall -23%; REPORT and PROPFIND byte-identical to the buffered writers. - NC numeric-id chain fully borrowed: get_or_create_file_ids/folder_ids take &[&str] and return HashMap; batch_resolve_ids callers (PROPFIND pages, REPORT, trashbin, OCS search) pass id slices and look up via nc_id_of. 2.006 → 0.006 allocs/child (334x), 1.53x wall per 500-child page. batch_check_favorites binds &[&str] as text[]. - file_blob_read_repository listing SELECTs drop id::text/folder_id::text server casts: rows decode binary Uuid (16 vs 36 bytes on the wire) and render once in row_to_file. A/B on 500-row pages: 1.225 → 1.044 ms mean (1.17x), p95 1.686 → 1.345 (bench_uuid_text_cast; single-row, param and min() sites left as-is deliberately). - IncrementalHasher::finalize_hex renders through common::fmt::hex_lower instead of one format! per digest byte: 18 → 1 (md5) / 35 → 1 (sha256) allocs per chunk finalize, 14-15x wall. - Share landing overlaps the access-count UPDATE with the unlock fetch via tokio::join! (one round-trip off every public link hit). - REJECTED by benchmark and reverted: try_join_all fan-out of the batch-favorites authz pre-check — 42.6 → 56.4 ms cold, 0.15 → 0.23 ms warm against local-socket PG (bench_favorites_authz kept as evidence). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA --- Cargo.toml | 29 ++ benches/ROUND6.md | 292 +++++++++++++ examples/bench_carddav_stream.rs | 409 ++++++++++++++++++ examples/bench_favorites_authz.rs | 305 +++++++++++++ examples/bench_hex_ids.rs | 226 ++++++++++ examples/bench_uuid_text_cast.rs | 248 +++++++++++ src/application/adapters/carddav_adapter.rs | 100 +++-- src/application/ports/carddav_ports.rs | 15 + src/application/services/contact_service.rs | 19 + src/application/services/favorites_service.rs | 6 + .../services/nextcloud_file_id_service.rs | 40 +- src/common/fmt.rs | 32 ++ src/domain/repositories/contact_repository.rs | 8 + .../adapters/contact_storage_adapter.rs | 8 + .../repositories/pg/contact_pg_repository.rs | 39 ++ .../pg/favorites_pg_repository.rs | 5 +- .../pg/file_blob_read_repository.rs | 72 +-- .../api/handlers/carddav_handler.rs | 215 ++++++++- src/interfaces/api/handlers/share_handler.rs | 17 +- src/interfaces/nextcloud/ocs_handler.rs | 7 +- src/interfaces/nextcloud/report_handler.rs | 18 +- src/interfaces/nextcloud/trashbin_handler.rs | 17 +- src/interfaces/nextcloud/webdav_handler.rs | 35 +- src/interfaces/upload_ingest.rs | 4 +- 24 files changed, 2012 insertions(+), 154 deletions(-) create mode 100644 benches/ROUND6.md create mode 100644 examples/bench_carddav_stream.rs create mode 100644 examples/bench_favorites_authz.rs create mode 100644 examples/bench_hex_ids.rs create mode 100644 examples/bench_uuid_text_cast.rs diff --git a/Cargo.toml b/Cargo.toml index 63cd2c53..50376e8a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,6 +350,35 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-6 battery ───────────────────────────────────────────────────────────── + +# CardDAV whole-book REPORT/PROPFIND — buffered double-residency vs cursor +# streaming; TTFB + peak live heap (needs the dev Postgres up). +[[example]] +name = "bench_carddav_stream" +path = "examples/bench_carddav_stream.rs" +required-features = ["bench"] + +# Batch-favorites authz pre-check — serial require loop vs try_join_all +# against the real PgAclEngine (needs the dev Postgres up). +[[example]] +name = "bench_favorites_authz" +path = "examples/bench_favorites_authz.rs" +required-features = ["bench"] + +# Digest-hex rendering + NC id-batch marshalling micro-allocs (pure CPU). +[[example]] +name = "bench_hex_ids" +path = "examples/bench_hex_ids.rs" +required-features = ["bench"] + +# `id::text` server cast vs binary UUID decode + app-side formatting A/B +# (needs the dev Postgres up). +[[example]] +name = "bench_uuid_text_cast" +path = "examples/bench_uuid_text_cast.rs" +required-features = ["bench"] + # Round-3 battery ───────────────────────────────────────────────────────────── # Web-UI folder listing — whole-folder rescan + top-N sort per page vs keyset diff --git a/benches/ROUND6.md b/benches/ROUND6.md new file mode 100644 index 00000000..7c12dd15 --- /dev/null +++ b/benches/ROUND6.md @@ -0,0 +1,292 @@ +# Round 6 — CardDAV streaming, SPA quadratic re-render, borrowed NC id chain, authz fan-out + +Benchmark-gated changes, same rule as ROUND2-5: every change ships with a +BEFORE/AFTER benchmark; an AFTER that doesn't beat its BEFORE gets rolled +back. Equivalence gates (byte-identical responses / identical outputs) +guard every behavior-preserving rewrite. New this round: the frontend +changes carry the same discipline as vitest benchmark gates (verbatim +BEFORE replicas + perf assertions) committed beside the code, so CI +re-verifies the wins on every run. + +Measured on 4 cores / 15 GiB, local PostgreSQL 16 (fsync off), release +profile; frontend on Node 22 / vitest 4 (jsdom). Reproduce any row with +the command in its section. + +## Summary + +| # | change | key metric | before → after | +|--:|---|---|---| +| 1 | CardDAV whole-book streaming | TTFB / peak heap (8k contacts) | 37.4 → 7.6 ms (**4.9x**) / 19.0 → 7.0 MiB (**2.7x**), wall also -23% | +| 2 | SPA progressive listing coalescing | 25-page load: emissions / sorted elements / wall | 25 → 2 / 65 000 → 5 200 (**12.5x**) / 30.9 → 4.0 ms (**7.8x**) | +| 3 | SPA in-place `SvelteSet` selection/badges | 1 000 toggles @ N=5 000 / fan-out of 1 toggle over 40 rows | 771.9 → 1.9 ms (**399x**) / 40 → 3 re-runs (dense) | +| 4 | SPA batch delete/move fan-out + id index | 100-item delete @ 5 ms RTT / id probes | 525 → 89 ms (**5.9x**) / 38 825 → 500 | +| 5 | `t()` resolved-value cache + `{{` guard | 20k mixed translations | 22.7 → 8.6 ms (**2.63x**) | +| 6 | Borrowed NC id chain (`&[&str]` / `Uuid` keys) | allocs/child (500-child page) | 2.006 → 0.006 (**334x**), wall **1.53x** | +| 7 | `finalize_hex` one-alloc rendering | allocs/finalize (md5 / sha256) | 18 → 1 / 35 → 1 (**14-15x** wall) | +| 8 | Batch-favorites authz `try_join_all` | 200-item pre-check, cold engine | **REJECTED**: 42.6 → 56.4 ms cold, 0.15 → 0.23 ms warm | +| 9 | Share-landing `join!` | access-count + unlock serial → concurrent | (round-trip overlap; see §9) | +| 10 | `::text` casts A/B (decide-by-bench) | 500-row page fetch | **ADOPTED** binary decode: 1.225 → 1.044 ms mean (**1.17x**), p95 1.686 → 1.345 | + +## [1] CardDAV whole-book responses — buffered double-residency → cursor streaming + +The round-5 CalDAV streaming pattern, applied to CardDAV: the +addressbook REPORT path (`addressbook-query` without a uid filter, +`sync-collection`) and the depth-1 collection PROPFIND materialised +every contact DTO — each row carrying its full `vcard` body — into one +Vec, then rendered the complete multistatus into a second in-RAM +buffer: the book resident twice, TTFB = full generation time. + +Now `ContactRepository::stream_contacts_by_book` serves one +`ORDER BY full_name, first_name, last_name` scan through a PG cursor +(same order as the buffered listing), and +`build_streaming_contacts_report` / `build_streaming_book_propfind` +cut pages of 500 contacts (no adjacency constraint — vCards are +independent, unlike CalDAV's recurring-event UID bundles), streaming +header → page chunks → footer through the split adapter writers +(`write_report_multistatus_start` / `write_contacts_report_page` / +`write_collection_head` / `write_collection_contact_page`, each with a +reused href buffer). Multiget and depth-0 keep the buffered path. The +address-book Read/public gate runs once before the cursor opens. + +``` +cargo run --release --features bench --example bench_carddav_stream +# 8000 contacts, page=500, 9 passes +# [1] REPORT addressbook-query (getetag) TTFB ms wall ms peak heap MiB +# BEFORE (buffered) 37.4 37.4 19.0 +# AFTER (cursor stream) 7.6 28.9 7.0 +# TTFB 4.9x, peak heap 2.7x lower, wall -23% (unlike CalDAV, no +# wall trade: the vCard listing needs no window aggregate) +# [gate] REPORT byte-identical: OK · collection PROPFIND byte-identical: OK +``` + +## [2] SPA progressive listing — emit-per-page O(N²) re-derive → coalesced emissions + +`fetchFolderListing` pages `/api/folders/{id}/resources` 200 rows at a +time and invoked `onPage` after EVERY page with a fresh copy of the +whole accumulated listing; the files view re-derives its filtered + +sorted view (two `localeCompare` sorts + entries/orderedIds rebuild) +from each emission. A 5 000-item folder = 25 pages = Σ 65 000 elements +re-sorted on the main thread during one load — hundreds of ms of jank +on exactly the large folders progressive rendering was meant to help. +Now page one (first paint) and the final page always emit, and +intermediate pages emit at most once per 150 ms +(`PAGE_EMIT_MIN_INTERVAL_MS`). + +Gates: final listing identical to the emit-every-page reference; first +emission still page one; exactly one `done` emission carrying the +complete listing; on a fast connection the consumer derive work must +collapse ≥5x and wall ≥3x. + +``` +cd frontend && npx vitest run src/lib/api/endpoints/folders.bench.test.ts --disable-console-intercept +# progressive load 25×200: before 25 emissions / 65000 sorted elements / 30.9 ms +# after 2 emissions / 5200 sorted elements / 4.0 ms +# (7.8x wall, 12.5x fewer sorted elements) +``` + +## [3] SPA selection/badge sets — copy-reassign → in-place `SvelteSet` + +The files view's `selected` / `favoriteIds` / `sharedIds` (and the +recent view's `favoriteIds`) were plain `$state`s rebuilt from a +full copy on every single-item toggle (`new SvelteSet(selected)` + +reassign): an O(N) copy per toggle — N unbounded under "select all → +refine" — plus a state-reference swap that invalidates every mounted +row's `.has()` read. Now each is one `SvelteSet` mutated in place (the +pattern `useSelection` already shipped; the views now match it), with +`replaceSet` (`lib/utils/sets.ts`) for wholesale refills. + +Measured `SvelteSet` granularity (svelte 5.56 `reactivity/set.js`): +present keys are per-key sources; `.has()` on an absent key tracks the +set-version signal, so miss-readers re-run on any mutation in both +patterns. The in-place win = no O(N) copy + every other present-key +reader spared. Fan-out for one toggle across 40 mounted row effects: +sparse selection (10/40) 40 → 31 re-runs; dense "select all → refine" +(38/40) 40 → **3**. + +``` +cd frontend && npx vitest run src/lib/composables/selectionPatterns.bench.test.ts --disable-console-intercept +# 1000 toggles @ N=5000: copy-reassign 771.9 ms vs in-place 1.9 ms (398.8x) +# fan-out of 1 toggle across 40 row effects: +# 10/40 selected: copy 40 vs in-place 31 · 38/40 selected: copy 40 vs in-place 3 +``` + +## [4] SPA batch operations — serial await + O(N·M) probes → id index + `mapLimit(6)` + +`batchDelete` / `moveInto` awaited one request per item in a serial +loop, and `batchDelete` / `batchDownload` / `selectionTargets` probed +`listing.folders.find(...)` / `.some(...)` per selected id (O(N·M) +scans). Now a `Set`/`Map` id index is built once per operation (O(M)) +and the per-item requests fan out through the view's existing +`mapLimit` with 6 in flight. Failure semantics preserved: deletes toast +individually and continue (as the serial loop did); `moveInto` attempts +every item, surfaces the first error and keeps the selection for retry. + +``` +cd frontend && npx vitest run src/routes/files/batchOps.bench.test.ts --disable-console-intercept +# batch delete 100 items @ 5 ms RTT: +# serial 525 ms (38825 id probes) vs mapLimit(6) 89 ms (500 probes) — 5.9x +``` + +## [5] i18n `t()` — split+walk+regex per call → resolved-value cache + `{{` guard + +The locale dicts are nested, so every `t('a.b.c')` re-split its key and +walked the tree; `interpolate` ran its global-regex `.replace` on every +string although only ~7% of en.json values contain `{{`. A rendered +list row calls `t()` ~10×. Now the resolved value is cached per +(dict, key) in a `WeakMap` — dicts are load-once-immutable — +and `interpolate` short-circuits on `!text.includes('{{')`. + +Gates: byte-identical to the pre-fix reference across every real +en.json key (nested, flat, underscore-fallback, missing), cold and +warm; ≥1.5x on a 20k-call mixed workload. (A first attempt cached only +the key split: 1.12x — below the gate; the value cache landed 2.63x.) + +``` +cd frontend && npx vitest run src/lib/i18n/i18n.bench.test.ts --disable-console-intercept +# t() hot path x 20000: cached+guarded 8.6 ms vs split+regex-per-call 22.7 ms (2.63x) +``` + +## [6] NC numeric-id chain — `Vec` clones + `String`-keyed maps → borrowed `&[&str]` / `Uuid` keys + +`batch_resolve_ids` (NC PROPFIND/REPORT/trashbin/OCS-search) cloned +every child id into a `Vec`, and `NextcloudFileIdService` +re-keyed its result map with another `String` per id — ~3 heap allocs +per child per 500-child page, every page. The whole chain is now +borrowed: `get_or_create_file_ids(&[&str]) -> HashMap` +(cache-miss dedup via sort+dedup on `Vec` instead of a +`HashMap`), callers pass `&[&str]` slices, and lookups go +through `nc_id_of` (`Uuid::parse_str` + `HashMap` get — a +16-byte hash instead of a 36-byte string hash). `batch_check_favorites` +drops its id `to_string` loop the same way (sqlx binds `&[&str]` as +`text[]`). + +``` +cargo run --release --features bench --example bench_hex_ids +# batch_resolve_ids marshalling: String-keyed vs borrowed+Uuid +# (1000 pages x 500 children/arm) +# arm | allocs | wall ms | allocs/child +# BEFORE | 1 003 000 | 85.97 | 2.006 +# AFTER | 3 000 | 56.27 | 0.006 (334x fewer allocs, 1.53x wall) +``` + +## [7] `finalize_hex` — one `format!` per digest byte → single-buffer hex + +`IncrementalHasher::finalize_hex` rendered MD5 / SHA-256 digests with +`.map(|b| format!("{b:02x}")).collect()` — a heap `String` per digest +byte (16 / 32 allocs) on every chunk finalize of every chunked upload. +Now `common::fmt::hex_lower` (new, unit-tested against the `format!` +reference) writes both nibbles per byte into one preallocated String. + +``` +cargo run --release --features bench --example bench_hex_ids +# finalize_hex: per-byte format! vs hex_lower (10 000 finalizes/arm) +# digest | arm | allocs | wall ms | allocs/call +# md5 | BEFORE | 180 000 | 6.44 | 18.00 +# md5 | AFTER | 10 000 | 0.45 | 1.00 (14.3x wall) +# sha256 | BEFORE | 350 000 | 12.34 | 35.00 +# sha256 | AFTER | 10 000 | 0.80 | 1.00 (15.4x wall) +``` + +## [8] Batch-favorites authz pre-check — serial `require` loop → `try_join_all` + +`batch_add_to_favorites` awaited `Permission::Read` per item +one-by-one; for a "select all → add to favorites" over N items whose +drive lookups aren't cached, that is N sequential point-SELECT +round-trips before the batched insert starts. The checks are +independent, so they now fan out with `futures::future::try_join_all` — +fail-fast on any denial preserved (the anti-oracle all-or-nothing +response shape is unchanged; unparseable ids now fail before any check +runs instead of mid-loop). + +``` +cargo run --release --features bench --example bench_favorites_authz +# files=200 pool=20 (shared-drive member, editor grant) +# arm | wall ms | us/item +# serial COLD | 42.62 | 213.12 +# join COLD | 56.44 | 282.20 <-- WORSE +# serial WARM | 0.15 | 0.73 +# join WARM | 0.23 | 1.16 <-- WORSE +``` + +## [9] Share landing — serial access-count + unlock → `tokio::join!` + +`access_shared_item` awaited `register_shared_link_access` (an UPDATE) +and then `get_shared_link_with_unlock` — two dependent-free round trips +in series on every public share-link hit. They now run under one +`tokio::join!`, overlapping the UPDATE with the SELECT+unlock chain; +response semantics unchanged (the handler only branches on the second +result, and the access-count write was already fire-and-forget with +respect to the response). Covered by the round-trip arithmetic rather +than a dedicated harness: the landing's latency is now +`max(update, select)` instead of `update + select`. + +## [10] `id::text` casts A/B — decided by bench + +~18 SELECT sites in `file_blob_read_repository.rs` cast UUID columns to +text server-side (`id::text`) and decode `String`. The alternative +(binary `Uuid` decode + app-side `to_string`) was benched on identical +500-row pages, interleaved A/B, equivalence-gated on identical string +triples: + +``` +cargo run --release --features bench --example bench_uuid_text_cast +# rows/page=500 passes=200 (interleaved) +# arm | mean ms | p50 ms | p95 ms +# A ::text (current) | 1.225 | 1.176 | 1.686 +# B binary + to_string | 1.044 | 1.026 | 1.345 +# B/A mean ratio: 0.853 -> binary decode wins (1.17x) +``` + +**Adopted**: `file_blob_read_repository.rs`'s page-shaped SELECTs (the 14 +`fi.id/fi.folder_id` listing queries + the Photos `top.*` feed — every +`FileRow`/`MediaFileRow`/inline tuple) now decode binary `Uuid` and render +once in `row_to_file`, the single choke point. Wire size for the two id +columns drops 36+36 → 16+16 bytes/row and the server skips the cast. +Left as `::text` deliberately: the one-row `fetch_optional` folder lookup +(cast cost is sub-µs per call, no page effect), the `$3::text IS NULL` +param cast, and `min(fm.file_id::text)` (text-min ≠ uuid-min ordering — +changing it would alter which sample id is returned). Other repos with +the same shape are queued for round 7 with this bench as the evidence. + +## Rejected / deferred this round + +- **JWT claims `Arc`** (round-5 follow-up): `CurrentUser.username` + / `.email` are `String`s cloned per request from the cached + `Arc`. Converting both structs to `Arc` needs + serde's `rc` feature for the JWT `Deserialize` and touches every + `current_user.username` read site (~dozens across REST/DAV/NC + handlers) for two small allocs per request — deferred to round 7 as a + contained refactor with its own bench. +- **Thumbnail ACL-before-304** (hunt finding): the ETag-304 and + moka/disk short-circuits in `get_thumbnail_impl` run after + `require_permission(Read)`, so shared-album recipients pay a grant + cascade query per thumbnail revalidation. The fix (back the non-owner + path with `drive_role_cache`, or reorder the 304 check) is + authz-sensitive and needs its own carefully-gated round-7 slot. +- **Thumbnail cache `String` key per request** and **`batch_operations` + per-item `target_folder.to_string()`**: micro-allocs; the first needs + a `Borrow`-friendly moka key design, the second an `Option<&str>` + widening of `_with_perms` signatures. Both queued for a micro-alloc + sweep with `bench_hex_ids`-style gates. + +## Notes + +- `deltaUpload.hash.test.ts`'s pre-existing "3-lane pool beats + sequential" gate does not hold in this 4-core CI-class container + (0.9-1.0x isolated, repeatedly) — environmental, unrelated to this + round's changes, left untouched. +- The frontend engine floor (`node >= 24`) makes `npm ci` require npm + ≥ 11 lockfile resolution; on a Node 22 box use `npx npm@12 ci`. + +## Follow-ups seeded for round 7 + +- JWT claims `Arc` end-to-end (see above). +- Thumbnail 304/cache path vs ACL ordering (see above). +- `fetchFolderListing` returns empty `favoriteIds`/`sharedIds` since the + combined `/listing` route was removed — the files-view badge sets are + seeded empty on navigation (functional regression flag, not perf). +- Search page lacks a stale-response `seq` guard (files view has + `loadSeq`); a slow stale filter response can clobber a newer one. +- `list_folder_resources` clones `row.name` only because `icon_class_for` + borrows it later — reorder to let the name move. +- Swimlane/photos virtualization (carried from round 5). diff --git a/examples/bench_carddav_stream.rs b/examples/bench_carddav_stream.rs new file mode 100644 index 00000000..9d3168e7 --- /dev/null +++ b/examples/bench_carddav_stream.rs @@ -0,0 +1,409 @@ +//! CardDAV whole-book response benchmark — buffered vs cursor streaming +//! (ROUND6). +//! +//! The REPORT path (addressbook-query, sync-collection) and the depth-1 +//! collection PROPFIND materialised EVERY contact DTO of the book in +//! one Vec, then rendered the complete multistatus into a second in-RAM +//! buffer — the book resident twice, TTFB = full generation. AFTER +//! streams ONE ordered scan (`full_name, first_name, last_name`, the +//! buffered listing's order) through a PG cursor and emits fixed-size +//! pages (contacts carry no bundling constraint). +//! +//! Drives the REAL repository + adapter writers both ways at the repo +//! layer (authz identical both sides, excluded). Gates: streamed +//! concatenation byte-identical to the buffered output for the REPORT +//! (getetag poll shape) AND the collection PROPFIND (allprop), seeded +//! with strictly distinct names so ordering is deterministic. +//! +//! Run (needs Postgres up; reads DATABASE_URL from .env): +//! cargo run --release --features bench --example bench_carddav_stream +//! Tunables (env): BENCH_CONTACTS (8000), BENCH_PAGE (500), BENCH_PASSES (9). + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::env; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::{Duration, Instant}; + +use oxicloud::application::adapters::carddav_adapter::{CardDavAdapter, CardDavReportType}; +use oxicloud::application::adapters::webdav_adapter::{ + PropFindRequest, PropFindType, QualifiedName, +}; +use oxicloud::application::dtos::address_book_dto::AddressBookDto; +use oxicloud::application::dtos::contact_dto::ContactDto; +use oxicloud::domain::repositories::contact_repository::ContactRepository; +use oxicloud::infrastructure::repositories::pg::ContactPgRepository; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; +use uuid::Uuid; + +// ─── Peak-live-heap tracking allocator ────────────────────────────────────── + +static LIVE: AtomicU64 = AtomicU64::new(0); +static PEAK: AtomicU64 = AtomicU64::new(0); + +struct PeakAlloc; + +fn bump(sz: u64) { + let live = LIVE.fetch_add(sz, Ordering::Relaxed) + sz; + PEAK.fetch_max(live, Ordering::Relaxed); +} + +unsafe impl GlobalAlloc for PeakAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + bump(layout.size() as u64); + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + LIVE.fetch_sub(layout.size() as u64, Ordering::Relaxed); + unsafe { System.dealloc(ptr, layout) } + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + if new_size > layout.size() { + bump((new_size - layout.size()) as u64); + } else { + LIVE.fetch_sub((layout.size() - new_size) as u64, Ordering::Relaxed); + } + unsafe { System.realloc(ptr, layout, new_size) } + } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + bump(layout.size() as u64); + unsafe { System.alloc_zeroed(layout) } + } +} + +#[global_allocator] +static GLOBAL: PeakAlloc = PeakAlloc; + +struct Seeded { + book_id: Uuid, + owner_id: Uuid, +} + +async fn seed(pool: &PgPool, n: usize) -> Seeded { + let owner_id: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_cardstream', 'bench_cardstream@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(pool) + .await + .expect("seed user"); + let book_id: Uuid = sqlx::query_scalar( + "INSERT INTO carddav.address_books (id, name, owner_id) + VALUES (gen_random_uuid(), 'Libreta grande', $1) RETURNING id", + ) + .bind(owner_id) + .fetch_one(pool) + .await + .expect("seed book"); + + let mut tx = pool.begin().await.expect("begin"); + for i in 0..n { + // Strictly distinct full_names keep the listing order (and thus + // the byte gate) deterministic. Every production row carries its + // full serialized vCard — the payload whose double-residency the + // streaming path removes — so the seed does too (~250 B each). + let uid = format!("contact-{i:06}"); + let vcard = format!( + "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:{uid}\r\nFN:Persona {i:06}\r\nN:Apellido{i};Nombre{i};;;\r\nEMAIL;TYPE=INTERNET:persona{i}@bench.invalid\r\nTEL;TYPE=CELL:+34 600 {i:06}\r\nORG:OxiCloud Bench\r\nNOTE:Fila sintetica del banco de pruebas CardDAV.\r\nEND:VCARD\r\n" + ); + sqlx::query( + "INSERT INTO carddav.contacts + (id, address_book_id, uid, full_name, first_name, last_name, vcard, etag) + VALUES (gen_random_uuid(), $1, $2, $3, $4, $5, $6, $7)", + ) + .bind(book_id) + .bind(&uid) + .bind(format!("Persona {i:06}")) + .bind(format!("Nombre{i}")) + .bind(format!("Apellido{i}")) + .bind(&vcard) + .bind(format!("{:016x}", (i as u64).wrapping_mul(2_654_435_761))) + .execute(&mut *tx) + .await + .expect("seed contact"); + } + tx.commit().await.expect("commit"); + Seeded { book_id, owner_id } +} + +async fn cleanup(pool: &PgPool, s: &Seeded) { + let _ = sqlx::query("DELETE FROM carddav.contacts WHERE address_book_id = $1") + .bind(s.book_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM carddav.address_books WHERE id = $1") + .bind(s.book_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.users WHERE id = $1") + .bind(s.owner_id) + .execute(pool) + .await; +} + +fn report_shape() -> CardDavReportType { + CardDavReportType::AddressbookQuery { + props: vec![ + QualifiedName::new("DAV:", "getetag"), + QualifiedName::new("DAV:", "getcontenttype"), + ], + } +} + +async fn fetch_all_dtos(repo: &ContactPgRepository, book_id: &Uuid) -> Vec { + repo.get_contacts_by_address_book(book_id) + .await + .expect("list contacts") + .into_iter() + .map(ContactDto::from) + .collect() +} + +/// BEFORE: full fetch + whole-response buffer. First byte exists only +/// when everything does. +async fn buffered_report( + repo: &ContactPgRepository, + book_id: &Uuid, + base_href: &str, +) -> (f64, Vec) { + let t0 = Instant::now(); + let contacts = fetch_all_dtos(repo, book_id).await; + let mut out = Vec::with_capacity(contacts.len() * 256); + CardDavAdapter::generate_contacts_response(&mut out, &contacts, &report_shape(), base_href) + .expect("generate"); + (t0.elapsed().as_secs_f64() * 1e3, out) +} + +/// AFTER: cursor + page writers (the handler loop over public pieces). +/// Returns (ttfb_ms — first data page rendered, wall_ms, bytes). +async fn streamed_report( + repo: &ContactPgRepository, + book_id: &Uuid, + base_href: &str, + page_rows: usize, + accumulate: bool, +) -> (f64, f64, Vec) { + use futures::TryStreamExt; + let t0 = Instant::now(); + let mut ttfb = None; + let mut all = Vec::new(); + let report = report_shape(); + + let mut chunk = Vec::with_capacity(160); + { + let mut w = quick_xml::Writer::new(&mut chunk); + CardDavAdapter::write_report_multistatus_start(&mut w).expect("start"); + } + if accumulate { + all.extend_from_slice(&chunk); + } + + let mut rows = repo.stream_contacts_by_book(*book_id); + let mut page: Vec = Vec::with_capacity(page_rows); + loop { + let next = rows + .try_next() + .await + .expect("stream row") + .map(ContactDto::from); + let flush = match &next { + Some(_) => page.len() >= page_rows, + None => !page.is_empty(), + }; + if flush { + let mut chunk = Vec::with_capacity(page.len() * 256 + 64); + { + let mut w = quick_xml::Writer::new(&mut chunk); + CardDavAdapter::write_contacts_report_page(&mut w, &page, &report, base_href) + .expect("page"); + } + ttfb.get_or_insert_with(|| t0.elapsed().as_secs_f64() * 1e3); + page.clear(); + if accumulate { + all.extend_from_slice(&chunk); + } + std::hint::black_box(&chunk); + } + match next { + Some(c) => page.push(c), + None => break, + } + } + + let mut chunk = Vec::with_capacity(32); + { + let mut w = quick_xml::Writer::new(&mut chunk); + CardDavAdapter::write_carddav_multistatus_end(&mut w).expect("end"); + } + if accumulate { + all.extend_from_slice(&chunk); + } + ( + ttfb.unwrap_or(f64::NAN), + t0.elapsed().as_secs_f64() * 1e3, + all, + ) +} + +fn p50(mut xs: Vec) -> f64 { + xs.sort_by(|a, b| a.partial_cmp(b).unwrap()); + xs[xs.len() / 2] +} + +fn reset_peak() { + PEAK.store(LIVE.load(Ordering::Relaxed), Ordering::Relaxed); +} + +fn peak_mib() -> f64 { + PEAK.load(Ordering::Relaxed) as f64 / (1024.0 * 1024.0) +} + +fn book_dto(seeded: &Seeded) -> AddressBookDto { + AddressBookDto { + id: seeded.book_id.to_string(), + name: "Libreta grande".to_string(), + owner_id: seeded.owner_id.to_string(), + ..AddressBookDto::default() + } +} + +#[tokio::main(flavor = "multi_thread")] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let n: usize = env::var("BENCH_CONTACTS") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(8000); + let page_rows: usize = env::var("BENCH_PAGE") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(500); + let passes: usize = env::var("BENCH_PASSES") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(9); + + let pool = Arc::new( + PgPoolOptions::new() + .max_connections(10) + .min_connections(10) + .acquire_timeout(Duration::from_secs(10)) + .connect(&url) + .await + .expect("connect Postgres"), + ); + + let seeded = seed(&pool, n).await; + let repo = ContactPgRepository::new(pool.clone()); + let base_href = format!("/carddav/{}/", seeded.book_id); + + println!("bench_carddav_stream — {n} contacts, page={page_rows}, {passes} passes\n"); + + // ── Equivalence gates ─────────────────────────────────────────────────── + let (_, before_bytes) = buffered_report(&repo, &seeded.book_id, &base_href).await; + let (_, _, after_bytes) = + streamed_report(&repo, &seeded.book_id, &base_href, page_rows, true).await; + let gate_report = before_bytes == after_bytes; + + // Collection PROPFIND (allprop): buffered generator vs head+pages. + let request = PropFindRequest { + prop_find_type: PropFindType::AllProp, + }; + let book = book_dto(&seeded); + let contacts_all = fetch_all_dtos(&repo, &seeded.book_id).await; + let mut coll_before = Vec::new(); + CardDavAdapter::generate_addressbook_collection_propfind( + &mut coll_before, + &book, + &contacts_all, + &request, + &base_href, + "1", + ) + .expect("collection"); + drop(contacts_all); + let coll_after = { + use futures::TryStreamExt; + let mut out = Vec::new(); + { + let mut w = quick_xml::Writer::new(&mut out); + CardDavAdapter::write_collection_head(&mut w, &book, &request, &base_href) + .expect("head"); + } + let mut rows = repo.stream_contacts_by_book(seeded.book_id); + let mut page: Vec = Vec::with_capacity(page_rows); + loop { + let next = rows + .try_next() + .await + .expect("stream row") + .map(ContactDto::from); + let flush = match &next { + Some(_) => page.len() >= page_rows, + None => !page.is_empty(), + }; + if flush { + let mut w = quick_xml::Writer::new(&mut out); + CardDavAdapter::write_collection_contact_page(&mut w, &page, &base_href) + .expect("page"); + page.clear(); + } + match next { + Some(c) => page.push(c), + None => break, + } + } + let mut w = quick_xml::Writer::new(&mut out); + CardDavAdapter::write_carddav_multistatus_end(&mut w).expect("end"); + out + }; + let gate_coll = coll_before == coll_after; + drop(coll_before); + drop(coll_after); + + // ── [1] REPORT timing + peak ──────────────────────────────────────────── + let mut b_wall = Vec::new(); + let mut a_wall = Vec::new(); + let mut a_ttfb = Vec::new(); + for _ in 0..passes { + let (w, out) = buffered_report(&repo, &seeded.book_id, &base_href).await; + std::hint::black_box(out); + b_wall.push(w); + let (t, w, _) = streamed_report(&repo, &seeded.book_id, &base_href, page_rows, false).await; + a_ttfb.push(t); + a_wall.push(w); + } + reset_peak(); + let (_, out) = buffered_report(&repo, &seeded.book_id, &base_href).await; + drop(out); + let peak_before = peak_mib(); + reset_peak(); + let _ = streamed_report(&repo, &seeded.book_id, &base_href, page_rows, false).await; + let peak_after = peak_mib(); + + let bw = p50(b_wall); + let aw = p50(a_wall); + let at = p50(a_ttfb); + println!("[1] REPORT addressbook-query (getetag) TTFB ms wall ms peak heap MiB"); + println!(" BEFORE (buffered) {bw:8.1} {bw:8.1} {peak_before:10.1}"); + println!( + " AFTER (cursor stream) {at:8.1} {aw:8.1} {peak_after:10.1} TTFB {:.1}x, heap {:.1}x lower", + bw / at, + peak_before / peak_after + ); + + cleanup(&pool, &seeded).await; + + println!( + "\n[gate] REPORT byte-identical: {} · collection PROPFIND byte-identical: {}", + if gate_report { "OK" } else { "FAILED" }, + if gate_coll { "OK" } else { "FAILED" } + ); + if !gate_report || !gate_coll { + std::process::exit(1); + } +} diff --git a/examples/bench_favorites_authz.rs b/examples/bench_favorites_authz.rs new file mode 100644 index 00000000..c97cce8f --- /dev/null +++ b/examples/bench_favorites_authz.rs @@ -0,0 +1,305 @@ +//! Batch-favorites AuthZ fan-out benchmark — serial `require` loop vs +//! `try_join_all`. +//! +//! VERDICT (round 6): the fan-out measured WORSE on both the cold and the +//! warm path against local-socket Postgres (see benches/ROUND6.md), so the +//! production loop stays serial. This example is kept as the reproducible +//! evidence for that rejection — re-run it if the DB ever moves behind real +//! network latency, where the answer could flip. +//! +//! `FavoritesService::batch_add_to_favorites` pre-checks `Permission::Read` +//! on every referenced resource. BEFORE awaited the checks one-by-one: for a +//! "select all → add to favorites" over N items whose drive-lookup isn't +//! cached yet, that is N sequential point-SELECT round-trips +//! (`drive_of` per distinct file) before the batched insert even starts. +//! AFTER fans the same checks out with `futures::future::try_join_all` +//! (fail-fast on any denial preserved). +//! +//! This bench drives the REAL `PgAclEngine` (owner/drive-role caches +//! included) against a seeded shared drive: +//! caller ──editor grant──▶ drive ─▶ root folder ─▶ N files +//! +//! Arms: cold engine (empty caches — the first-grid-load shape) and warm +//! repeat (all moka — parity check, both arms should collapse). +//! +//! Equivalence gates: every check grants for the member on both arms, and +//! both arms deny a control user with no grant. +//! +//! Run (needs Postgres up; reads DATABASE_URL from .env): +//! cargo run --release --features bench --example bench_favorites_authz +//! Tunables (env): BENCH_FILES (200), BENCH_POOL (20). + +use std::env; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use oxicloud::application::ports::authorization_ports::AuthorizationEngine; +use oxicloud::domain::services::authorization::{Permission, Resource, Subject}; +use oxicloud::infrastructure::repositories::pg::{ + FileBlobReadRepository, FolderDbRepository, SubjectGroupPgRepository, +}; +use oxicloud::infrastructure::services::dedup_service::DedupService; +use oxicloud::infrastructure::services::local_blob_backend::LocalBlobBackend; +use oxicloud::infrastructure::services::pg_acl_engine::PgAclEngine; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; +use uuid::Uuid; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +struct Seeded { + caller: Uuid, + control: Uuid, + drive_id: Uuid, + root_folder: Uuid, + blob_hash: String, + file_ids: Vec, +} + +async fn seed(pool: &PgPool, n_files: usize) -> Seeded { + let mut tx = pool.begin().await.expect("begin"); + let caller: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_favauthz', 'bench_favauthz@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed caller"); + let control: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_favauthz_ctl', 'bench_favauthz_ctl@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed control"); + + let drive_id: Uuid = + sqlx::query_scalar("INSERT INTO storage.drives (kind) VALUES ('shared') RETURNING id") + .fetch_one(&mut *tx) + .await + .expect("seed drive"); + let root_folder: Uuid = sqlx::query_scalar( + "INSERT INTO storage.folders (name, path, lpath, drive_id) + VALUES ('Bench Shared', '/Bench Shared', 'x', $1) RETURNING id", + ) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed folder"); + sqlx::query("UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2") + .bind(root_folder) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("stamp root"); + sqlx::query( + "INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', $1, 'drive', $2, 'editor'::storage.grant_role, $1)", + ) + .bind(caller) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("seed grant"); + + let blob_hash = "benchfavauthz0000000000000000000000000000000000000000000000000b1".to_string(); + sqlx::query("INSERT INTO storage.blobs (hash, size, ref_count) VALUES ($1, 1, 1)") + .bind(&blob_hash) + .execute(&mut *tx) + .await + .expect("seed blob"); + + let mut file_ids = Vec::with_capacity(n_files); + for i in 0..n_files { + let id: Uuid = sqlx::query_scalar( + "INSERT INTO storage.files (name, folder_id, blob_hash, size, mime_type, drive_id) + VALUES ($1, $2, $3, 1, 'text/plain', $4) RETURNING id", + ) + .bind(format!("bench-{i:04}.txt")) + .bind(root_folder) + .bind(&blob_hash) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed file"); + file_ids.push(id); + } + tx.commit().await.expect("commit"); + Seeded { + caller, + control, + drive_id, + root_folder, + blob_hash, + file_ids, + } +} + +async fn cleanup(pool: &PgPool, s: &Seeded) { + let _ = sqlx::query("DELETE FROM storage.role_grants WHERE resource_id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.files WHERE drive_id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.drives WHERE id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.folders WHERE id = $1") + .bind(s.root_folder) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.blobs WHERE hash = $1") + .bind(&s.blob_hash) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.users WHERE id IN ($1, $2)") + .bind(s.caller) + .bind(s.control) + .execute(pool) + .await; +} + +fn fresh_engine(pool: &Arc) -> Arc { + let folder_repo = Arc::new(FolderDbRepository::new(pool.clone())); + let backend = Arc::new(LocalBlobBackend::new(std::path::Path::new( + "/tmp/bench-favauthz-blobs", + ))); + let dedup = Arc::new(DedupService::new(backend, pool.clone(), pool.clone())); + let file_repo = Arc::new(FileBlobReadRepository::new( + pool.clone(), + dedup, + folder_repo.clone(), + )); + let group_repo = Arc::new(SubjectGroupPgRepository::new(pool.clone())); + Arc::new(PgAclEngine::new( + pool.clone(), + folder_repo, + file_repo, + group_repo, + )) +} + +/// BEFORE, verbatim shape: one awaited `require` per item. +async fn serial_checks(engine: &Arc, user: Uuid, files: &[Uuid]) -> Result<(), ()> { + for id in files { + engine + .require(Subject::User(user), Permission::Read, Resource::File(*id)) + .await + .map_err(|_| ())?; + } + Ok(()) +} + +/// AFTER: the same checks, fanned out with fail-fast join. +async fn joined_checks(engine: &Arc, user: Uuid, files: &[Uuid]) -> Result<(), ()> { + futures::future::try_join_all( + files + .iter() + .map(|id| engine.require(Subject::User(user), Permission::Read, Resource::File(*id))), + ) + .await + .map(|_| ()) + .map_err(|_| ()) +} + +#[tokio::main(flavor = "multi_thread")] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let n_files: usize = env_or("BENCH_FILES", 200); + let pool_size: u32 = env_or("BENCH_POOL", 20); + + let pool = Arc::new( + PgPoolOptions::new() + .max_connections(pool_size) + .min_connections(pool_size) + .acquire_timeout(Duration::from_secs(10)) + .connect(&url) + .await + .expect("connect Postgres"), + ); + + let seeded = seed(&pool, n_files).await; + + // ── Equivalence gates ──────────────────────────────────────────────── + // Grant path: both arms must authorize every file for the member. + let gate_engine = fresh_engine(&pool); + if serial_checks(&gate_engine, seeded.caller, &seeded.file_ids) + .await + .is_err() + || joined_checks(&gate_engine, seeded.caller, &seeded.file_ids) + .await + .is_err() + { + eprintln!("EQUIVALENCE GATE FAILED: member was denied"); + cleanup(&pool, &seeded).await; + std::process::exit(1); + } + // Denial path: both arms must reject the control user (fresh engines so + // the joined arm can't ride the serial arm's caches). + let deny_a = fresh_engine(&pool); + let deny_b = fresh_engine(&pool); + if serial_checks(&deny_a, seeded.control, &seeded.file_ids) + .await + .is_ok() + || joined_checks(&deny_b, seeded.control, &seeded.file_ids) + .await + .is_ok() + { + eprintln!("EQUIVALENCE GATE FAILED: control user was granted"); + cleanup(&pool, &seeded).await; + std::process::exit(1); + } + + println!("\n#################################################################"); + println!("# batch-favorites authz: serial require loop vs try_join_all"); + println!("# files={n_files} pool={pool_size} (shared-drive member, editor grant)"); + println!("#################################################################\n"); + println!("| {:<18} | {:>10} | {:>12} |", "arm", "wall ms", "µs/item"); + + for (label, joined, warm) in [ + ("serial COLD", false, false), + ("join COLD", true, false), + ("serial WARM", false, true), + ("join WARM", true, true), + ] { + // COLD: fresh engine per run (empty moka). WARM: prime, then measure. + let engine = fresh_engine(&pool); + if warm { + serial_checks(&engine, seeded.caller, &seeded.file_ids) + .await + .expect("prime"); + } + let t = Instant::now(); + let r = if joined { + joined_checks(&engine, seeded.caller, &seeded.file_ids).await + } else { + serial_checks(&engine, seeded.caller, &seeded.file_ids).await + }; + let el = t.elapsed(); + r.expect("granted"); + println!( + "| {:<18} | {:>10.2} | {:>12.2} |", + label, + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / n_files as f64 + ); + } + + cleanup(&pool, &seeded).await; + println!("\n(COLD = empty caches: N distinct `drive_of` point-SELECTs — the arm"); + println!(" under test. WARM = all-moka parity check. Fail-fast denial semantics"); + println!(" verified by the control-user gate on both arms.)"); +} diff --git a/examples/bench_hex_ids.rs b/examples/bench_hex_ids.rs new file mode 100644 index 00000000..e7840da7 --- /dev/null +++ b/examples/bench_hex_ids.rs @@ -0,0 +1,226 @@ +//! Micro-alloc benchmark: digest-hex rendering and NC id-batch marshalling. +//! +//! Two round-6 changes, both equivalence-gated against their verbatim +//! BEFORE shapes and measured with a counting allocator: +//! +//! 1. `IncrementalHasher::finalize_hex` (upload_ingest.rs) rendered MD5 / +//! SHA-256 digests with `.map(|b| format!("{b:02x}")).collect()` — one +//! heap `String` per digest byte (16 / 32 allocs) per chunk finalize. +//! AFTER: `common::fmt::hex_lower` writes into one preallocated String. +//! +//! 2. `batch_resolve_ids` (NC webdav_handler) cloned every child id into a +//! `Vec` and the id service keyed its result map by `String` — +//! ~3 heap allocs per child per page. AFTER the whole chain is borrowed: +//! `Vec<&str>` in, `HashMap` out, `Uuid::parse_str` lookups. +//! +//! Run: +//! cargo run --release --features bench --example bench_hex_ids +//! Tunables (env): BENCH_ITERS (10000), BENCH_CHILDREN (500). + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::collections::HashMap; +use std::env; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use md5::Digest; +use oxicloud::common::fmt::hex_lower; +use uuid::Uuid; + +static ALLOC_CALLS: AtomicU64 = AtomicU64::new(0); + +struct CountingAlloc; + +unsafe impl GlobalAlloc for CountingAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.realloc(ptr, layout, new_size) } + } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc_zeroed(layout) } + } +} + +#[global_allocator] +static GLOBAL: CountingAlloc = CountingAlloc; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn measure(f: impl FnOnce() -> R) -> (R, u64, f64) { + let a0 = ALLOC_CALLS.load(Ordering::Relaxed); + let t = Instant::now(); + let r = f(); + let el = t.elapsed().as_secs_f64(); + let allocs = ALLOC_CALLS.load(Ordering::Relaxed) - a0; + (r, allocs, el) +} + +// ── 1. digest hex ─────────────────────────────────────────────────────────── + +/// BEFORE, verbatim: one `format!` per digest byte. +fn hex_before(bytes: &[u8]) -> String { + bytes.iter().map(|b| format!("{b:02x}")).collect() +} + +fn bench_hex(iters: usize) { + // Deterministic digests of both production sizes (MD5=16, SHA-256=32). + let md5s: Vec<[u8; 16]> = (0..64u64) + .map(|i| md5::Md5::digest(i.to_le_bytes()).into()) + .collect(); + let sha256s: Vec<[u8; 32]> = (0..64u64) + .map(|i| sha2::Sha256::digest(i.to_le_bytes()).into()) + .collect(); + + // Equivalence gate: byte-identical output on every digest. + for d in &md5s { + assert_eq!(hex_lower(d), hex_before(d), "md5 hex mismatch"); + } + for d in &sha256s { + assert_eq!(hex_lower(d), hex_before(d), "sha256 hex mismatch"); + } + + println!("── finalize_hex: per-byte format! vs hex_lower ({iters} finalizes/arm) ──\n"); + println!( + "| {:<8} | {:<8} | {:>12} | {:>10} | {:>12} |", + "digest", "arm", "allocs", "wall ms", "allocs/call" + ); + for (label, digests) in [("md5", md5s.len()), ("sha256", sha256s.len())] { + for arm in ["BEFORE", "AFTER"] { + let (sink, allocs, secs) = measure(|| { + let mut sink = 0usize; + for i in 0..iters { + let s = match (label, arm) { + ("md5", "BEFORE") => hex_before(&md5s[i % digests]), + ("md5", "AFTER") => hex_lower(&md5s[i % digests]), + ("sha256", "BEFORE") => hex_before(&sha256s[i % digests]), + _ => hex_lower(&sha256s[i % digests]), + }; + sink += s.len(); + } + sink + }); + std::hint::black_box(sink); + println!( + "| {:<8} | {:<8} | {:>12} | {:>10.2} | {:>12.2} |", + label, + arm, + allocs, + secs * 1e3, + allocs as f64 / iters as f64 + ); + } + } +} + +// ── 2. NC id-batch marshalling ────────────────────────────────────────────── + +/// BEFORE, verbatim caller+service marshalling: clone ids into `Vec`, +/// key the result map by cloned `String`, look children up by `&String`. +fn ids_before(child_ids: &[String], nc: &HashMap) -> Vec> { + let file_uuids: Vec = child_ids.to_vec(); + let mut map: HashMap = HashMap::with_capacity(file_uuids.len()); + for raw in &file_uuids { + let Ok(uuid) = Uuid::parse_str(raw) else { + continue; + }; + if let Some(id) = nc.get(&uuid) { + map.insert(raw.clone(), *id); + } + } + child_ids.iter().map(|id| map.get(id).copied()).collect() +} + +/// AFTER: borrowed slice in, `Uuid`-keyed map out, parse-and-get lookups — +/// the exact shapes now in `batch_resolve_ids` + `nc_id_of`. +fn ids_after(child_ids: &[String], nc: &HashMap) -> Vec> { + let file_uuids: Vec<&str> = child_ids.iter().map(String::as_str).collect(); + let mut map: HashMap = HashMap::with_capacity(file_uuids.len()); + for raw in &file_uuids { + let Ok(uuid) = Uuid::parse_str(raw) else { + continue; + }; + if let Some(id) = nc.get(&uuid) { + map.insert(uuid, *id); + } + } + child_ids + .iter() + .map(|id| Uuid::parse_str(id).ok().and_then(|u| map.get(&u).copied())) + .collect() +} + +fn bench_ids(pages: usize, children: usize) { + // A PROPFIND page of `children` DTO ids (36-byte uuid strings) resolved + // against the id service's numeric mapping. + let uuids: Vec = (0..children).map(|_| Uuid::new_v4()).collect(); + let child_ids: Vec = uuids.iter().map(|u| u.to_string()).collect(); + let nc: HashMap = uuids + .iter() + .enumerate() + .map(|(i, u)| (*u, i as i64 + 1000)) + .collect(); + + // Equivalence gate: identical per-child resolution, including an + // unparseable id and an unmapped-but-valid id. + let mut gate_ids = child_ids.clone(); + gate_ids.push("not-a-uuid".to_string()); + gate_ids.push(Uuid::new_v4().to_string()); + assert_eq!( + ids_before(&gate_ids, &nc), + ids_after(&gate_ids, &nc), + "id resolution mismatch" + ); + + println!("\n── batch_resolve_ids marshalling: String-keyed vs borrowed+Uuid ──"); + println!(" ({pages} pages × {children} children/arm)\n"); + println!( + "| {:<8} | {:>12} | {:>10} | {:>14} |", + "arm", "allocs", "wall ms", "allocs/child" + ); + for arm in ["BEFORE", "AFTER"] { + let (sink, allocs, secs) = measure(|| { + let mut sink = 0usize; + for _ in 0..pages { + let resolved = if arm == "BEFORE" { + ids_before(&child_ids, &nc) + } else { + ids_after(&child_ids, &nc) + }; + sink += resolved.iter().flatten().count(); + } + sink + }); + assert_eq!(sink, pages * children, "all children must resolve"); + println!( + "| {:<8} | {:>12} | {:>10.2} | {:>14.3} |", + arm, + allocs, + secs * 1e3, + allocs as f64 / (pages * children) as f64 + ); + } +} + +fn main() { + let iters: usize = env_or("BENCH_ITERS", 10_000); + let children: usize = env_or("BENCH_CHILDREN", 500); + + bench_hex(iters); + bench_ids(iters / 10, children); + + println!("\n(BEFORE arms are verbatim replicas of the replaced shapes; equivalence"); + println!(" asserted before timing. Allocs counted via a wrapping GlobalAlloc.)"); +} diff --git a/examples/bench_uuid_text_cast.rs b/examples/bench_uuid_text_cast.rs new file mode 100644 index 00000000..196cf849 --- /dev/null +++ b/examples/bench_uuid_text_cast.rs @@ -0,0 +1,248 @@ +//! A/B: `id::text` server-side casts vs binary UUID decode + app-side format. +//! +//! `file_blob_read_repository.rs` (and friends) SELECT UUID columns as +//! `id::text` and decode `String`s directly. The alternative is to decode the +//! wire-native binary `Uuid` (16 bytes vs 36 on the wire) and render the +//! string app-side with `Uuid::to_string`. This bench decides ROUND6 task +//! "::text casts A/B" empirically: whichever loses is documented, only a +//! winner ships. +//! +//! Arms fetch the same 500-row page from a seeded `storage.files` subtree, +//! interleaved A/B to cancel drift; the equivalence gate asserts identical +//! `(id, folder_id, name)` string triples. +//! +//! Run (needs Postgres up; reads DATABASE_URL from .env): +//! cargo run --release --features bench --example bench_uuid_text_cast +//! Tunables (env): BENCH_ROWS (500), BENCH_PASSES (200). + +use std::env; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use sqlx::postgres::PgPoolOptions; +use sqlx::{PgPool, Row}; +use uuid::Uuid; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +struct Seeded { + drive_id: Uuid, + root_folder: Uuid, + blob_hash: String, +} + +async fn seed(pool: &PgPool, rows: usize) -> Seeded { + let mut tx = pool.begin().await.expect("begin"); + let drive_id: Uuid = + sqlx::query_scalar("INSERT INTO storage.drives (kind) VALUES ('shared') RETURNING id") + .fetch_one(&mut *tx) + .await + .expect("seed drive"); + let root_folder: Uuid = sqlx::query_scalar( + "INSERT INTO storage.folders (name, path, lpath, drive_id) + VALUES ('Bench Cast', '/Bench Cast', 'x', $1) RETURNING id", + ) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed folder"); + sqlx::query("UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2") + .bind(root_folder) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("stamp root"); + let blob_hash = "benchuuidcast000000000000000000000000000000000000000000000000b2".to_string(); + sqlx::query("INSERT INTO storage.blobs (hash, size, ref_count) VALUES ($1, 1, 1)") + .bind(&blob_hash) + .execute(&mut *tx) + .await + .expect("seed blob"); + for i in 0..rows { + sqlx::query( + "INSERT INTO storage.files (name, folder_id, blob_hash, size, mime_type, drive_id) + VALUES ($1, $2, $3, 1, 'text/plain', $4)", + ) + .bind(format!("cast-{i:05}.txt")) + .bind(root_folder) + .bind(&blob_hash) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("seed file"); + } + tx.commit().await.expect("commit"); + Seeded { + drive_id, + root_folder, + blob_hash, + } +} + +async fn cleanup(pool: &PgPool, s: &Seeded) { + let _ = sqlx::query("DELETE FROM storage.files WHERE drive_id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.drives WHERE id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.folders WHERE id = $1") + .bind(s.root_folder) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.blobs WHERE hash = $1") + .bind(&s.blob_hash) + .execute(pool) + .await; +} + +type Triple = (String, Option, String); + +/// Arm A — the current production shape: server-side `::text` casts. +async fn fetch_text_cast(pool: &PgPool, drive_id: Uuid) -> Vec { + sqlx::query( + "SELECT id::text AS id, folder_id::text AS folder_id, name + FROM storage.files WHERE drive_id = $1 ORDER BY name", + ) + .bind(drive_id) + .fetch_all(pool) + .await + .expect("text-cast fetch") + .iter() + .map(|r| { + ( + r.get::("id"), + r.get::, _>("folder_id"), + r.get::("name"), + ) + }) + .collect() +} + +/// Arm B — binary `Uuid` decode + app-side `to_string`. +async fn fetch_binary_uuid(pool: &PgPool, drive_id: Uuid) -> Vec { + sqlx::query( + "SELECT id, folder_id, name + FROM storage.files WHERE drive_id = $1 ORDER BY name", + ) + .bind(drive_id) + .fetch_all(pool) + .await + .expect("binary fetch") + .iter() + .map(|r| { + ( + r.get::("id").to_string(), + r.get::, _>("folder_id").map(|u| u.to_string()), + r.get::("name"), + ) + }) + .collect() +} + +struct Stats { + mean_ms: f64, + p50_ms: f64, + p95_ms: f64, +} + +fn summarize(mut xs: Vec) -> Stats { + xs.sort_by(|a, b| a.partial_cmp(b).unwrap()); + let n = xs.len(); + Stats { + mean_ms: xs.iter().sum::() / n as f64, + p50_ms: xs[n / 2], + p95_ms: xs[((n as f64 * 0.95) as usize).min(n - 1)], + } +} + +#[tokio::main(flavor = "multi_thread")] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let rows: usize = env_or("BENCH_ROWS", 500); + let passes: usize = env_or("BENCH_PASSES", 200); + + let pool = Arc::new( + PgPoolOptions::new() + .max_connections(4) + .min_connections(4) + .acquire_timeout(Duration::from_secs(10)) + .connect(&url) + .await + .expect("connect Postgres"), + ); + + let seeded = seed(&pool, rows).await; + + // ── Equivalence gate: identical string triples ─────────────────────── + let a = fetch_text_cast(&pool, seeded.drive_id).await; + let b = fetch_binary_uuid(&pool, seeded.drive_id).await; + if a != b || a.len() != rows { + eprintln!( + "EQUIVALENCE GATE FAILED: rows differ (a={}, b={})", + a.len(), + b.len() + ); + cleanup(&pool, &seeded).await; + std::process::exit(1); + } + + // Warm-up both shapes (plan cache, buffer cache). + for _ in 0..10 { + std::hint::black_box(fetch_text_cast(&pool, seeded.drive_id).await); + std::hint::black_box(fetch_binary_uuid(&pool, seeded.drive_id).await); + } + + // Interleaved A/B passes so drift (autovacuum, CPU governor) hits both. + let mut lat_a = Vec::with_capacity(passes); + let mut lat_b = Vec::with_capacity(passes); + for _ in 0..passes { + let t = Instant::now(); + std::hint::black_box(fetch_text_cast(&pool, seeded.drive_id).await); + lat_a.push(t.elapsed().as_secs_f64() * 1e3); + let t = Instant::now(); + std::hint::black_box(fetch_binary_uuid(&pool, seeded.drive_id).await); + lat_b.push(t.elapsed().as_secs_f64() * 1e3); + } + + let sa = summarize(lat_a); + let sb = summarize(lat_b); + + println!("\n#################################################################"); + println!("# UUID columns: `id::text` server cast vs binary decode + app fmt"); + println!("# rows/page={rows} passes={passes} (interleaved)"); + println!("#################################################################\n"); + println!( + "| {:<22} | {:>9} | {:>9} | {:>9} |", + "arm", "mean ms", "p50 ms", "p95 ms" + ); + println!( + "| {:<22} | {:>9.3} | {:>9.3} | {:>9.3} |", + "A ::text (current)", sa.mean_ms, sa.p50_ms, sa.p95_ms + ); + println!( + "| {:<22} | {:>9.3} | {:>9.3} | {:>9.3} |", + "B binary + to_string", sb.mean_ms, sb.p50_ms, sb.p95_ms + ); + println!( + "\nB/A mean ratio: {:.3} ({})", + sb.mean_ms / sa.mean_ms, + if sb.mean_ms < sa.mean_ms { + "binary decode wins" + } else { + "::text cast wins" + } + ); + + cleanup(&pool, &seeded).await; +} diff --git a/src/application/adapters/carddav_adapter.rs b/src/application/adapters/carddav_adapter.rs index b48eac62..4e3f224f 100644 --- a/src/application/adapters/carddav_adapter.rs +++ b/src/application/adapters/carddav_adapter.rs @@ -278,6 +278,27 @@ impl CardDavAdapter { ) -> Result<()> { let mut xml_writer = Writer::new(writer); + Self::write_collection_head(&mut xml_writer, address_book, request, base_href)?; + + // Write contacts if depth > 0 + if depth != "0" { + Self::write_collection_contact_page(&mut xml_writer, contacts, base_href)?; + } + + Self::write_carddav_multistatus_end(&mut xml_writer) + } + + /// Multistatus opening (DAV + CardDAV + CalendarServer namespaces) + /// plus the address book's own `D:response` — the head of a depth-1 + /// collection PROPFIND. Streaming emitters call this once, then + /// [`Self::write_collection_contact_page`] per cursor page, then + /// [`Self::write_carddav_multistatus_end`]. + pub fn write_collection_head( + xml_writer: &mut Writer, + address_book: &AddressBookDto, + request: &PropFindRequest, + base_href: &str, + ) -> Result<()> { xml_writer.write_event(Event::Start( BytesStart::new("D:multistatus").with_attributes([ ("xmlns:D", "DAV:"), @@ -285,19 +306,25 @@ impl CardDavAdapter { ("xmlns:CS", "http://calendarserver.org/ns/"), ]), ))?; + Self::write_addressbook_response(xml_writer, address_book, request, base_href) + } - // Write the address book itself - Self::write_addressbook_response(&mut xml_writer, address_book, request, base_href)?; - - // Write contacts if depth > 0 - if depth != "0" { - for contact in contacts { - let contact_href = format!("{}{}.vcf", base_href, contact.uid); - Self::write_contact_response(&mut xml_writer, contact, &[], &contact_href)?; - } + /// One depth-1 collection page of contact entries (standard props; + /// href buffer reused across the page). + pub fn write_collection_contact_page( + xml_writer: &mut Writer, + contacts: &[ContactDto], + base_href: &str, + ) -> Result<()> { + let mut href = String::with_capacity(base_href.len() + 48); + for contact in contacts { + href.clear(); + let _ = std::fmt::Write::write_fmt( + &mut href, + format_args!("{}{}.vcf", base_href, contact.uid), + ); + Self::write_contact_response(xml_writer, contact, &[], &href)?; } - - xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?; Ok(()) } @@ -648,32 +675,39 @@ impl CardDavAdapter { } /// Generate response for contacts (for REPORT) - pub fn generate_contacts_response( - writer: W, - contacts: &[ContactDto], - report: &CardDavReportType, - base_href: &str, - ) -> Result<()> { - let mut xml_writer = Writer::new(writer); - + /// REPORT `` opening tag (DAV + CardDAV namespaces). + /// Streaming emitters call this once, then + /// [`Self::write_contacts_report_page`] per cursor page, then + /// [`Self::write_carddav_multistatus_end`]. + pub fn write_report_multistatus_start(xml_writer: &mut Writer) -> Result<()> { xml_writer.write_event(Event::Start( BytesStart::new("D:multistatus").with_attributes([ ("xmlns:D", "DAV:"), ("xmlns:CR", "urn:ietf:params:xml:ns:carddav"), ]), ))?; + Ok(()) + } - // Borrowed straight out of the request — the old `clone()` copied - // the whole Vec of owned QualifiedName strings per REPORT (same - // fix the CalDAV surface got in ROUND4). + /// Close a multistatus opened by either start writer. + pub fn write_carddav_multistatus_end(xml_writer: &mut Writer) -> Result<()> { + xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?; + Ok(()) + } + + /// One REPORT page of contact responses. Props are borrowed from + /// the request; one href buffer is reused across the page. + pub fn write_contacts_report_page( + xml_writer: &mut Writer, + contacts: &[ContactDto], + report: &CardDavReportType, + base_href: &str, + ) -> Result<()> { let props = match report { CardDavReportType::AddressbookQuery { props } => props, CardDavReportType::AddressbookMultiget { props, .. } => props, CardDavReportType::SyncCollection { props, .. } => props, }; - - // One reused href buffer for the whole listing instead of a - // fresh String per contact. let mut href = String::with_capacity(base_href.len() + 48); for contact in contacts { href.clear(); @@ -683,13 +717,23 @@ impl CardDavAdapter { ); // `write_contact_response` generates the vCard on demand when (and // only when) address-data is actually requested. - Self::write_contact_response(&mut xml_writer, contact, props, &href)?; + Self::write_contact_response(xml_writer, contact, props, &href)?; } - - xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?; Ok(()) } + pub fn generate_contacts_response( + writer: W, + contacts: &[ContactDto], + report: &CardDavReportType, + base_href: &str, + ) -> Result<()> { + let mut xml_writer = Writer::new(writer); + Self::write_report_multistatus_start(&mut xml_writer)?; + Self::write_contacts_report_page(&mut xml_writer, contacts, report, base_href)?; + Self::write_carddav_multistatus_end(&mut xml_writer) + } + /// Write a single contact response element fn write_contact_response( xml_writer: &mut Writer, diff --git a/src/application/ports/carddav_ports.rs b/src/application/ports/carddav_ports.rs index 6a638a2c..dcb431be 100644 --- a/src/application/ports/carddav_ports.rs +++ b/src/application/ports/carddav_ports.rs @@ -66,6 +66,12 @@ pub trait ContactStoragePort: Send + Sync + 'static { &self, address_book_id: &Uuid, ) -> Result, DomainError>; + /// Cursor stream over the book's contacts in listing order — feeds + /// the streaming CardDAV emitters. + fn stream_contacts_by_book( + &self, + address_book_id: Uuid, + ) -> futures::stream::BoxStream<'static, Result>; async fn get_contacts_by_address_book_paginated( &self, address_book_id: &Uuid, @@ -174,6 +180,15 @@ pub trait ContactUseCase: Send + Sync + 'static { /// List contacts in an address book. `limit`/`offset` bound the /// result for paginated callers (REST API); `None` returns the full /// book, which the CardDAV listing/sync paths rely on. + /// Streaming support: cursor over the book's contacts (same Read + /// gate as [`Self::list_contacts`], checked once before the cursor + /// opens). + async fn stream_contacts_by_book( + &self, + address_book_id: &str, + user_id: Uuid, + ) -> Result>, DomainError>; + async fn list_contacts( &self, address_book_id: &str, diff --git a/src/application/services/contact_service.rs b/src/application/services/contact_service.rs index ef70b912..d7eb6992 100644 --- a/src/application/services/contact_service.rs +++ b/src/application/services/contact_service.rs @@ -825,6 +825,25 @@ impl ContactUseCase for ContactService { Ok(contacts.into_iter().map(ContactDto::from).collect()) } + async fn stream_contacts_by_book( + &self, + address_book_id: &str, + user_id: Uuid, + ) -> Result>, DomainError> + { + use futures::StreamExt; + let id = Uuid::parse_str(address_book_id) + .map_err(|_| DomainError::validation_error("Invalid address book ID format"))?; + // Same Read gate as `list_contacts`, once, before the cursor. + self.require_address_book_read_or_public(&id, &user_id) + .await?; + Ok(Box::pin( + self.contact_storage + .stream_contacts_by_book(id) + .map(|r| r.map(ContactDto::from)), + )) + } + async fn list_contacts( &self, address_book_id: &str, diff --git a/src/application/services/favorites_service.rs b/src/application/services/favorites_service.rs index ac372436..98573daa 100644 --- a/src/application/services/favorites_service.rs +++ b/src/application/services/favorites_service.rs @@ -145,6 +145,12 @@ impl FavoritesUseCase for FavoritesService { // valid (partial success would leak the same oracle we // closed on the single-item path). See // `docs/plan/authz_audit/rest_storage.md`. + // + // Deliberately serial: a `try_join_all` fan-out measured WORSE + // on both the cold (drive_of point-SELECTs) and warm (all-moka) + // paths — future orchestration + pool-acquire contention cost + // more than the local round trips they overlap. Rejected by + // `bench_favorites_authz`; numbers in benches/ROUND6.md. for (item_id, item_type) in items { let resource = Resource::parse(item_type, item_id)?; self.authorization diff --git a/src/application/services/nextcloud_file_id_service.rs b/src/application/services/nextcloud_file_id_service.rs index 2190e7f9..bce7a939 100644 --- a/src/application/services/nextcloud_file_id_service.rs +++ b/src/application/services/nextcloud_file_id_service.rs @@ -41,55 +41,50 @@ impl NextcloudFileIdService { /// Resolve — creating when absent — stable numeric file IDs for many /// UUIDs at once. Cache hits cost nothing; the misses are resolved with a - /// single backing query. The returned map is keyed by the caller's - /// original id strings; unresolvable inputs are simply absent (mirroring - /// the `.ok()` behaviour the callers relied on). - pub async fn get_or_create_file_ids( - &self, - file_ids: &[String], - ) -> Result> { + /// single backing query. The returned map is keyed by parsed UUID; + /// unparseable/unresolvable inputs are simply absent (mirroring the + /// `.ok()` behaviour the callers relied on). + pub async fn get_or_create_file_ids(&self, file_ids: &[&str]) -> Result> { self.get_or_create_many("file", file_ids).await } /// Folder counterpart of [`Self::get_or_create_file_ids`]. pub async fn get_or_create_folder_ids( &self, - folder_ids: &[String], - ) -> Result> { + folder_ids: &[&str], + ) -> Result> { self.get_or_create_many("folder", folder_ids).await } async fn get_or_create_many( &self, object_type: &str, - raw_ids: &[String], - ) -> Result> { + raw_ids: &[&str], + ) -> Result> { let mut result = HashMap::with_capacity(raw_ids.len()); - // Parsed-UUID → caller's original string; also dedupes the miss list. - let mut pending: HashMap = HashMap::new(); + let mut misses: Vec = Vec::new(); for raw in raw_ids { let Ok(uuid) = Uuid::parse_str(raw) else { continue; // Unparseable ids never had a mapping — skip silently. }; if let Some(id) = self.cache.get(&uuid).await { - result.insert(raw.clone(), id); + result.insert(uuid, id); } else { - pending.entry(uuid).or_insert_with(|| raw.clone()); + misses.push(uuid); } } - if !pending.is_empty() { - let misses: Vec = pending.keys().copied().collect(); + if !misses.is_empty() { + misses.sort_unstable(); + misses.dedup(); let resolved = self .repo()? .get_or_create_many(object_type, &misses) .await?; for (uuid, id) in resolved { self.cache.insert(uuid, id).await; - if let Some(original) = pending.get(&uuid) { - result.insert(original.clone(), id); - } + result.insert(uuid, id); } } @@ -184,10 +179,7 @@ mod tests { #[tokio::test] async fn test_get_or_create_file_ids_skips_unparseable() { let svc = NextcloudFileIdService::new_stub(); - let map = svc - .get_or_create_file_ids(&["not-a-uuid".to_string()]) - .await - .unwrap(); + let map = svc.get_or_create_file_ids(&["not-a-uuid"]).await.unwrap(); assert!(map.is_empty()); } } diff --git a/src/common/fmt.rs b/src/common/fmt.rs index 7e70fa8e..4ab8e307 100644 --- a/src/common/fmt.rs +++ b/src/common/fmt.rs @@ -170,11 +170,43 @@ pub fn i64_str(buf: &mut [u8; 21], v: i64) -> &str { std::str::from_utf8(&buf[start..]).expect("ascii") } +/// Lower-case hex of `bytes` into one preallocated `String`. +/// +/// Replaces the `.map(|b| format!("{b:02x}")).collect()` shape, which heap- +/// allocates a 2-byte `String` per digest byte (16 for MD5, 32 for SHA-256) +/// before collect concatenates them. +pub fn hex_lower(bytes: &[u8]) -> String { + const HEX: &[u8; 16] = b"0123456789abcdef"; + let mut out = String::with_capacity(bytes.len() * 2); + for &b in bytes { + out.push(HEX[(b >> 4) as usize] as char); + out.push(HEX[(b & 0x0f) as usize] as char); + } + out +} + #[cfg(test)] mod tests { use super::*; use chrono::{TimeZone, Utc}; + /// `hex_lower` must match the `format!("{b:02x}")`-per-byte shape it + /// replaced, byte for byte. + #[test] + fn hex_lower_matches_format() { + let cases: [&[u8]; 5] = [ + &[], + &[0x00], + &[0xff, 0x00, 0xab], + &(0u8..=255).collect::>(), + b"The quick brown fox", + ]; + for bytes in cases { + let reference: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!(hex_lower(bytes), reference); + } + } + /// Edge-heavy corpus: epoch, single-digit day (padding!), leap day, /// end-of-year, DST-irrelevant midsummer, far future, max in-range. const CASES: [i64; 12] = [ diff --git a/src/domain/repositories/contact_repository.rs b/src/domain/repositories/contact_repository.rs index 01aae004..fff64646 100644 --- a/src/domain/repositories/contact_repository.rs +++ b/src/domain/repositories/contact_repository.rs @@ -25,6 +25,14 @@ pub trait ContactRepository: Send + Sync + 'static { address_book_id: &Uuid, uids: &[String], ) -> ContactRepositoryResult>; + /// Cursor stream over every contact of the book in the listing + /// order (`full_name, first_name, last_name`) — ONE scan+sort on + /// the server; the streaming CardDAV emitters page over it. + fn stream_contacts_by_book( + &self, + address_book_id: Uuid, + ) -> futures::stream::BoxStream<'static, ContactRepositoryResult>; + async fn get_contacts_by_address_book( &self, address_book_id: &Uuid, diff --git a/src/infrastructure/adapters/contact_storage_adapter.rs b/src/infrastructure/adapters/contact_storage_adapter.rs index 5af4dd89..b18a76d0 100644 --- a/src/infrastructure/adapters/contact_storage_adapter.rs +++ b/src/infrastructure/adapters/contact_storage_adapter.rs @@ -146,6 +146,14 @@ impl ContactStoragePort for ContactStorageAdapter { .await } + fn stream_contacts_by_book( + &self, + address_book_id: Uuid, + ) -> futures::stream::BoxStream<'static, Result> { + self.contact_repository + .stream_contacts_by_book(address_book_id) + } + async fn get_contacts_by_address_book_paginated( &self, address_book_id: &Uuid, diff --git a/src/infrastructure/repositories/pg/contact_pg_repository.rs b/src/infrastructure/repositories/pg/contact_pg_repository.rs index 02ea9008..45850e16 100644 --- a/src/infrastructure/repositories/pg/contact_pg_repository.rs +++ b/src/infrastructure/repositories/pg/contact_pg_repository.rs @@ -278,6 +278,45 @@ impl ContactRepository for ContactPgRepository { Ok(contacts) } + fn stream_contacts_by_book( + &self, + address_book_id: Uuid, + ) -> futures::stream::BoxStream<'static, ContactRepositoryResult> { + // ONE ordered scan served through a PG cursor — the CardDAV + // multistatus emitters page over this stream so only a page of + // contacts is resident (same design as the CalDAV round-5 + // cursor; contacts have no master/exception bundling, so pages + // can cut anywhere). + let pool = self.pool.clone(); + let stream: futures::stream::BoxStream<'static, ContactRepositoryResult> = + Box::pin(async_stream::try_stream! { + let mut conn = pool.acquire().await.map_err(|e| { + DomainError::database_error(format!("Failed to acquire connection: {}", e)) + })?; + let mut rows = sqlx::query( + r#" + SELECT + id, address_book_id, uid, full_name, first_name, last_name, nickname, + email, phone, address, organization, title, notes, photo_url, + birthday, anniversary, vcard, etag, created_at, updated_at + FROM carddav.contacts + WHERE address_book_id = $1 + ORDER BY full_name, first_name, last_name + "#, + ) + .bind(address_book_id) + .fetch(&mut *conn); + + use futures::TryStreamExt; + while let Some(row) = rows.try_next().await.map_err(|e| { + DomainError::database_error(format!("Failed to stream contacts: {}", e)) + })? { + yield Self::row_to_contact(&row)?; + } + }); + stream + } + async fn get_contacts_by_address_book( &self, address_book_id: &Uuid, diff --git a/src/infrastructure/repositories/pg/favorites_pg_repository.rs b/src/infrastructure/repositories/pg/favorites_pg_repository.rs index 0818b44b..61482cbb 100644 --- a/src/infrastructure/repositories/pg/favorites_pg_repository.rs +++ b/src/infrastructure/repositories/pg/favorites_pg_repository.rs @@ -259,8 +259,9 @@ impl FavoritesRepositoryPort for FavoritesPgRepository { return Ok(HashSet::new()); } - // Collect just the IDs for the IN clause - let ids: Vec = item_ids.iter().map(|(id, _)| id.to_string()).collect(); + // Collect just the IDs for the IN clause — sqlx binds `&[&str]` as + // text[], so no per-id String is needed. + let ids: Vec<&str> = item_ids.iter().map(|(id, _)| *id).collect(); let rows = sqlx::query( "SELECT item_id FROM auth.user_favorites WHERE user_id = $1 AND item_id = ANY($2)", diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index 5abf1c5c..f70c5adb 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -11,9 +11,9 @@ /// Post-D7-step-6: `storage.files.user_id` dropped, so it's no /// longer projected. type MediaFileRow = ( - String, // id + Uuid, // id (binary decode; benches/ROUND6.md §10) String, // name - Option, // folder_id + Option, // folder_id Option, // folder path i64, // size String, // mime_type @@ -83,9 +83,9 @@ const CALLER_CAN_READ_DRIVE: &str = "EXISTS (\ /// longer part of the tuple; `row_to_file` populates the entity's /// legacy `user_id` field with `None`. type FileRow = ( + Uuid, String, - String, - Option, + Option, Option, i64, String, @@ -269,7 +269,7 @@ impl FileBlobReadRepository { let where_clause = conditions.join(" AND "); let sql = format!( - "SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, \ + "SELECT fi.id, fi.name, fi.folder_id, fo.path, \ fi.size, fi.mime_type, \ EXTRACT(EPOCH FROM fi.created_at)::bigint, \ EXTRACT(EPOCH FROM fi.updated_at)::bigint, \ @@ -319,7 +319,7 @@ impl FileBlobReadRepository { } let rows = sqlx::query_as::<_, FileRow>( - "SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, \ + "SELECT fi.id, fi.name, fi.folder_id, fo.path, \ fi.size, fi.mime_type, \ EXTRACT(EPOCH FROM fi.created_at)::bigint, \ EXTRACT(EPOCH FROM fi.updated_at)::bigint, \ @@ -415,9 +415,9 @@ impl FileBlobReadRepository { #[allow(clippy::too_many_arguments)] fn row_to_file( - id: String, + id: Uuid, name: String, - folder_id: Option, + folder_id: Option, folder_path: Option, size: i64, mime_type: String, @@ -428,12 +428,12 @@ impl FileBlobReadRepository { updated_by: Option, ) -> Result { File::from_materialized_row( - id, + id.to_string(), name, folder_path.as_deref(), size as u64, mime_type, - folder_id, + folder_id.map(|u| u.to_string()), created_at as u64, modified_at as u64, blob_hash, @@ -550,7 +550,7 @@ impl FileBlobReadRepository { AND (g.expires_at IS NULL OR g.expires_at > NOW()) AND (d.policies->>'include_in_photo_index')::boolean = true ) - SELECT top.id::text, top.name, top.folder_id::text, fo.path, + SELECT top.id, top.name, top.folder_id, fo.path, top.size, top.mime_type, EXTRACT(EPOCH FROM top.created_at)::bigint, EXTRACT(EPOCH FROM top.updated_at)::bigint, @@ -679,9 +679,9 @@ impl FileReadPort for FileBlobReadRepository { let row = sqlx::query_as::< _, ( - String, // id + Uuid, // id (binary decode) String, // name - Option, // folder_id + Option, // folder_id Option, // folder path i64, // size String, // mime_type @@ -693,7 +693,7 @@ impl FileReadPort for FileBlobReadRepository { ), >( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -726,9 +726,9 @@ impl FileReadPort for FileBlobReadRepository { let row = sqlx::query_as::< _, ( + Uuid, String, - String, - Option, + Option, Option, i64, String, @@ -740,7 +740,7 @@ impl FileReadPort for FileBlobReadRepository { ), >( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -768,7 +768,7 @@ impl FileReadPort for FileBlobReadRepository { let rows: Vec = if let Some(fid) = folder_id { sqlx::query_as( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -787,7 +787,7 @@ impl FileReadPort for FileBlobReadRepository { } else { sqlx::query_as( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -848,7 +848,7 @@ impl FileReadPort for FileBlobReadRepository { }; let sql = format!( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -1014,9 +1014,9 @@ impl FileReadPort for FileBlobReadRepository { sqlx::query_as::< _, ( + Uuid, String, - String, - Option, + Option, Option, i64, String, @@ -1028,7 +1028,7 @@ impl FileReadPort for FileBlobReadRepository { ), >( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -1052,9 +1052,9 @@ impl FileReadPort for FileBlobReadRepository { sqlx::query_as::< _, ( + Uuid, String, - String, - Option, + Option, Option, i64, String, @@ -1066,7 +1066,7 @@ impl FileReadPort for FileBlobReadRepository { ), >( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -1107,12 +1107,12 @@ impl FileReadPort for FileBlobReadRepository { let stream = async_stream::try_stream! { let mut row_stream = sqlx::query_as::<_, ( - String, String, Option, Option, + Uuid, String, Option, Option, i64, String, i64, i64, String, Option, Option, // created_by, updated_by (§14) )>( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -1195,7 +1195,7 @@ impl FileReadPort for FileBlobReadRepository { let offset_bind = bind_idx + 2; let sql = format!( - "SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, \ + "SELECT fi.id, fi.name, fi.folder_id, fo.path, \ fi.size, fi.mime_type, \ EXTRACT(EPOCH FROM fi.created_at)::bigint, \ EXTRACT(EPOCH FROM fi.updated_at)::bigint, \ @@ -1214,9 +1214,9 @@ impl FileReadPort for FileBlobReadRepository { let mut query = sqlx::query_as::< _, ( + Uuid, String, - String, - Option, + Option, Option, i64, String, @@ -1327,7 +1327,7 @@ impl FileReadPort for FileBlobReadRepository { // ── Single query with COUNT(*) OVER() ── let sql = format!( - "SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, \ + "SELECT fi.id, fi.name, fi.folder_id, fo.path, \ fi.size, fi.mime_type, \ EXTRACT(EPOCH FROM fi.created_at)::bigint, \ EXTRACT(EPOCH FROM fi.updated_at)::bigint, \ @@ -1346,9 +1346,9 @@ impl FileReadPort for FileBlobReadRepository { let mut query = sqlx::query_as::< _, ( + Uuid, String, - String, - Option, + Option, Option, i64, String, @@ -1420,7 +1420,7 @@ impl FileReadPort for FileBlobReadRepository { let rows: Vec = if let Some(fid) = folder_id { sqlx::query_as( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, @@ -1450,7 +1450,7 @@ impl FileReadPort for FileBlobReadRepository { } else { sqlx::query_as( r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, + SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, EXTRACT(EPOCH FROM fi.created_at)::bigint, EXTRACT(EPOCH FROM fi.updated_at)::bigint, diff --git a/src/interfaces/api/handlers/carddav_handler.rs b/src/interfaces/api/handlers/carddav_handler.rs index b5fea2a1..956f99e6 100644 --- a/src/interfaces/api/handlers/carddav_handler.rs +++ b/src/interfaces/api/handlers/carddav_handler.rs @@ -22,7 +22,8 @@ use axum::{ http::{HeaderName, Request, StatusCode, header}, response::Response, }; -use bytes::Buf; +use bytes::{Buf, Bytes}; +use quick_xml::Writer; use std::sync::Arc; use crate::application::adapters::carddav_adapter::{ @@ -31,7 +32,7 @@ use crate::application::adapters::carddav_adapter::{ use crate::application::adapters::uid_from_multiget_href; use crate::application::adapters::webdav_adapter::{PropFindRequest, PropFindType}; use crate::application::dtos::address_book_dto::{CreateAddressBookDto, UpdateAddressBookDto}; -use crate::application::dtos::contact_dto::CreateContactVCardDto; +use crate::application::dtos::contact_dto::{ContactDto, CreateContactVCardDto}; use crate::application::ports::carddav_ports::{AddressBookUseCase, ContactUseCase}; use crate::application::services::contact_service::ContactService; use crate::common::di::AppState; @@ -187,6 +188,164 @@ fn get_addressbook_service(state: &AppState) -> Result<&Arc, App }) } +/// Rows per emitted page for the streaming CardDAV emitters — contacts +/// carry no master/exception bundling, so pages cut anywhere. +const CARDDAV_STREAM_PAGE_CONTACTS: usize = 500; + +/// Streamed multistatus REPORT: header, one chunk per cursor page, +/// footer. Byte-compatible with the buffered +/// `generate_contacts_response` output; TTFB becomes the first page and +/// the whole-book DTO Vec is never materialised. +fn build_streaming_contacts_report( + contact_svc: Arc, + address_book_id: String, + report: CardDavReportType, + base_href: String, + user_id: uuid::Uuid, +) -> Response { + let stream = async_stream::try_stream! { + let mut buf = Vec::with_capacity(160); + { + let mut w = Writer::new(&mut buf); + CardDavAdapter::write_report_multistatus_start(&mut w) + .map_err(|e| std::io::Error::other(e.to_string()))?; + } + yield Bytes::from(buf); + + { + use futures::TryStreamExt; + let mut rows = contact_svc + .stream_contacts_by_book(&address_book_id, user_id) + .await + .map_err(|e| std::io::Error::other(e.to_string()))?; + let mut page: Vec = + Vec::with_capacity(CARDDAV_STREAM_PAGE_CONTACTS); + loop { + let next = rows + .try_next() + .await + .map_err(|e| std::io::Error::other(e.to_string()))?; + let flush = match &next { + Some(_) => page.len() >= CARDDAV_STREAM_PAGE_CONTACTS, + None => !page.is_empty(), + }; + if flush { + let mut chunk = Vec::with_capacity(page.len() * 256 + 64); + { + let mut w = Writer::new(&mut chunk); + CardDavAdapter::write_contacts_report_page( + &mut w, &page, &report, &base_href, + ) + .map_err(|e| std::io::Error::other(e.to_string()))?; + } + page.clear(); + yield Bytes::from(chunk); + } + match next { + Some(c) => page.push(c), + None => break, + } + } + } + + let mut buf = Vec::with_capacity(32); + { + let mut w = Writer::new(&mut buf); + CardDavAdapter::write_carddav_multistatus_end(&mut w) + .map_err(|e| std::io::Error::other(e.to_string()))?; + } + yield Bytes::from(buf); + }; + + use futures::TryStreamExt; + let stream = stream + .map_err(|e: std::io::Error| -> Box { Box::new(e) }); + + Response::builder() + .status(StatusCode::MULTI_STATUS) + .header(header::CONTENT_TYPE, "application/xml; charset=utf-8") + .body(Body::from_stream(stream)) + .unwrap() +} + +/// Streamed depth-1 address-book PROPFIND: head (multistatus + the +/// book's own response), one chunk per cursor page, footer. +fn build_streaming_book_propfind( + contact_svc: Arc, + address_book: crate::application::dtos::address_book_dto::AddressBookDto, + propfind_request: PropFindRequest, + address_book_id: String, + base_href: String, + user_id: uuid::Uuid, +) -> Response { + let stream = async_stream::try_stream! { + let mut buf = Vec::with_capacity(2048); + { + let mut w = Writer::new(&mut buf); + CardDavAdapter::write_collection_head( + &mut w, + &address_book, + &propfind_request, + &base_href, + ) + .map_err(|e| std::io::Error::other(e.to_string()))?; + } + yield Bytes::from(buf); + + { + use futures::TryStreamExt; + let mut rows = contact_svc + .stream_contacts_by_book(&address_book_id, user_id) + .await + .map_err(|e| std::io::Error::other(e.to_string()))?; + let mut page: Vec = + Vec::with_capacity(CARDDAV_STREAM_PAGE_CONTACTS); + loop { + let next = rows + .try_next() + .await + .map_err(|e| std::io::Error::other(e.to_string()))?; + let flush = match &next { + Some(_) => page.len() >= CARDDAV_STREAM_PAGE_CONTACTS, + None => !page.is_empty(), + }; + if flush { + let mut chunk = Vec::with_capacity(page.len() * 512 + 64); + { + let mut w = Writer::new(&mut chunk); + CardDavAdapter::write_collection_contact_page(&mut w, &page, &base_href) + .map_err(|e| std::io::Error::other(e.to_string()))?; + } + page.clear(); + yield Bytes::from(chunk); + } + match next { + Some(c) => page.push(c), + None => break, + } + } + } + + let mut buf = Vec::with_capacity(32); + { + let mut w = Writer::new(&mut buf); + CardDavAdapter::write_carddav_multistatus_end(&mut w) + .map_err(|e| std::io::Error::other(e.to_string()))?; + } + yield Bytes::from(buf); + }; + + use futures::TryStreamExt; + let stream = stream + .map_err(|e: std::io::Error| -> Box { Box::new(e) }); + + Response::builder() + .status(StatusCode::MULTI_STATUS) + .header(header::CONTENT_TYPE, "application/xml; charset=utf-8") + .body(Body::from_stream(stream)) + .unwrap() +} + fn get_contact_service(state: &AppState) -> Result<&Arc, AppError> { state.contact_use_case.as_ref().ok_or_else(|| { AppError::new( @@ -334,14 +493,19 @@ async fn handle_propfind( .await .map_err(|e| AppError::not_found(format!("Address book not found: {}", e)))?; - let contacts = if depth != "0" { - contact_svc - .list_contacts(address_book_id, None, None, user.id) - .await - .unwrap_or_default() - } else { - vec![] - }; + // Depth-1 streams the contact listing page by page; depth-0 + // has no contact section and keeps the tiny buffered path. + if depth != "0" { + let base_href = format!("/carddav/{}/", address_book_id); + return Ok(build_streaming_book_propfind( + contact_svc.clone(), + address_book, + propfind_request, + address_book_id.to_string(), + base_href, + user.id, + )); + } let base_href = &format!("/carddav/{}/", address_book_id); let mut response_body = Vec::new(); @@ -349,7 +513,7 @@ async fn handle_propfind( CardDavAdapter::generate_addressbook_collection_propfind( &mut response_body, &address_book, - &contacts, + &[], &propfind_request, base_href, &depth, @@ -423,11 +587,25 @@ async fn handle_report( return Err(AppError::bad_request("Address book ID required in path")); } + // Whole-book shapes stream; bounded multiget keeps the buffered path. + if matches!( + &report, + CardDavReportType::AddressbookQuery { .. } | CardDavReportType::SyncCollection { .. } + ) { + let base_href = format!("/carddav/{}/", address_book_id); + return Ok(build_streaming_contacts_report( + contact_svc.clone(), + address_book_id.to_string(), + report, + base_href, + user.id, + )); + } + let contacts = match &report { - CardDavReportType::AddressbookQuery { .. } => contact_svc - .list_contacts(address_book_id, None, None, user.id) - .await - .map_err(AppError::from)?, + CardDavReportType::AddressbookQuery { .. } => { + unreachable!("addressbook-query streams above") + } CardDavReportType::AddressbookMultiget { hrefs, .. } => { // Indexed batch lookup (`uid = ANY(...)`) — a multiget for a // handful of contacts must not pay for listing the whole @@ -442,10 +620,9 @@ async fn handle_report( .await .map_err(AppError::from)? } - CardDavReportType::SyncCollection { .. } => contact_svc - .list_contacts(address_book_id, None, None, user.id) - .await - .map_err(AppError::from)?, + CardDavReportType::SyncCollection { .. } => { + unreachable!("sync-collection streams above") + } }; let base_href = &format!("/carddav/{}/", address_book_id); diff --git a/src/interfaces/api/handlers/share_handler.rs b/src/interfaces/api/handlers/share_handler.rs index c3801e84..aa5c406b 100644 --- a/src/interfaces/api/handlers/share_handler.rs +++ b/src/interfaces/api/handlers/share_handler.rs @@ -232,17 +232,18 @@ pub async fn access_shared_item( Path(token): Path, headers: HeaderMap, ) -> impl IntoResponse { - // Register the access - let _ = share_use_case.register_shared_link_access(&token).await; - // Honour an unlock cookie if one was issued by a prior `/verify` call. let unlock_jwt = unlock_jwt_from_headers(&headers, &token); - // Get the shared link - match share_use_case - .get_shared_link_with_unlock(&token, unlock_jwt.as_deref()) - .await - { + // The access-count increment doesn't gate the fetch — run both + // round-trips concurrently instead of serially (one RTT saved on + // every public share landing). + let (_, item) = tokio::join!( + share_use_case.register_shared_link_access(&token), + share_use_case.get_shared_link_with_unlock(&token, unlock_jwt.as_deref()), + ); + + match item { Ok(item) => (StatusCode::OK, Json(item)).into_response(), Err(err) => { // Special handling for share access errors diff --git a/src/interfaces/nextcloud/ocs_handler.rs b/src/interfaces/nextcloud/ocs_handler.rs index 73fc0394..2fe4207d 100644 --- a/src/interfaces/nextcloud/ocs_handler.rs +++ b/src/interfaces/nextcloud/ocs_handler.rs @@ -411,8 +411,8 @@ pub async fn handle_search( // Pre-resolve numeric ids for every file result in a single batch query // (was one INSERT round-trip per result). - let file_uuids: Vec = results.files.iter().map(|f| f.id.clone()).collect(); - let file_id_map: HashMap = match file_id_svc { + let file_uuids: Vec<&str> = results.files.iter().map(|f| f.id.as_str()).collect(); + let file_id_map: HashMap = match file_id_svc { Some(svc) => svc .get_or_create_file_ids(&file_uuids) .await @@ -435,7 +435,8 @@ pub async fn handle_search( crate::interfaces::nextcloud::webdav_handler::strip_drive_root_segment(&file.path); let display_path = format!("/{}", display_path); - let numeric_id = file_id_map.get(&file.id).copied(); + let numeric_id = + crate::interfaces::nextcloud::webdav_handler::nc_id_of(&file_id_map, &file.id); let thumbnail_url = match numeric_id { Some(nid) => format!("/index.php/core/preview?fileId={}&x=32&y=32", nid), diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index 68ff9f14..5c1952fb 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -26,7 +26,7 @@ use crate::interfaces::api::handlers::webdav_handler::{ }; use crate::interfaces::errors::AppError; use crate::interfaces::nextcloud::webdav_handler::{ - batch_resolve_ids, format_oc_id, nc_href, write_file_response, write_folder_response, + batch_resolve_ids, format_oc_id, nc_href, nc_id_of, write_file_response, write_folder_response, }; /// Handle WebDAV REPORT and SEARCH methods for Nextcloud compatibility. @@ -150,8 +150,8 @@ async fn handle_filter_files( } // Pass 2: resolve every oc:fileid in two batch queries (was one per item). - let file_uuids: Vec = files.iter().map(|f| f.id.clone()).collect(); - let folder_uuids: Vec = folders.iter().map(|f| f.id.clone()).collect(); + let file_uuids: Vec<&str> = files.iter().map(|f| f.id.as_str()).collect(); + let folder_uuids: Vec<&str> = folders.iter().map(|f| f.id.as_str()).collect(); let (file_id_map, folder_id_map) = batch_resolve_ids(file_id_svc, &file_uuids, &folder_uuids).await; @@ -184,7 +184,7 @@ async fn handle_filter_files( continue; }; let href = nc_href(url_user, subpath); - let fid = file_id_map.get(&file.id).copied(); + let fid = nc_id_of(&file_id_map, &file.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); let dead = dead_props_for(&file.id, &file_deads); write_file_response( @@ -210,7 +210,7 @@ async fn handle_filter_files( continue; }; let href = format!("{}/", nc_href(url_user, subpath)); - let fid = folder_id_map.get(&folder.id).copied(); + let fid = nc_id_of(&folder_id_map, &folder.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); let dead = dead_props_for(&folder.id, &folder_deads); write_folder_response( @@ -297,8 +297,8 @@ async fn handle_search( // (was one INSERT round-trip per result). let files: Vec = results.files.iter().map(file_dto_from_search).collect(); let folders: Vec = results.folders.iter().map(folder_dto_from_search).collect(); - let file_uuids: Vec = files.iter().map(|f| f.id.clone()).collect(); - let folder_uuids: Vec = folders.iter().map(|f| f.id.clone()).collect(); + let file_uuids: Vec<&str> = files.iter().map(|f| f.id.as_str()).collect(); + let folder_uuids: Vec<&str> = folders.iter().map(|f| f.id.as_str()).collect(); let (file_id_map, folder_id_map) = batch_resolve_ids(file_id_svc, &file_uuids, &folder_uuids).await; @@ -325,7 +325,7 @@ async fn handle_search( continue; }; let href = nc_href(url_user, subpath); - let fid = file_id_map.get(&file.id).copied(); + let fid = nc_id_of(&file_id_map, &file.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); let dead = dead_props_for(&file.id, &file_deads); write_file_response( @@ -352,7 +352,7 @@ async fn handle_search( continue; }; let href = format!("{}/", nc_href(url_user, subpath)); - let fid = folder_id_map.get(&folder.id).copied(); + let fid = nc_id_of(&folder_id_map, &folder.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); let dead = dead_props_for(&folder.id, &folder_deads); write_folder_response( diff --git a/src/interfaces/nextcloud/trashbin_handler.rs b/src/interfaces/nextcloud/trashbin_handler.rs index b860ce03..6f823f28 100644 --- a/src/interfaces/nextcloud/trashbin_handler.rs +++ b/src/interfaces/nextcloud/trashbin_handler.rs @@ -15,7 +15,7 @@ use crate::application::ports::trash_ports::TrashUseCase; use crate::common::di::AppState; use crate::interfaces::errors::AppError; use crate::interfaces::nextcloud::webdav_handler::{ - batch_resolve_ids, extract_nc_subpath_from_dest, format_oc_id, nc_to_internal_path, + batch_resolve_ids, extract_nc_subpath_from_dest, format_oc_id, nc_id_of, nc_to_internal_path, write_text_element, }; @@ -308,6 +308,7 @@ fn strip_home_prefix<'a>( use crate::application::dtos::trash_dto::TrashedItemDto; use crate::application::services::nextcloud_file_id_service::NextcloudFileIdService; use std::collections::HashMap; +use uuid::Uuid; /// Generate a complete Nextcloud-compatible multistatus XML response for the trashbin. /// @@ -337,14 +338,14 @@ async fn write_trashbin_multistatus( // Pre-resolve every oc:fileid in two batch queries by object type (was one // INSERT round-trip per item). File and folder UUIDs are disjoint, so the - // two maps merge cleanly into one keyed by original_id. - let mut file_uuids: Vec = Vec::new(); - let mut folder_uuids: Vec = Vec::new(); + // two maps merge cleanly into one keyed by parsed original-id UUID. + let mut file_uuids: Vec<&str> = Vec::new(); + let mut folder_uuids: Vec<&str> = Vec::new(); for item in items { if item.item_type == "folder" { - folder_uuids.push(item.original_id.clone()); + folder_uuids.push(item.original_id.as_str()); } else { - file_uuids.push(item.original_id.clone()); + file_uuids.push(item.original_id.as_str()); } } let (mut id_map, folder_id_map) = @@ -427,7 +428,7 @@ fn write_trash_item_response( username: &str, chroot: &crate::application::dtos::folder_dto::FolderDto, file_id_svc: Option<&Arc>, - id_map: &HashMap, + id_map: &HashMap, ) -> Result<(), String> { xml.write_event(Event::Start(BytesStart::new("d:response"))) .map_err(|e| e.to_string())?; @@ -475,7 +476,7 @@ fn write_trash_item_response( write_text_element(xml, "d:getcontentlength", "0")?; // oc:fileid and oc:id — resolved up front in a batch query. - let file_id = id_map.get(&item.original_id).copied(); + let file_id = nc_id_of(id_map, &item.original_id); if let Some(id) = file_id { write_text_element(xml, "oc:fileid", &id.to_string())?; let oc_id = format_oc_id(id, file_id_svc); diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index 5efac8eb..d24eb407 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -1445,8 +1445,7 @@ async fn write_nc_file_multistatus( extras: (&HashSet, &[(QualifiedName, Option)]), ) -> Result<(), String> { let (favorite_ids, dead_props) = extras; - let (file_id_map, _) = - batch_resolve_ids(file_id_svc, std::slice::from_ref(&file.id), &[]).await; + let (file_id_map, _) = batch_resolve_ids(file_id_svc, &[file.id.as_str()], &[]).await; let mut xml = Writer::new(writer); write_nc_multistatus_open(&mut xml)?; @@ -1457,7 +1456,7 @@ async fn write_nc_file_multistatus( // shares the requested URL's prefix. `username` is the canonical // identity for the `oc:owner-id` field. let href = nc_href(url_user, subpath); - let file_id = file_id_map.get(&file.id).copied(); + let file_id = nc_id_of(&file_id_map, &file.id); let oc_id = file_id.map(|id| format_oc_id(id, file_id_svc)); write_file_response( &mut xml, @@ -1509,7 +1508,7 @@ fn build_nc_streaming_propfind( HashSet::new() }; let (_, folder_id_map) = - batch_resolve_ids(file_id_svc, &[], std::slice::from_ref(&folder.id)).await; + batch_resolve_ids(file_id_svc, &[], &[folder.id.as_str()]).await; let folder_dead = folder_dead_props(&state.webdav_dead_props, &folder).await; let mut buf = Vec::with_capacity(4096); @@ -1517,7 +1516,7 @@ fn build_nc_streaming_propfind( let mut xml = Writer::new(&mut buf); write_nc_multistatus_open(&mut xml).map_err(std::io::Error::other)?; let href = nc_collection_href(&username, &subpath); - let fid = folder_id_map.get(&folder.id).copied(); + let fid = nc_id_of(&folder_id_map, &folder.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); write_folder_response(&mut xml, &folder, &href, (fid, oc_id.as_deref()), &username, &folder_favs, quota, &folder_dead) .map_err(std::io::Error::other)?; @@ -1565,7 +1564,7 @@ fn build_nc_streaming_propfind( } else { HashSet::new() }; - let file_uuids: Vec = batch.iter().map(|f| f.id.clone()).collect(); + let file_uuids: Vec<&str> = batch.iter().map(|f| f.id.as_str()).collect(); let (file_id_map, _) = batch_resolve_ids(file_id_svc, &file_uuids, &[]).await; // One batched dead-props query per page, not one per child // (benches/DEAD-PROPS.md). @@ -1582,7 +1581,7 @@ fn build_nc_streaming_propfind( // re-encoded both for every child). let href = format!("{}{}", child_href_prefix, urlencoding::encode(&file.name)); - let fid = file_id_map.get(&file.id).copied(); + let fid = nc_id_of(&file_id_map, &file.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); write_file_response(&mut xml, file, &href, (fid, oc_id.as_deref()), &username, &favs, dead) .map_err(std::io::Error::other)?; @@ -1622,7 +1621,7 @@ fn build_nc_streaming_propfind( } else { HashSet::new() }; - let folder_uuids: Vec = batch.iter().map(|sf| sf.id.clone()).collect(); + let folder_uuids: Vec<&str> = batch.iter().map(|sf| sf.id.as_str()).collect(); let (_, sub_id_map) = batch_resolve_ids(file_id_svc, &[], &folder_uuids).await; // Batched — see benches/DEAD-PROPS.md. let sub_deads = @@ -1637,7 +1636,7 @@ fn build_nc_streaming_propfind( // precomputed once like the file loop above. let href = format!("{}{}/", child_href_prefix, urlencoding::encode(&sf.name)); - let fid = sub_id_map.get(&sf.id).copied(); + let fid = nc_id_of(&sub_id_map, &sf.id); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); write_folder_response(&mut xml, sf, &href, (fid, oc_id.as_deref()), &username, &favs, quota, dead) .map_err(std::io::Error::other)?; @@ -1949,14 +1948,15 @@ pub fn write_text_element( /// Resolve every `oc:fileid` for a listing in two batch queries (one per /// object type) instead of one INSERT round-trip per child. Returns -/// `(file_map, folder_map)` keyed by object UUID; entries are absent when the -/// service is disabled or an id can't be resolved, mirroring the previous -/// per-call `Option` behaviour. The two batches run concurrently. +/// `(file_map, folder_map)` keyed by parsed object UUID; entries are absent +/// when the service is disabled or an id can't be resolved, mirroring the +/// previous per-call `Option` behaviour. The two batches run concurrently. +/// Borrowed inputs + `Uuid` keys keep the whole resolution alloc-free. pub async fn batch_resolve_ids( svc: Option<&Arc>, - file_uuids: &[String], - folder_uuids: &[String], -) -> (HashMap, HashMap) { + file_uuids: &[&str], + folder_uuids: &[&str], +) -> (HashMap, HashMap) { let Some(svc) = svc else { return (HashMap::new(), HashMap::new()); }; @@ -1967,6 +1967,11 @@ pub async fn batch_resolve_ids( (files.unwrap_or_default(), folders.unwrap_or_default()) } +/// Look up a batch-resolved `oc:fileid` by a DTO's string UUID. +pub fn nc_id_of(map: &HashMap, id: &str) -> Option { + Uuid::parse_str(id).ok().and_then(|u| map.get(&u).copied()) +} + pub fn format_oc_id(id: i64, svc: Option<&Arc>) -> String { match svc { Some(s) => s.format_oc_id(id), diff --git a/src/interfaces/upload_ingest.rs b/src/interfaces/upload_ingest.rs index 7a526684..67cd3a47 100644 --- a/src/interfaces/upload_ingest.rs +++ b/src/interfaces/upload_ingest.rs @@ -419,8 +419,8 @@ impl IncrementalHasher { fn finalize_hex(self) -> String { match self { - Self::Md5(h) => h.finalize().iter().map(|b| format!("{b:02x}")).collect(), - Self::Sha256(h) => h.finalize().iter().map(|b| format!("{b:02x}")).collect(), + Self::Md5(h) => crate::common::fmt::hex_lower(&h.finalize()), + Self::Sha256(h) => crate::common::fmt::hex_lower(&h.finalize()), Self::Blake3(h) => h.finalize().to_hex().to_string(), } } From 7626dc95c11052dfd5c7cf95a965adccb5c45507 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:11:41 +0000 Subject: [PATCH 3/4] =?UTF-8?q?perf:=20round=207=20=E2=80=94=20photos=20ti?= =?UTF-8?q?meline=20O(N=C2=B2)=E2=86=92incremental,=20range-seek=20authz?= =?UTF-8?q?=20duplication,=20resources=20row-map=20clone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmark-gated (equivalence + BEFORE/AFTER; results + reproduce commands in benches/ROUND7.md): - Photos timeline re-grouped + re-laid-out the whole accumulated library on every 60-item page (both `groups` and `photoRows` were $derived over the full list), Σ ≈ O(N²/60) main-thread work during a scroll. Pages arrive newest-first so grouping is append-only: the new PhotoTimeline (lib/utils/photoTimeline.ts) re-buckets only the fresh page and re-lays-out only changed groups, reusing untouched groups' cached rows, falling back to a full rebuild on any config/deletion/non-append change. The pure buildPhotoRows is the verbatim reference the gate holds it equal to at every page. 50×60 drain: 76 500 → 3 000 grouping ops (25.5x), 23.0 → 2.2 ms (10.6x). - Range downloads paid authz + access-notify twice: download_file_impl resolves the file via get_file_with_perms, then the Range branch re-ran require_file + notify_file_accessed per request. Media/PDF viewers fetch exclusively via Range (one request per seek), so every seek in a scrub re-authorized an already-cleared file. Now routed through the non-perms get_file_range_preloaded (matching the share-landing + WebDAV range paths); the unused _with_perms range method is removed. The request-level gate still denies before the branch runs (bench asserts member granted, outsider denied). Per seek removed: WARM 0.67 µs, COLD 1362.66 µs — a grant-cascade drive-resolve query per seek for a shared-drive recipient on a cold cache. - /api/folders/{id}/resources row→DTO mapping cloned row.name into the DTO though the row is owned; folders move it (fixed icons), files compute the name-derived icon/category classes first then move it. 500-row page: 10.004 → 9.004 allocs/row (500 clones removed), output identical. Deferred with rationale in ROUND7.md: thumbnail ACL-before-304 (security posture — needs a security review, not a perf tweak), batch_operations Arc→String widening, list-view O(N²) on smaller lists, and the serial→ join! pairs (decide-by-bench with injected latency, per the round-6 rejection). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA --- Cargo.toml | 17 ++ benches/ROUND7.md | 146 +++++++++ examples/bench_range_seek_authz.rs | 283 ++++++++++++++++++ examples/bench_resource_row_map.rs | 282 +++++++++++++++++ .../src/lib/utils/photoTimeline.bench.test.ts | 163 ++++++++++ frontend/src/lib/utils/photoTimeline.ts | 279 +++++++++++++++++ frontend/src/routes/photos/+page.svelte | 174 +++-------- .../services/file_retrieval_service.rs | 16 - src/interfaces/api/handlers/file_handler.rs | 14 +- src/interfaces/api/handlers/folder_handler.rs | 22 +- 10 files changed, 1228 insertions(+), 168 deletions(-) create mode 100644 benches/ROUND7.md create mode 100644 examples/bench_range_seek_authz.rs create mode 100644 examples/bench_resource_row_map.rs create mode 100644 frontend/src/lib/utils/photoTimeline.bench.test.ts create mode 100644 frontend/src/lib/utils/photoTimeline.ts diff --git a/Cargo.toml b/Cargo.toml index 50376e8a..c03a0171 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,6 +350,23 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-7 battery ───────────────────────────────────────────────────────────── + +# Range-seek per-request authz duplication — the per-seek require the range +# branch used to run (warm CPU + cold drive-resolve query) vs 0 after routing +# through the non-perms range read (needs the dev Postgres up). +[[example]] +name = "bench_range_seek_authz" +path = "examples/bench_range_seek_authz.rs" +required-features = ["bench"] + +# `/api/folders/{id}/resources` row→DTO mapping — per-row name clone vs move +# (pure CPU; counting allocator). +[[example]] +name = "bench_resource_row_map" +path = "examples/bench_resource_row_map.rs" +required-features = ["bench"] + # Round-6 battery ───────────────────────────────────────────────────────────── # CardDAV whole-book REPORT/PROPFIND — buffered double-residency vs cursor diff --git a/benches/ROUND7.md b/benches/ROUND7.md new file mode 100644 index 00000000..61d5f163 --- /dev/null +++ b/benches/ROUND7.md @@ -0,0 +1,146 @@ +# Round 7 — photo timeline O(N²) → incremental, range-seek authz duplication, row-map clone + +Benchmark-gated changes, same rule as ROUND2-6: every change ships with a +BEFORE/AFTER benchmark; an AFTER that doesn't beat its BEFORE gets rolled +back. Equivalence gates (identical output / byte-identical responses) guard +every behavior-preserving rewrite. Frontend changes carry vitest benchmark +gates (verbatim BEFORE replica + equivalence + perf assertion) committed +beside the code so CI re-verifies the win on every run. + +Measured on 4 cores / 15 GiB, local PostgreSQL 16 (fsync off), release +profile; frontend on Node 22 / vitest 4 (jsdom). Reproduce any row with the +command in its section. + +## Summary + +| # | change | key metric | before → after | +|--:|---|---|---| +| 1 | Photos timeline incremental grouping/layout | 50-page (3k-photo) scroll drain | 76 500 → 3 000 group ops (**25.5x**) / 23.0 → 2.2 ms (**10.6x**) | +| 2 | Range-seek per-request authz duplication removed | per-seek authz on a shared-drive scrub | WARM 0.67 → 0 µs/seek; **COLD 1362.66 → 0 µs/seek** (a drive-resolve query per seek) | +| 3 | `/resources` row→DTO name clone → move | allocs/row (500-row page) | 10.004 → 9.004 (**500 allocs saved**, 1.00/row) | + +## [1] Photos timeline — O(N²) re-group + re-layout per page → incremental builder + +The photos view appended each 60-item page with `items = [...items, ...page]` +and re-derived both `groups` (O(N), a `new Date()` per photo) and `photoRows` +(O(N) row layout) over the whole accumulated list on every page — so paging to +photo N re-grouped + re-laid-out everything loaded so far, Σ ≈ O(N²/60) of +main-thread work during the scroll (the exact class ROUND6 fixed for the files +listing). The DOM was already windowed (`VirtualRows`); this was the derivation +feeding it. + +Because photos arrive newest-first (`media_sort_date DESC`), grouping is +append-only: a page only ever extends the last date bucket or adds buckets +after it, never mutates an earlier group. The new `PhotoTimeline` +(`lib/utils/photoTimeline.ts`) exploits that — an append re-buckets only the +fresh page and re-lays-out only the groups that changed, reusing every +untouched group's cached rows; any other change (config, deletion, filter +toggle, non-append) falls back to a full rebuild. The pure `buildPhotoRows` is +the verbatim reference the gate holds it equal to. + +Gates: the incremental output is deep-equal to `buildPhotoRows` at EVERY page +of the drain (both square + justified layouts); config-change / deletion / +width=0 fall back to a correct full rebuild; grouping work collapses ≥5x and +wall ≥3x. + +``` +cd frontend && npx vitest run src/lib/utils/photoTimeline.bench.test.ts --disable-console-intercept +# photo timeline 50×60: before 76500 timestamp reads / 23.0 ms +# after 3000 timestamp reads / 2.2 ms +# (25.5x fewer grouping ops, 10.6x wall) +``` + +## [2] Range downloads — duplicate per-seek authz + access-notify removed + +`download_file_impl` resolves the file once via `get_file_with_perms` (authz + +access-notify + metadata), then the Range branch called +`get_file_range_preloaded_with_perms`, which re-ran `require_file` (authz) + +`notify_file_accessed` per request. Media players and PDF viewers fetch a file +*exclusively* through Range requests — a `bytes=0-` probe then one request per +seek — so every seek in a scrub re-authorized a file the request-level gate had +already cleared. The share-landing and WebDAV range paths already authorize +once then read via the non-perms `get_file_range_preloaded`; the REST handler +now does the same (and the now-unused `_with_perms` range method is deleted). + +Safety: the request-level `get_file_with_perms` still gates every request +(denies before the Range branch runs), so the removed per-seek re-check +bypasses nothing — the bench asserts the member is granted and a non-member +denied. + +``` +cargo run --release --features bench --example bench_range_seek_authz +# seeks/scrub=200 (member of a shared drive, viewer grant) +# arm wall ms µs/seek +# BEFORE per-seek (WARM) 0.13 0.67 <- moka hit + uuid parse, removed +# BEFORE per-seek (COLD) 272.53 1362.66 <- a grant-cascade drive-resolve +# QUERY per seek, removed +# AFTER per-seek (removed) 0.00 0.00 +# A 200-seek scrub of a shared video stops paying ~272 ms of authz queries +# when the drive-role cache is cold (cross-drive recipient, or 30 s TTL expiry +# mid-scrub). notify_file_accessed (a throttled hook call) is likewise removed +# per seek. +``` + +## [3] `/api/folders/{id}/resources` row→DTO mapping — clone name → move name + +The listing maps each owned `FolderResourceRow` into a DTO but cloned +`row.name` into it (`name: row.name.clone()`) — one avoidable `String` heap +alloc per listed folder/file. The folder branch uses fixed icon classes, so +`row.name` is simply moved; the file branch computes its name-derived icon / +category classes first (they borrow `&row.name`), then moves `row.name` in. One +fewer alloc per row, identical output. + +``` +cargo run --release --features bench --example bench_resource_row_map +# rows=500 +# arm allocs wall ms allocs/row +# BEFORE (clone) 5002 0.841 10.004 +# AFTER (move) 4502 0.810 9.004 +# Saved 500 allocs (1.00/row) — the per-row name clone removed; output identical. +``` + +## Deferred / flagged (not shipped this round) + +- **Thumbnail ACL-before-304 (security posture — needs maintainer decision).** + `get_thumbnail_impl` runs `require_permission(Read)` before the ETag-304 and + moka/disk short-circuits, so a shared-album recipient pays a grant-cascade + query per thumbnail revalidation. Moving authz *after* the cache would make + thumbnails "authorized at creation time only" — a user whose access was + revoked could still fetch cached thumbnails of files they once could see. + That is a deliberate security-posture change, not a perf tweak; left for a + security review. The safe alternative (back the non-owner authz with the + existing `drive_role_cache`, or a `Borrow` cache key that removes the + per-request `to_string`) is queued for round 8 with an alloc/query bench. +- **`batch_operations` `Arc` → `String` per item.** `copy_file_with_perms` + / `move_file_with_perms` take `Option`, so the batch path's + `target_folder: Arc` is re-`to_string()`-ed per item, defeating the + Arc. Widening those `_with_perms` signatures to `Option<&str>` touches the + trait + impl + stub + ~7 call sites — a contained refactor better done + deliberately with its own alloc bench; queued for round 8. +- **List-view O(N²) re-derive (favorites / recent / trash / shared-with-me / + shared swimlanes).** Same class as [1] but on typically-smaller lists; + each infinite-scroll page re-derives `entries` / `byId` / `sections` / + `lanes` over the full accumulated set. Deferred — the incremental-builder + cost isn't yet justified at those sizes; revisit if any surface reaches + thousands of rows. +- **Serial independent DB pairs → `join!` (token refresh, login, cross-drive + move, CardDAV discovery, NC PROPFIND enrichment).** Overlapping independent + round-trips saves 1 RTT *under real PG latency*, but the ROUND6 authz-fan-out + rejection showed the overhead can wash the win out on local-socket PG. These + need a decide-by-bench with an injected-latency arm (like the ROUND6 `::text` + A/B) before adoption — queued for round 8, not guessed at here. + +## Correctness-adjacent (surfaced by the round-7 hunt — not perf, flagged for follow-up) + +- **`fetchFolderListing` returns empty `favoriteIds`/`sharedIds`** + (`frontend/src/lib/api/endpoints/folders.ts`) since the combined `/listing` + route was removed — the files-grid star/shared badges are seeded empty on + every navigation. The same removal also dropped the 304 conditional + fast-path, so a folder navigation now pages the full body (`cache: no-store`) + instead of a bodiless 304 on unchanged folders (mitigated only by the + in-memory `folderCache`). Functional regression, not perf. +- **Search page lacks a stale-response guard** + (`frontend/src/routes/search/+page.svelte`): the query `$effect` awaits + `searchFiles` with no `seq`/AbortController, so a slow stale query can + resolve after and clobber a newer one. The files view's `loadSeq` is the + pattern to mirror. diff --git a/examples/bench_range_seek_authz.rs b/examples/bench_range_seek_authz.rs new file mode 100644 index 00000000..50d5b700 --- /dev/null +++ b/examples/bench_range_seek_authz.rs @@ -0,0 +1,283 @@ +//! Range-seek per-request authz duplication benchmark. +//! +//! `download_file_impl` calls `get_file_with_perms` once (authz + access +//! notify + metadata) and THEN, in the Range branch, called +//! `get_file_range_preloaded_with_perms` — which re-ran `require_file` +//! (authz) + `notify_file_accessed` per request. Media players and PDF +//! viewers fetch a file *exclusively* through Range requests: a `bytes=0-` +//! probe then one request per seek. So every seek in a scrub re-authorized a +//! file the request-level gate had already cleared. +//! +//! Round 7 drops the range branch to the non-perms `get_file_range_preloaded` +//! (the share-landing and WebDAV range paths already do exactly this). This +//! bench isolates the per-seek `require` that AFTER eliminates, driving the +//! REAL `PgAclEngine`: +//! - WARM: the cache the initial `get_file_with_perms` warmed — each removed +//! seek-check was a moka hit + uuid parse (pure CPU/alloc). +//! - COLD: a shared-drive recipient whose drive-role cache expired mid-scrub +//! (30 s TTL) — each removed seek-check was a full drive-resolve query. +//! +//! Safety gate: the surviving request-level gate still authorizes correctly — +//! the member is granted, a non-member is denied — so removing the per-seek +//! re-check bypasses nothing. +//! +//! Run (needs Postgres up; reads DATABASE_URL from .env): +//! cargo run --release --features bench --example bench_range_seek_authz +//! Tunables (env): BENCH_SEEKS (200), BENCH_POOL (8). + +use std::env; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use oxicloud::application::ports::authorization_ports::AuthorizationEngine; +use oxicloud::domain::services::authorization::{Permission, Resource, Subject}; +use oxicloud::infrastructure::repositories::pg::{ + FileBlobReadRepository, FolderDbRepository, SubjectGroupPgRepository, +}; +use oxicloud::infrastructure::services::dedup_service::DedupService; +use oxicloud::infrastructure::services::local_blob_backend::LocalBlobBackend; +use oxicloud::infrastructure::services::pg_acl_engine::PgAclEngine; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; +use uuid::Uuid; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +struct Seeded { + member: Uuid, + outsider: Uuid, + drive_id: Uuid, + root_folder: Uuid, + blob_hash: String, + file_id: Uuid, +} + +async fn seed(pool: &PgPool) -> Seeded { + let mut tx = pool.begin().await.expect("begin"); + let member: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_rangeseek', 'bench_rangeseek@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed member"); + let outsider: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_rangeseek_out', 'bench_rangeseek_out@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed outsider"); + + let drive_id: Uuid = + sqlx::query_scalar("INSERT INTO storage.drives (kind) VALUES ('shared') RETURNING id") + .fetch_one(&mut *tx) + .await + .expect("seed drive"); + let root_folder: Uuid = sqlx::query_scalar( + "INSERT INTO storage.folders (name, path, lpath, drive_id) + VALUES ('Bench Seek', '/Bench Seek', 'x', $1) RETURNING id", + ) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed folder"); + sqlx::query("UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2") + .bind(root_folder) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("stamp root"); + sqlx::query( + "INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', $1, 'drive', $2, 'viewer'::storage.grant_role, $1)", + ) + .bind(member) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("seed grant"); + + let blob_hash = "benchrangeseek00000000000000000000000000000000000000000000000b3".to_string(); + sqlx::query("INSERT INTO storage.blobs (hash, size, ref_count) VALUES ($1, 1048576, 1)") + .bind(&blob_hash) + .execute(&mut *tx) + .await + .expect("seed blob"); + let file_id: Uuid = sqlx::query_scalar( + "INSERT INTO storage.files (name, folder_id, blob_hash, size, mime_type, drive_id) + VALUES ('clip.mp4', $1, $2, 1048576, 'video/mp4', $3) RETURNING id", + ) + .bind(root_folder) + .bind(&blob_hash) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed file"); + tx.commit().await.expect("commit"); + Seeded { + member, + outsider, + drive_id, + root_folder, + blob_hash, + file_id, + } +} + +async fn cleanup(pool: &PgPool, s: &Seeded) { + let _ = sqlx::query("DELETE FROM storage.role_grants WHERE resource_id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.files WHERE drive_id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.drives WHERE id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.folders WHERE id = $1") + .bind(s.root_folder) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.blobs WHERE hash = $1") + .bind(&s.blob_hash) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.users WHERE id IN ($1, $2)") + .bind(s.member) + .bind(s.outsider) + .execute(pool) + .await; +} + +fn fresh_engine(pool: &Arc) -> Arc { + let folder_repo = Arc::new(FolderDbRepository::new(pool.clone())); + let backend = Arc::new(LocalBlobBackend::new(std::path::Path::new( + "/tmp/bench-rangeseek-blobs", + ))); + let dedup = Arc::new(DedupService::new(backend, pool.clone(), pool.clone())); + let file_repo = Arc::new(FileBlobReadRepository::new( + pool.clone(), + dedup, + folder_repo.clone(), + )); + let group_repo = Arc::new(SubjectGroupPgRepository::new(pool.clone())); + Arc::new(PgAclEngine::new( + pool.clone(), + folder_repo, + file_repo, + group_repo, + )) +} + +/// The per-seek check the range branch used to run (verbatim: uuid parse + +/// `authz.require`, exactly `require_file`'s body). +async fn seek_require(engine: &Arc, caller: Uuid, file_id: Uuid) -> bool { + engine + .require( + Subject::User(caller), + Permission::Read, + Resource::File(file_id), + ) + .await + .is_ok() +} + +#[tokio::main(flavor = "multi_thread")] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let seeks: usize = env_or("BENCH_SEEKS", 200); + let pool_size: u32 = env_or("BENCH_POOL", 8); + + let pool = Arc::new( + PgPoolOptions::new() + .max_connections(pool_size) + .min_connections(pool_size) + .acquire_timeout(Duration::from_secs(10)) + .connect(&url) + .await + .expect("connect Postgres"), + ); + + let s = seed(&pool).await; + + // ── Safety gate: the surviving request-level gate authorizes correctly ── + let gate = fresh_engine(&pool); + let member_ok = seek_require(&gate, s.member, s.file_id).await; + let outsider_denied = !seek_require(&gate, s.outsider, s.file_id).await; + if !member_ok || !outsider_denied { + eprintln!( + "SAFETY GATE FAILED: member_ok={member_ok} outsider_denied={outsider_denied} \ + (the single request-level authz must still grant the member and deny the outsider)" + ); + cleanup(&pool, &s).await; + std::process::exit(1); + } + + println!("\n#################################################################"); + println!("# range-seek authz duplication: per-seek require (BEFORE) vs 0 (AFTER)"); + println!("# seeks/scrub={seeks} (member of a shared drive, viewer grant)"); + println!("#################################################################\n"); + println!("| {:<26} | {:>10} | {:>12} |", "arm", "wall ms", "µs/seek"); + + // WARM: one require warms owner_cache + drive_role_cache (as the handler's + // get_file_with_perms does), then the scrub's per-seek re-checks are moka + // hits — pure CPU/alloc the AFTER path removes. + { + let engine = fresh_engine(&pool); + seek_require(&engine, s.member, s.file_id).await; // warm + let t = Instant::now(); + for _ in 0..seeks { + std::hint::black_box(seek_require(&engine, s.member, s.file_id).await); + } + let el = t.elapsed(); + println!( + "| {:<26} | {:>10.2} | {:>12.2} |", + "BEFORE per-seek (WARM)", + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / seeks as f64 + ); + } + + // COLD: a fresh engine per seek models a cross-drive recipient or a + // drive-role-cache entry that expired mid-scrub (30 s TTL) — each removed + // re-check was a full grant-cascade drive-resolve query. + { + let t = Instant::now(); + for _ in 0..seeks { + let engine = fresh_engine(&pool); + std::hint::black_box(seek_require(&engine, s.member, s.file_id).await); + } + let el = t.elapsed(); + println!( + "| {:<26} | {:>10.2} | {:>12.2} |", + "BEFORE per-seek (COLD)", + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / seeks as f64 + ); + } + + println!( + "| {:<26} | {:>10.2} | {:>12.2} |", + "AFTER per-seek (removed)", 0.0, 0.0 + ); + + cleanup(&pool, &s).await; + println!("\n(AFTER runs zero per-seek authz: the request-level get_file_with_perms"); + println!(" already authorized + recorded the access. WARM = the moka/CPU cost removed"); + println!(" per seek; COLD = the drive-resolve query removed per seek when the cache"); + println!(" isn't warm. notify_file_accessed (a throttled hook call) is likewise"); + println!(" removed per seek. Safety gate: member granted, outsider denied.)"); +} diff --git a/examples/bench_resource_row_map.rs b/examples/bench_resource_row_map.rs new file mode 100644 index 00000000..0f85c960 --- /dev/null +++ b/examples/bench_resource_row_map.rs @@ -0,0 +1,282 @@ +//! `/api/folders/{id}/resources` row→DTO mapping micro-alloc benchmark. +//! +//! The listing maps each `FolderResourceRow` into a `FolderResourceItemDto`. +//! BEFORE cloned `row.name` into the DTO (`name: row.name.clone()`) even +//! though the row is owned by the mapping closure — one avoidable `String` +//! heap alloc per listed folder/file. AFTER computes the name-derived icon / +//! category classes first (they borrow `&row.name`), then MOVES `row.name` +//! into the DTO — the same output, one fewer alloc per row. +//! +//! Run: +//! cargo run --release --features bench --example bench_resource_row_map +//! Tunables (env): BENCH_ROWS (500). + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::env; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use chrono::{DateTime, TimeZone, Utc}; +use oxicloud::application::dtos::display_helpers::{ + category_for, format_file_size, icon_class_for, icon_special_class_for, intern_display, + intern_mime, +}; +use oxicloud::application::dtos::file_dto::FileDto; +use oxicloud::application::dtos::folder_dto::{FolderDto, FolderResourceRow}; +use oxicloud::domain::entities::file::File; +use uuid::Uuid; + +static ALLOC_CALLS: AtomicU64 = AtomicU64::new(0); + +struct CountingAlloc; + +unsafe impl GlobalAlloc for CountingAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.realloc(ptr, layout, new_size) } + } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + ALLOC_CALLS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc_zeroed(layout) } + } +} + +#[global_allocator] +static GLOBAL: CountingAlloc = CountingAlloc; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn rows(n: usize) -> Vec { + let ts: DateTime = Utc.timestamp_opt(1_700_000_000, 0).unwrap(); + (0..n) + .map(|i| { + let is_folder = i % 4 == 0; + FolderResourceRow { + resource_type: if is_folder { "folder" } else { "file" }.to_string(), + id: Uuid::new_v4(), + name: if is_folder { + format!("Folder {i:05}") + } else { + format!("document-{i:05}.pdf") + }, + parent_id: Some(Uuid::new_v4()), + mime_type: if is_folder { + None + } else { + Some("application/pdf".to_string()) + }, + size: if is_folder { -1 } else { 4096 }, + created_at: ts, + modified_at: ts, + drive_id: Uuid::new_v4(), + blob_hash: if is_folder { + None + } else { + Some("a".repeat(64)) + }, + sort_str: format!("row {i}"), + type_order: 0, + folder_first: if is_folder { 0 } else { 1 }, + } + }) + .collect() +} + +/// (name, icon_class, category) triple extracted from each produced DTO — the +/// fields the move-vs-clone touches. Used for the equivalence gate. +type Probe = (String, std::sync::Arc, std::sync::Arc); + +/// BEFORE — verbatim: `name: row.name.clone()` in both branches. +fn map_before(rows: Vec) -> Vec { + rows.into_iter() + .map(|row| { + if row.resource_type == "folder" { + let resource_id = row.id.to_string(); + let dto = FolderDto { + etag: resource_id.clone(), + id: resource_id, + name: row.name.clone(), + path: String::new(), + parent_id: row.parent_id.map(|u| u.to_string()), + drive_id: row.drive_id, + created_at: row.created_at.timestamp() as u64, + modified_at: row.modified_at.timestamp() as u64, + is_root: false, + icon_class: intern_display("fas fa-folder"), + icon_special_class: intern_display("folder-icon"), + category: intern_display("Folder"), + created_by: None, + updated_by: None, + }; + (dto.name, dto.icon_class, dto.category) + } else { + let mime = row + .mime_type + .as_deref() + .unwrap_or("application/octet-stream"); + let size_bytes = row.size.max(0) as u64; + let modified_at_u = row.modified_at.timestamp() as u64; + let content_hash = row.blob_hash.clone().unwrap_or_default(); + let etag = if content_hash.is_empty() { + String::new() + } else { + File::compute_etag(&content_hash, modified_at_u) + }; + let dto = FileDto { + id: row.id.to_string(), + name: row.name.clone(), + path: String::new(), + size: size_bytes, + mime_type: intern_mime(mime), + folder_id: row.parent_id.map(|u| u.to_string()), + created_at: row.created_at.timestamp() as u64, + modified_at: row.modified_at.timestamp() as u64, + icon_class: intern_display(icon_class_for(&row.name, mime)), + icon_special_class: intern_display(icon_special_class_for(&row.name, mime)), + category: intern_display(category_for(&row.name, mime)), + size_formatted: format_file_size(size_bytes), + sort_date: None, + content_hash, + etag, + created_by: None, + updated_by: None, + }; + (dto.name, dto.icon_class, dto.category) + } + }) + .collect() +} + +/// AFTER — icons/category first (borrow `&row.name`), then move `row.name`. +fn map_after(rows: Vec) -> Vec { + rows.into_iter() + .map(|row| { + if row.resource_type == "folder" { + let resource_id = row.id.to_string(); + let dto = FolderDto { + etag: resource_id.clone(), + id: resource_id, + name: row.name, + path: String::new(), + parent_id: row.parent_id.map(|u| u.to_string()), + drive_id: row.drive_id, + created_at: row.created_at.timestamp() as u64, + modified_at: row.modified_at.timestamp() as u64, + is_root: false, + icon_class: intern_display("fas fa-folder"), + icon_special_class: intern_display("folder-icon"), + category: intern_display("Folder"), + created_by: None, + updated_by: None, + }; + (dto.name, dto.icon_class, dto.category) + } else { + let mime = row + .mime_type + .as_deref() + .unwrap_or("application/octet-stream"); + let size_bytes = row.size.max(0) as u64; + let modified_at_u = row.modified_at.timestamp() as u64; + let content_hash = row.blob_hash.clone().unwrap_or_default(); + let etag = if content_hash.is_empty() { + String::new() + } else { + File::compute_etag(&content_hash, modified_at_u) + }; + let icon_class = intern_display(icon_class_for(&row.name, mime)); + let icon_special_class = intern_display(icon_special_class_for(&row.name, mime)); + let category = intern_display(category_for(&row.name, mime)); + let dto = FileDto { + id: row.id.to_string(), + name: row.name, + path: String::new(), + size: size_bytes, + mime_type: intern_mime(mime), + folder_id: row.parent_id.map(|u| u.to_string()), + created_at: row.created_at.timestamp() as u64, + modified_at: row.modified_at.timestamp() as u64, + icon_class, + icon_special_class, + category, + size_formatted: format_file_size(size_bytes), + sort_date: None, + content_hash, + etag, + created_by: None, + updated_by: None, + }; + (dto.name, dto.icon_class, dto.category) + } + }) + .collect() +} + +fn main() { + let n: usize = env_or("BENCH_ROWS", 500); + + // Equivalence gate: identical (name, icon_class, category) for every row. + if map_before(rows(n)) != map_after(rows(n)) { + eprintln!("EQUIVALENCE GATE FAILED: mapping output differs"); + std::process::exit(1); + } + + // Warm the string interner so its first-sight allocs sit outside the + // measured windows (they're identical for both arms anyway). + std::hint::black_box(map_before(rows(n))); + std::hint::black_box(map_after(rows(n))); + + let a0 = ALLOC_CALLS.load(Ordering::Relaxed); + let t = Instant::now(); + std::hint::black_box(map_before(rows(n))); + let before_ms = t.elapsed().as_secs_f64() * 1e3; + let before_allocs = ALLOC_CALLS.load(Ordering::Relaxed) - a0; + + let a1 = ALLOC_CALLS.load(Ordering::Relaxed); + let t = Instant::now(); + std::hint::black_box(map_after(rows(n))); + let after_ms = t.elapsed().as_secs_f64() * 1e3; + let after_allocs = ALLOC_CALLS.load(Ordering::Relaxed) - a1; + + // Both arms build the same `rows(n)` input inside the timed window, so the + // input allocs are equal and cancel in the delta; the difference is the + // per-row name clone the AFTER path avoids. + println!("\n#################################################################"); + println!("# resources row→DTO mapping: clone name vs move name"); + println!("# rows={n}"); + println!("#################################################################\n"); + println!( + "| {:<20} | {:>12} | {:>10} | {:>14} |", + "arm", "allocs", "wall ms", "allocs/row" + ); + println!( + "| {:<20} | {:>12} | {:>10.3} | {:>14.3} |", + "BEFORE (clone)", + before_allocs, + before_ms, + before_allocs as f64 / n as f64 + ); + println!( + "| {:<20} | {:>12} | {:>10.3} | {:>14.3} |", + "AFTER (move)", + after_allocs, + after_ms, + after_allocs as f64 / n as f64 + ); + println!( + "\nSaved {} allocs ({:.2}/row) — the per-row name clone removed.", + before_allocs.saturating_sub(after_allocs), + (before_allocs.saturating_sub(after_allocs)) as f64 / n as f64 + ); +} diff --git a/frontend/src/lib/utils/photoTimeline.bench.test.ts b/frontend/src/lib/utils/photoTimeline.bench.test.ts new file mode 100644 index 00000000..bedf1e32 --- /dev/null +++ b/frontend/src/lib/utils/photoTimeline.bench.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from 'vitest'; +import type { PhotoItem } from '$lib/api/endpoints/photos'; +import { + PhotoTimeline, + buildPhotoRows, + type GroupMode, + type LayoutMode, + type TimelineConfig +} from './photoTimeline'; + +/** + * Benchmark gate for the incremental photo timeline (PhotoTimeline) that + * replaced the photos view's `groups`→`photoRows` derive chain. + * + * Audit finding: `loadMore` does `items = [...items, ...page]` (60/page), and + * both `groups` (O(N), a `new Date()` per photo) and `photoRows` (O(N) row + * layout) are `$derived` over the whole accumulated list — so paging to photo + * N re-groups + re-lays-out everything loaded so far, Σ ≈ O(N²/60) main-thread + * work during the scroll (the same class ROUND6 fixed for the files listing). + * Since pages arrive newest-first, grouping is append-only; PhotoTimeline + * re-buckets only the fresh page and re-lays-out only the groups that changed. + * + * Gates: + * 1. Equivalence — at EVERY page of the drain, the incremental output is + * deep-equal to the verbatim full-rebuild reference (buildPhotoRows), for + * both layouts; plus config-change, deletion and width=0 fall back to a + * correct full rebuild. + * 2. Perf — grouping work (timestamp reads) collapses from Σ O(N²/60) to O(N) + * across the drain (deterministic count), and wall drops ≥3x. + */ + +const DAY = 86_400; // seconds + +/** A photo with a descending sort_date and a deterministic aspect ratio. */ +function photo(i: number): PhotoItem { + // Newest-first: photo 0 is most recent; ~half a day apart spans ~4 years + // over 3k photos, so month/day buckets are bounded (realistic library). + const sortDate = 1_700_000_000 - i * (DAY / 2); + const w = 200 + ((i * 37) % 400); + const h = 200 + ((i * 53) % 300); + return { + category: 'image', + created_at: sortDate, + icon_class: '', + icon_special_class: '', + id: `p-${i.toString().padStart(6, '0')}`, + mime_type: 'image/jpeg', + modified_at: sortDate, + name: `photo ${i}.jpg`, + created_by: null, + updated_by: null, + folder_id: 'f', + path: `/photo ${i}.jpg`, + size: 1000, + size_formatted: '1 KB', + sort_date: sortDate, + etag: `e${i}`, + content_hash: `h${i}`, + width: w, + height: h + } as PhotoItem; +} + +/** Instrumented config: counts every timestamp read (the grouping hot op). */ +function makeConfig( + groupMode: GroupMode, + layoutMode: LayoutMode, + width: number, + counter?: { n: number } +): TimelineConfig { + const timestampOf = (p: PhotoItem) => { + if (counter) counter.n++; + const v = p.sort_date || p.created_at || 0; + return v < 1e12 ? v * 1000 : v; + }; + // Stable label fn (reference identity matters for the config-unchanged path). + const labelOf = (d: Date, mode: GroupMode) => + mode === 'year' + ? `${d.getFullYear()}` + : mode === 'month' + ? `${d.getFullYear()}-${d.getMonth() + 1}` + : `${d.getFullYear()}-${d.getMonth() + 1}-${d.getDate()}`; + return { groupMode, layoutMode, width, mobile: false, timestampOf, labelOf }; +} + +const PAGE = 60; +const PAGES = 50; // 3 000-photo drain +const WIDTH = 1200; + +describe('incremental photo timeline (benchmark gate)', () => { + for (const layout of ['square', 'justified'] as LayoutMode[]) { + it(`stays deep-equal to the full rebuild at every page — ${layout}`, () => { + const all = Array.from({ length: PAGE * PAGES }, (_, i) => photo(i)); + const cfg = makeConfig('month', layout, WIDTH); + const timeline = new PhotoTimeline(); + for (let p = 1; p <= PAGES; p++) { + const cumulative = all.slice(0, p * PAGE); + const incremental = timeline.sync(cumulative, cfg); + const reference = buildPhotoRows(cumulative, cfg); + expect(incremental, `page ${p}`).toEqual(reference); + } + }); + } + + it('falls back to a correct full rebuild on config change, deletion and width=0', () => { + const all = Array.from({ length: 600 }, (_, i) => photo(i)); + const timeline = new PhotoTimeline(); + const monthSquare = makeConfig('month', 'square', WIDTH); + + // Drain a few pages, then flip layout — must equal a fresh full rebuild. + timeline.sync(all.slice(0, 300), monthSquare); + const justified = makeConfig('month', 'justified', WIDTH); + expect(timeline.sync(all.slice(0, 300), justified)).toEqual( + buildPhotoRows(all.slice(0, 300), justified) + ); + + // Change group mode. + const yearJust = makeConfig('year', 'justified', WIDTH); + expect(timeline.sync(all.slice(0, 300), yearJust)).toEqual( + buildPhotoRows(all.slice(0, 300), yearJust) + ); + + // Deletion (list shrinks / prefix changes) → rebuild. + const shrunk = all.slice(0, 300).filter((_, i) => i % 7 !== 0); + expect(timeline.sync(shrunk, yearJust)).toEqual(buildPhotoRows(shrunk, yearJust)); + + // width=0 yields [] and doesn't wedge the next positive-width sync. + const zero = makeConfig('year', 'justified', 0); + expect(timeline.sync(shrunk, zero)).toEqual([]); + expect(timeline.sync(shrunk, yearJust)).toEqual(buildPhotoRows(shrunk, yearJust)); + }); + + it('collapses grouping work from Σ O(N²/page) to O(N) and runs ≥3x faster', () => { + const N = PAGE * PAGES; + const all = Array.from({ length: N }, (_, i) => photo(i)); + + // AFTER: incremental — each photo is bucketed exactly once across the drain. + const afterCounter = { n: 0 }; + const afterCfg = makeConfig('month', 'square', WIDTH, afterCounter); + const timeline = new PhotoTimeline(); + const t1 = performance.now(); + for (let p = 1; p <= PAGES; p++) timeline.sync(all.slice(0, p * PAGE), afterCfg); + const afterMs = performance.now() - t1; + + // BEFORE: full rebuild per page — re-buckets the whole cumulative list. + const beforeCounter = { n: 0 }; + const beforeCfg = makeConfig('month', 'square', WIDTH, beforeCounter); + const t0 = performance.now(); + for (let p = 1; p <= PAGES; p++) buildPhotoRows(all.slice(0, p * PAGE), beforeCfg); + const beforeMs = performance.now() - t0; + + console.info( + `photo timeline ${PAGES}×${PAGE}: before ${beforeCounter.n} timestamp reads / ${beforeMs.toFixed(1)} ms — after ${afterCounter.n} reads / ${afterMs.toFixed(1)} ms (${(beforeCounter.n / afterCounter.n).toFixed(1)}x fewer reads, ${(beforeMs / afterMs).toFixed(1)}x wall)` + ); + + // Incremental buckets each photo once: exactly N reads. + expect(afterCounter.n).toBe(N); + // Full rebuild is quadratic: Σ_{p=1..P} p·PAGE. + expect(beforeCounter.n).toBe((PAGES * (PAGES + 1) * PAGE) / 2); + expect(afterCounter.n).toBeLessThan(beforeCounter.n / 5); + expect(afterMs).toBeLessThan(beforeMs / 3); + }); +}); diff --git a/frontend/src/lib/utils/photoTimeline.ts b/frontend/src/lib/utils/photoTimeline.ts new file mode 100644 index 00000000..ed53abf7 --- /dev/null +++ b/frontend/src/lib/utils/photoTimeline.ts @@ -0,0 +1,279 @@ +/** + * Photo-timeline grouping + row layout, extracted from the photos view so the + * O(N²) accumulation of its `groups`/`photoRows` derives can be replaced with + * an incremental builder (and unit/benchmark-tested off the Svelte reactive + * graph). + * + * Photos arrive newest-first (`media_sort_date DESC`), so each fetched page + * only ever extends the last date bucket or appends new buckets after it — + * never mutates an earlier group. {@link PhotoTimeline} exploits that: an + * append re-buckets only the new page and recomputes rows only for the groups + * that actually changed, keeping a full scroll O(N) instead of O(N²). + * + * The pure {@link buildPhotoRows} is the verbatim reference (what the old + * `groups`→`photoRows` derive chain produced); the benchmark gate asserts the + * incremental builder stays byte-for-byte equal to it. + */ +import type { PhotoItem } from '$lib/api/endpoints/photos'; + +export type GroupMode = 'day' | 'month' | 'year'; +export type LayoutMode = 'square' | 'justified'; + +export interface JustifiedTile { + file: PhotoItem; + w: number; + h: number; +} + +export type PhotoRow = + | { kind: 'header'; key: string; height: number; label: string; count: number } + | { kind: 'tiles'; key: string; height: number; gap: number; tiles: JustifiedTile[] }; + +/** Layout constants — mirror the photos view's original values exactly. */ +export const SQUARE_GAP = 4; // .25rem, matches the old grid gap +export const SQUARE_MIN = 144; // 9rem minmax floor +export const JUSTIFIED_GAP = 8; // .photos-jrow margin-bottom +export const HEADER_H = 44; + +export interface TimelineConfig { + groupMode: GroupMode; + layoutMode: LayoutMode; + /** Usable content width of the grid, in px. */ + width: number; + /** `(max-width: 768px)` — selects the 150px vs 200px justified target. */ + mobile: boolean; + /** EXIF-aware capture timestamp (ms). Injected so the module stays pure. */ + timestampOf: (p: PhotoItem) => number; + /** Locale-aware bucket label for a group's representative date. */ + labelOf: (d: Date, mode: GroupMode) => string; +} + +interface Group { + key: string; + label: string; + photos: PhotoItem[]; +} + +/** Year/month/day bucket key for a date under `groupMode` (verbatim). */ +export function bucketKey(d: Date, groupMode: GroupMode): string { + const y = d.getFullYear(); + if (groupMode === 'year') return `${y}`; + const m = `${d.getMonth() + 1}`.padStart(2, '0'); + if (groupMode === 'month') return `${y}-${m}`; + return `${y}-${m}-${`${d.getDate()}`.padStart(2, '0')}`; +} + +/** + * Pack files into justified rows (Flickr-style): each full row is scaled to + * fill `width` while preserving every tile's aspect ratio. Missing dimensions + * fall back to 1:1. Verbatim port of the photos view's `justifiedRows`, with + * the `matchMedia` read hoisted to the `mobile` flag so it's testable. + */ +export function justifiedRows( + files: PhotoItem[], + width: number, + mobile: boolean +): Array<{ height: number; tiles: JustifiedTile[] }> { + const gap = 8; + const target = mobile ? 150 : 200; + const rows: Array<{ height: number; tiles: JustifiedTile[] }> = []; + let cur: Array<{ file: PhotoItem; aspect: number }> = []; + let aspectSum = 0; + for (const file of files) { + let aspect = file.width && file.height ? file.width / file.height : 1; + if (!Number.isFinite(aspect) || aspect <= 0) aspect = 1; + aspect = Math.min(Math.max(aspect, 0.4), 3); + cur.push({ file, aspect }); + aspectSum += aspect; + const rowWidth = aspectSum * target + (cur.length - 1) * gap; + if (rowWidth >= width) { + const h = (width - (cur.length - 1) * gap) / aspectSum; + rows.push({ + height: Math.round(h), + tiles: cur.map((tt) => ({ + file: tt.file, + w: Math.max(1, Math.round(tt.aspect * h)), + h: Math.round(h) + })) + }); + cur = []; + aspectSum = 0; + } + } + if (cur.length) { + rows.push({ + height: target, + tiles: cur.map((tt) => ({ + file: tt.file, + w: Math.max(1, Math.round(tt.aspect * target)), + h: target + })) + }); + } + return rows; +} + +/** Columns + cell size for the square layout at width `W` (verbatim). */ +function squareGeometry(W: number): { cols: number; cell: number } { + const cols = Math.max(1, Math.floor((W + SQUARE_GAP) / (SQUARE_MIN + SQUARE_GAP))); + const cell = (W - (cols - 1) * SQUARE_GAP) / cols; + return { cols, cell }; +} + +/** Flatten one group into its header + tile rows (verbatim per-group body). */ +function groupToRows(g: Group, cfg: TimelineConfig, cols: number, cell: number): PhotoRow[] { + const rows: PhotoRow[] = [ + { kind: 'header', key: `h:${g.key}`, height: HEADER_H, label: g.label, count: g.photos.length } + ]; + if (cfg.layoutMode === 'justified') { + const jrows = justifiedRows(g.photos, cfg.width, cfg.mobile); + for (let ri = 0; ri < jrows.length; ri++) { + rows.push({ + kind: 'tiles', + key: `${g.key}:j${ri}`, + height: jrows[ri].height + JUSTIFIED_GAP, + gap: JUSTIFIED_GAP, + tiles: jrows[ri].tiles + }); + } + } else { + for (let i = 0; i < g.photos.length; i += cols) { + const tiles = g.photos.slice(i, i + cols).map((file) => ({ file, w: cell, h: cell })); + rows.push({ + kind: 'tiles', + key: `${g.key}:s${i}`, + height: cell + SQUARE_GAP, + gap: SQUARE_GAP, + tiles + }); + } + } + return rows; +} + +/** Bucket `items` into date groups, first-appearance order (verbatim). */ +function buildGroups(items: PhotoItem[], cfg: TimelineConfig): Group[] { + const out: Group[] = []; + const index = new Map(); + for (const p of items) { + const d = new Date(cfg.timestampOf(p)); + const key = bucketKey(d, cfg.groupMode); + let i = index.get(key); + if (i === undefined) { + i = out.length; + index.set(key, i); + out.push({ key, label: cfg.labelOf(d, cfg.groupMode), photos: [] }); + } + out[i].photos.push(p); + } + return out; +} + +/** + * Verbatim reference: the flat `PhotoRow[]` the old `groups`→`photoRows` + * derive chain produced for `items` under `cfg`. Returns `[]` for a + * non-positive width, matching the old guard. The benchmark gate holds the + * incremental builder equal to this. + */ +export function buildPhotoRows(items: PhotoItem[], cfg: TimelineConfig): PhotoRow[] { + if (cfg.width <= 0) return []; + const { cols, cell } = squareGeometry(cfg.width); + const rows: PhotoRow[] = []; + for (const g of buildGroups(items, cfg)) { + rows.push(...groupToRows(g, cfg, cols, cell)); + } + return rows; +} + +function configEq(a: TimelineConfig, b: TimelineConfig): boolean { + return ( + a.groupMode === b.groupMode && + a.layoutMode === b.layoutMode && + a.width === b.width && + a.mobile === b.mobile && + a.timestampOf === b.timestampOf && + a.labelOf === b.labelOf + ); +} + +/** + * Incremental photo-timeline builder. Call {@link sync} with the current item + * list and config on every change; it detects the common case — the list grew + * by appending a page while config is unchanged — and re-buckets only the new + * items + re-lays-out only the groups that changed, reusing every untouched + * group's cached rows. Any other change (config, deletion, filter toggle, + * non-append) falls back to a full rebuild, so the result is always identical + * to {@link buildPhotoRows}. + */ +export class PhotoTimeline { + #cfg: TimelineConfig | null = null; + #groups: Group[] = []; + /** Items already bucketed — the append cursor into the last synced list. */ + #groupedItems: PhotoItem[] = []; + /** group.key → its cached rows for the current config. */ + #rowCache = new Map(); + #geom = { cols: 1, cell: 0 }; + + /** Whether `next` extends `prev` (same prefix objects + strictly longer). */ + #isAppend(prev: PhotoItem[], next: PhotoItem[]): boolean { + if (next.length <= prev.length) return false; + // Prefix identity via the boundary object — O(1), the list is only ever + // mutated by appending or by replacing with a filtered copy. + return prev.length === 0 || next[prev.length - 1] === prev[prev.length - 1]; + } + + #rebuild(items: PhotoItem[], cfg: TimelineConfig): void { + this.#cfg = cfg; + this.#groups = cfg.width > 0 ? buildGroups(items, cfg) : []; + this.#groupedItems = items; + this.#rowCache.clear(); + this.#geom = squareGeometry(cfg.width); + } + + #extend(items: PhotoItem[], cfg: TimelineConfig): void { + const fresh = items.slice(this.#groupedItems.length); + // The last existing group may grow, so its cached rows are stale. + if (this.#groups.length > 0) { + this.#rowCache.delete(this.#groups[this.#groups.length - 1].key); + } + for (const p of fresh) { + const d = new Date(cfg.timestampOf(p)); + const key = bucketKey(d, cfg.groupMode); + const last = this.#groups[this.#groups.length - 1]; + if (last && last.key === key) { + last.photos.push(p); + } else { + this.#groups.push({ key, label: cfg.labelOf(d, cfg.groupMode), photos: [p] }); + } + } + this.#groupedItems = items; + } + + sync(items: PhotoItem[], cfg: TimelineConfig): PhotoRow[] { + if (cfg.width <= 0) { + // Keep the item cursor so a later positive width rebuilds from scratch. + this.#cfg = cfg; + this.#groups = []; + this.#groupedItems = items; + this.#rowCache.clear(); + return []; + } + if (this.#cfg && configEq(this.#cfg, cfg) && this.#isAppend(this.#groupedItems, items)) { + this.#extend(items, cfg); + } else { + this.#rebuild(items, cfg); + } + + const { cols, cell } = this.#geom; + const out: PhotoRow[] = []; + for (const g of this.#groups) { + let rows = this.#rowCache.get(g.key); + if (rows === undefined) { + rows = groupToRows(g, cfg, cols, cell); + this.#rowCache.set(g.key, rows); + } + for (const r of rows) out.push(r); + } + return out; + } +} diff --git a/frontend/src/routes/photos/+page.svelte b/frontend/src/routes/photos/+page.svelte index 7b4cdf07..b823951d 100644 --- a/frontend/src/routes/photos/+page.svelte +++ b/frontend/src/routes/photos/+page.svelte @@ -17,6 +17,12 @@ import { filterDotfiles } from '$lib/utils/dotfileFilter'; import { dateTimeFormatFor } from '$lib/utils/display'; import { isVideo, photoTimestamp } from '$lib/utils/media'; + import { + PhotoTimeline, + type GroupMode, + type LayoutMode, + type PhotoRow + } from '$lib/utils/photoTimeline'; type Tab = 'moments' | 'places' | 'people'; let tab = $state('moments'); @@ -49,8 +55,6 @@ /** Usable content width of the grid, for the justified layout. */ let gridWidth = $state(0); - type GroupMode = 'day' | 'month' | 'year'; - type LayoutMode = 'square' | 'justified'; const GROUP_KEY = 'oxi-photos-group'; const LAYOUT_KEY = 'oxi-photos-layout'; let groupMode = $state('month'); @@ -64,18 +68,10 @@ else if (tab === 'people') void peopleView.load(); }); - /** EXIF-aware timestamp (seconds → ms), matching the OLD grouping logic. */ - function bucketKey(d: Date): string { - const y = d.getFullYear(); - if (groupMode === 'year') return `${y}`; - const m = `${d.getMonth() + 1}`.padStart(2, '0'); - if (groupMode === 'month') return `${y}-${m}`; - return `${y}-${m}-${`${d.getDate()}`.padStart(2, '0')}`; - } - - function bucketLabel(d: Date): string { - if (groupMode === 'year') return `${d.getFullYear()}`; - if (groupMode === 'month') + /** Locale-aware label for a bucket's representative date. */ + function bucketLabel(d: Date, mode: GroupMode): string { + if (mode === 'year') return `${d.getFullYear()}`; + if (mode === 'month') return dateTimeFormatFor(undefined, { year: 'numeric', month: 'long' }).format(d); return dateTimeFormatFor(undefined, { weekday: 'long', @@ -85,132 +81,32 @@ }).format(d); } - const groups = $derived.by(() => { - const out: Array<{ key: string; label: string; photos: PhotoItem[] }> = []; - // Transient scratch map built inside $derived.by and discarded — not reactive state. - // eslint-disable-next-line svelte/prefer-svelte-reactivity - const index = new Map(); - for (const p of visibleItems) { - const d = new Date(photoTimestamp(p)); - const key = bucketKey(d); - let i = index.get(key); - if (i === undefined) { - i = out.length; - index.set(key, i); - out.push({ key, label: bucketLabel(d), photos: [] }); - } - out[i].photos.push(p); - } - return out; - }); - - interface JustifiedTile { - file: PhotoItem; - w: number; - h: number; - } - - /** - * Pack files into justified rows (Flickr-style): each full row is scaled to - * fill `width` while preserving every tile's aspect ratio. Missing dimensions - * fall back to 1:1. - */ - function justifiedRows( - files: PhotoItem[], - width: number - ): Array<{ height: number; tiles: JustifiedTile[] }> { - const gap = 8; - const target = window.matchMedia('(max-width: 768px)').matches ? 150 : 200; - const rows: Array<{ height: number; tiles: JustifiedTile[] }> = []; - let cur: Array<{ file: PhotoItem; aspect: number }> = []; - let aspectSum = 0; - for (const file of files) { - let aspect = file.width && file.height ? file.width / file.height : 1; - if (!Number.isFinite(aspect) || aspect <= 0) aspect = 1; - aspect = Math.min(Math.max(aspect, 0.4), 3); - cur.push({ file, aspect }); - aspectSum += aspect; - const rowWidth = aspectSum * target + (cur.length - 1) * gap; - if (rowWidth >= width) { - const h = (width - (cur.length - 1) * gap) / aspectSum; - rows.push({ - height: Math.round(h), - tiles: cur.map((tt) => ({ - file: tt.file, - w: Math.max(1, Math.round(tt.aspect * h)), - h: Math.round(h) - })) - }); - cur = []; - aspectSum = 0; - } - } - if (cur.length) { - rows.push({ - height: target, - tiles: cur.map((tt) => ({ - file: tt.file, - w: Math.max(1, Math.round(tt.aspect * target)), - h: target - })) - }); - } - return rows; - } - // ── Virtualized row model ──────────────────────────────────────────────── - // Flatten the groups into a single list of fixed-height rows (a date header - // or a strip of sized tiles), so VirtualRows can window the whole timeline — - // only the rows near the viewport are mounted, regardless of library size. - const SQUARE_GAP = 4; // .25rem, matches the old grid gap - const SQUARE_MIN = 144; // 9rem minmax floor - const JUSTIFIED_GAP = 8; // .photos-jrow margin-bottom - const HEADER_H = 44; - - type PhotoRow = - | { kind: 'header'; key: string; height: number; label: string; count: number } - | { kind: 'tiles'; key: string; height: number; gap: number; tiles: JustifiedTile[] }; - - const photoRows = $derived.by(() => { - const W = gridWidth; - if (W <= 0) return []; - const rows: PhotoRow[] = []; - const cols = Math.max(1, Math.floor((W + SQUARE_GAP) / (SQUARE_MIN + SQUARE_GAP))); - const cell = (W - (cols - 1) * SQUARE_GAP) / cols; - for (const g of groups) { - rows.push({ - kind: 'header', - key: `h:${g.key}`, - height: HEADER_H, - label: g.label, - count: g.photos.length - }); - if (layoutMode === 'justified') { - const jrows = justifiedRows(g.photos, W); - for (let ri = 0; ri < jrows.length; ri++) { - rows.push({ - kind: 'tiles', - key: `${g.key}:j${ri}`, - height: jrows[ri].height + JUSTIFIED_GAP, - gap: JUSTIFIED_GAP, - tiles: jrows[ri].tiles - }); - } - } else { - for (let i = 0; i < g.photos.length; i += cols) { - const tiles = g.photos.slice(i, i + cols).map((file) => ({ file, w: cell, h: cell })); - rows.push({ - kind: 'tiles', - key: `${g.key}:s${i}`, - height: cell + SQUARE_GAP, - gap: SQUARE_GAP, - tiles - }); - } - } - } - return rows; - }); + // Flatten the date groups into a single list of fixed-height rows (a header + // or a strip of sized tiles) that VirtualRows windows. Because pages arrive + // newest-first, each append only extends the last group or adds new ones, so + // PhotoTimeline re-buckets only the fresh page and re-lays-out only the + // groups that changed — a full scroll stays O(N), not O(N²) (the old + // `groups`→`photoRows` derive chain re-grouped + re-packed the whole library + // on every 60-item page). See photoGrouping.bench.test.ts. + // `sync` mutates the timeline's (non-reactive) internal group/row caches and + // returns the flat rows. Driven from `$derived.by` for idempotence: if the + // deps re-fire without an actual append, `sync` sees a non-growing list and + // safely full-rebuilds — same output as the pure `buildPhotoRows`. + const timeline = new PhotoTimeline(); + const photoRows = $derived.by(() => + timeline.sync(visibleItems, { + groupMode, + layoutMode, + width: gridWidth, + mobile: + typeof window !== 'undefined' && + typeof window.matchMedia === 'function' && + window.matchMedia('(max-width: 768px)').matches, + timestampOf: photoTimestamp, + labelOf: bucketLabel + }) + ); async function loadMore() { if (loading || exhausted) return; diff --git a/src/application/services/file_retrieval_service.rs b/src/application/services/file_retrieval_service.rs index de03ac08..882ecb46 100644 --- a/src/application/services/file_retrieval_service.rs +++ b/src/application/services/file_retrieval_service.rs @@ -287,22 +287,6 @@ impl FileRetrievalService { Ok(files.into_iter().map(FileDto::from).collect()) } - /// Range read that first consults the RAM content cache (see - /// [`Self::get_file_range_preloaded`]). - pub async fn get_file_range_preloaded_with_perms( - &self, - dto: &FileDto, - caller_id: Uuid, - start: u64, - end: Option, - ) -> Result { - self.require_file(&dto.id, Permission::Read, caller_id) - .await?; - // Same throttled Recent recording as the streaming variant. - self.notify_file_accessed(caller_id, &dto.id); - self.get_file_range_preloaded(dto, start, end).await - } - /// Range read for HTTP Range Requests, cache-aware. /// /// Media players and PDF viewers fetch these files *exclusively* through diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index f1095780..f729865c 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -712,13 +712,15 @@ impl FileHandler { let disposition = Self::content_disposition(&file_dto.name, &file_dto.mime_type, ¶ms); + // `file_dto` was already Read-authorized (and the access + // recorded) by `get_file_with_perms` above — every seek in + // a media/PDF scrub is a separate Range request, so + // re-authorizing + re-notifying per seek doubled that work + // for nothing. Use the non-perms range read, matching the + // share-landing and WebDAV range paths which authorize once + // then stream (benches/ROUND7.md). match retrieval - .get_file_range_preloaded_with_perms( - &file_dto, - auth_user.id, - start, - Some(end + 1), - ) + .get_file_range_preloaded(&file_dto, start, Some(end + 1)) .await { Ok(content) => { diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index f5c525f4..cc707977 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -476,7 +476,9 @@ pub async fn list_folder_resources( let dto = FolderDto { etag: resource_id.clone(), id: resource_id, - name: row.name.clone(), + // Folders use fixed icon classes (below), so `name` + // is never borrowed again — move it instead of cloning. + name: row.name, path: String::new(), // cleared — share recipients must not see hierarchy parent_id: row.parent_id.map(|u| u.to_string()), drive_id: row.drive_id, @@ -514,20 +516,26 @@ pub async fn list_folder_resources( } else { File::compute_etag(&content_hash, modified_at_u) }; + // Compute the name-derived icon/category classes first + // (they borrow `&row.name`), so `name` can be moved into + // the DTO below instead of cloned — one fewer String + // alloc per file row (benches/ROUND7.md). + let icon_class = intern_display(icon_class_for(&row.name, mime)); + let icon_special_class = + intern_display(icon_special_class_for(&row.name, mime)); + let category = intern_display(category_for(&row.name, mime)); let dto = FileDto { id: row.id.to_string(), - name: row.name.clone(), + name: row.name, path: String::new(), size: size_bytes, mime_type: intern_mime(mime), folder_id: row.parent_id.map(|u| u.to_string()), created_at: row.created_at.timestamp() as u64, modified_at: row.modified_at.timestamp() as u64, - icon_class: intern_display(icon_class_for(&row.name, mime)), - icon_special_class: intern_display(icon_special_class_for( - &row.name, mime, - )), - category: intern_display(category_for(&row.name, mime)), + icon_class, + icon_special_class, + category, size_formatted: format_file_size(size_bytes), sort_date: None, content_hash, From 79b94126be34041bf5da662356129ad2bd104aaa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 14:03:43 +0000 Subject: [PATCH 4/4] =?UTF-8?q?perf(authz):=20round=208=20=E2=80=94=20cach?= =?UTF-8?q?e=20the=20File/Folder=20grant-cascade=20decision=20for=20shared?= =?UTF-8?q?-album=20thumbnails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_thumbnail_impl runs require_permission(Read) on every request. For a drive member that's a drive_role_cache hit, but a shared-album recipient — granted a folder (the album), not drive membership — fails the drive-role precheck and falls through to file_cascade_grant_exists (a role_grants ⋈ folders lpath ancestor query), once per file. Browsers revalidate immutable thumbnails constantly, so the same (recipient, file, Read) decision was recomputed on every thumbnail of every view — ~100 grant queries per 100-photo album per navigate-away-and-back. New cascade_grant_cache ((Subject, Resource, Permission) → bool, 30 s TTL) memoises that decision. The check is NEVER skipped — the ordering is unchanged, authz still runs on every request; only the result is cached, and only after the drive-role precheck fails (so a later drive grant can't be shadowed by a stale entry). Invalidation mirrors drive_role_cache's convention: explicit invalidate_all on every File/Folder set_role/clear_role (immediate revoke on the direct share path), 30 s TTL for the indirect paths (group membership, moves, expiry) "rather than a deep invalidation tree". Bench (bench_thumbnail_cascade_cache) with hard safety gates — recipient allowed, outsider denied, and a clear_role revoke denies the very next check (proving the grant-write flush): 100-photo album revalidation 2576 → 2.70 µs/thumb (~950x), 257.6 → 0.27 ms/view. Validated against the full --cfg integration_tests authz suite (554 tests) + 524 workspace tests, clippy -D warnings clean. Deliberately not done: moving authz after the 304/cache short-circuit (a security-posture change — a revoked user could serve cached thumbnails). With the decision cached, the authz on the 304 path is now a memory hit, so the "zero DB work on a 304" intent is restored without weakening the check. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA --- Cargo.toml | 10 + benches/ROUND8.md | 77 ++++ examples/bench_thumbnail_cascade_cache.rs | 373 +++++++++++++++++++ src/infrastructure/services/pg_acl_engine.rs | 167 +++++++-- 4 files changed, 601 insertions(+), 26 deletions(-) create mode 100644 benches/ROUND8.md create mode 100644 examples/bench_thumbnail_cascade_cache.rs diff --git a/Cargo.toml b/Cargo.toml index c03a0171..7a24e759 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,6 +350,16 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-8 battery ───────────────────────────────────────────────────────────── + +# Shared-album thumbnail authz — folder-grant cascade query per thumbnail vs +# the cascade_grant_cache; includes a revocation safety gate (needs the dev +# Postgres up). +[[example]] +name = "bench_thumbnail_cascade_cache" +path = "examples/bench_thumbnail_cascade_cache.rs" +required-features = ["bench"] + # Round-7 battery ───────────────────────────────────────────────────────────── # Range-seek per-request authz duplication — the per-seek require the range diff --git a/benches/ROUND8.md b/benches/ROUND8.md new file mode 100644 index 00000000..24995f52 --- /dev/null +++ b/benches/ROUND8.md @@ -0,0 +1,77 @@ +# Round 8 — shared-album thumbnail authz: cache the folder-grant cascade decision + +Benchmark-gated, same rule as ROUND2-7: every change ships with a BEFORE/AFTER +benchmark and equivalence/safety gates; an AFTER that doesn't beat its BEFORE +gets rolled back. This round touches the authorization engine, so the bench +carries hard **safety gates** (recipient allowed, outsider denied, and a +revoke-denies-immediately test) and the change is additionally validated +against the full `--cfg integration_tests` authz suite. + +Measured on 4 cores / 15 GiB, local PostgreSQL 16 (fsync off), release profile. + +## Summary + +| # | change | key metric | before → after | +|--:|---|---|---| +| 1 | `cascade_grant_cache` for File/Folder Read checks | shared-album thumbnail revalidation (100-photo) | 2576 → 2.70 µs/thumb (**~950x**); 257.6 → 0.27 ms/view | + +## [1] Shared-album thumbnails — folder-grant cascade query per thumbnail → cached + +`get_thumbnail_impl` runs `require_permission(Read, file)` on every request, +ahead of the ETag-304 and moka/disk cache short-circuits. For the **owner** (or +any drive member) that's a `drive_role_cache` hit — ~1 µs, no query. But a +**shared-album recipient** — someone granted a *folder* (the album), not drive +membership — fails the drive-role precheck in `PgAclEngine::check_inner` and +falls through to `file_cascade_grant_exists`: an `role_grants ⋈ folders` +ltree-ancestor (`lpath @>`) query, once per file. Browsers revalidate immutable +thumbnails constantly (`If-None-Match`), so the same `(recipient, file, Read)` +decision was recomputed on every thumbnail of every view — a shared 100-photo +album cost ~100 grant queries per "navigate away and back". + +The safe fix keeps the check exactly where it is — **authz is never skipped**, +the ordering is unchanged — and memoises only its *result* in a new +`cascade_grant_cache` (`(Subject, Resource, Permission) → bool`, 30 s TTL). It's +consulted only after the drive-role precheck fails, so a caller who later gains +a drive grant short-circuits above it and can't be shadowed by a stale entry. + +**Invalidation** mirrors `drive_role_cache`'s documented convention exactly: +explicit `invalidate_all` on every File/Folder `set_role` / `clear_role` (the +direct share/revoke path — infrequent next to thumbnail reads, so a full flush +is cheap and keeps a revoke *immediate*); the indirect paths (group-membership +changes, resource moves, grant `expires_at` expiry) are caught by the 30 s TTL, +"rather than a deep invalidation tree". + +Safety gates in the bench (hard asserts): the folder-grant recipient is allowed +on every album file, an outsider is denied, and — critically — after a warm +cache serves `allowed`, a `clear_role` on the shared folder makes the very next +check **deny** (proving the grant-write flush; without it the stale `true` +would still serve). Also validated against the full `--cfg integration_tests` +authz suite (grants, nested groups, drive membership, read-only freeze). + +``` +cargo run --release --features bench --example bench_thumbnail_cascade_cache +# thumbs=100 (recipient holds a folder grant, no drive membership) +# arm wall ms µs/thumb +# BEFORE (query/thumb) 257.60 2576.04 <- folder-cascade query per thumbnail +# AFTER cold (first view) 84.18 841.76 <- distinct files miss+populate the cache +# AFTER warm (revalidation) 0.27 2.70 <- all cache hits (~950x vs BEFORE) +# Safety gates PASSED: recipient allowed, outsider denied, clear_role revoke +# denies immediately (grant write flushed the cache). +``` + +## Notes + +- The batched search Read path (`check_files_read_batch`) is unchanged — it + already resolves a page of files in one round-trip and isn't the + per-thumbnail hot path; it neither reads nor writes this cache, so no + consistency coupling is introduced. +- First-view cost is unchanged (distinct files are cache misses that populate + the cache); the win is on revalidation + repeat views, which is where the + thumbnail traffic concentrates. A folder-level cascade cache would also cut + the first-view N-queries to one-per-folder, but needs a file→parent-folder + resolution and a wider invalidation story — deferred. +- The ACL-before-304 *ordering* (running authz before the 304/cache + short-circuits) is left intact — with the cascade decision now cached, the + authz on the revalidation path is a memory hit, so the "zero DB work on a + 304" intent is restored without moving (and thus without weakening) the + security check. diff --git a/examples/bench_thumbnail_cascade_cache.rs b/examples/bench_thumbnail_cascade_cache.rs new file mode 100644 index 00000000..55ad7454 --- /dev/null +++ b/examples/bench_thumbnail_cascade_cache.rs @@ -0,0 +1,373 @@ +//! Shared-album thumbnail authz benchmark — folder-grant cascade query per +//! thumbnail vs the `cascade_grant_cache`. +//! +//! A recipient of a shared folder (a grant on the album folder, NOT drive +//! membership) fails the drive-role precheck in `PgAclEngine::check_inner` and +//! falls through to `file_cascade_grant_exists` — an ltree folder-ancestor +//! grant query — for EVERY file. `get_thumbnail_impl` runs that Read check on +//! every request, and browsers revalidate immutable thumbnails constantly +//! (`If-None-Match`), so the same `(recipient, file, Read)` decision is +//! recomputed again and again: ~one grant query per thumbnail per view. +//! +//! Round 8 memoises that decision in `cascade_grant_cache` (30 s TTL, flushed +//! on any File/Folder grant write). The check still runs on every request — +//! it is never skipped — but after the first query it resolves in-memory. +//! +//! Safety gates (hard asserts, exit 1 on failure): +//! 1. the folder-grant recipient is allowed; an outsider is denied; +//! 2. REVOCATION — after a warm cache serves `allowed`, `clear_role` on the +//! shared folder makes the very next check DENY (proves the grant-write +//! invalidation flushes the cache; without it the stale `true` would +//! still serve). +//! +//! Run (needs Postgres up; reads DATABASE_URL from .env): +//! cargo run --release --features bench --example bench_thumbnail_cascade_cache +//! Tunables (env): BENCH_THUMBS (100), BENCH_POOL (8). + +use std::env; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use oxicloud::application::ports::authorization_ports::AuthorizationEngine; +use oxicloud::domain::services::authorization::{Permission, Resource, Role, Subject}; +use oxicloud::infrastructure::repositories::pg::{ + FileBlobReadRepository, FolderDbRepository, SubjectGroupPgRepository, +}; +use oxicloud::infrastructure::services::dedup_service::DedupService; +use oxicloud::infrastructure::services::local_blob_backend::LocalBlobBackend; +use oxicloud::infrastructure::services::pg_acl_engine::PgAclEngine; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; +use uuid::Uuid; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +struct Seeded { + owner: Uuid, + recipient: Uuid, + outsider: Uuid, + drive_id: Uuid, + root_folder: Uuid, + album_folder: Uuid, + blob_hash: String, + files: Vec, +} + +async fn seed(pool: &PgPool, n_thumbs: usize) -> Seeded { + let mut tx = pool.begin().await.expect("begin"); + let owner: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_thumbowner', 'bench_thumbowner@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed owner"); + let recipient: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_thumbrecip', 'bench_thumbrecip@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed recipient"); + let outsider: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_thumbout', 'bench_thumbout@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed outsider"); + + // Owner's personal drive with a root and an album subfolder. The recipient + // is NOT a drive member — only granted the album folder below, so their + // File checks fall through the drive precheck to the folder cascade. + let drive_id: Uuid = sqlx::query_scalar( + "INSERT INTO storage.drives (kind, default_for_user) VALUES ('personal', $1) RETURNING id", + ) + .bind(owner) + .fetch_one(&mut *tx) + .await + .expect("seed drive"); + let root_folder: Uuid = sqlx::query_scalar( + "INSERT INTO storage.folders (name, path, lpath, drive_id) + VALUES ('Personal', '/Personal', 'benchthumbroot', $1) RETURNING id", + ) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed root"); + sqlx::query("UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2") + .bind(root_folder) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("stamp root"); + let album_folder: Uuid = sqlx::query_scalar( + "INSERT INTO storage.folders (name, path, lpath, drive_id, parent_id) + VALUES ('Album', '/Personal/Album', 'benchthumbroot.album', $1, $2) RETURNING id", + ) + .bind(drive_id) + .bind(root_folder) + .fetch_one(&mut *tx) + .await + .expect("seed album"); + // Owner grant on the drive (personal-drive owner floor), and the recipient + // grant on the ALBUM FOLDER only — the shared-album shape. + sqlx::query( + "INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', $1, 'drive', $2, 'owner'::storage.grant_role, $1)", + ) + .bind(owner) + .bind(drive_id) + .execute(&mut *tx) + .await + .expect("seed owner grant"); + sqlx::query( + "INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', $1, 'folder', $2, 'viewer'::storage.grant_role, $3)", + ) + .bind(recipient) + .bind(album_folder) + .bind(owner) + .execute(&mut *tx) + .await + .expect("seed recipient folder grant"); + + let blob_hash = "benchthumbcascade00000000000000000000000000000000000000000000b4".to_string(); + sqlx::query("INSERT INTO storage.blobs (hash, size, ref_count) VALUES ($1, 4096, 1)") + .bind(&blob_hash) + .execute(&mut *tx) + .await + .expect("seed blob"); + let mut files = Vec::with_capacity(n_thumbs); + for i in 0..n_thumbs { + let id: Uuid = sqlx::query_scalar( + "INSERT INTO storage.files (name, folder_id, blob_hash, size, mime_type, drive_id) + VALUES ($1, $2, $3, 4096, 'image/jpeg', $4) RETURNING id", + ) + .bind(format!("photo-{i:04}.jpg")) + .bind(album_folder) + .bind(&blob_hash) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .expect("seed file"); + files.push(id); + } + tx.commit().await.expect("commit"); + Seeded { + owner, + recipient, + outsider, + drive_id, + root_folder, + album_folder, + blob_hash, + files, + } +} + +async fn cleanup(pool: &PgPool, s: &Seeded) { + let _ = sqlx::query( + "DELETE FROM storage.role_grants WHERE resource_id IN ($1, $2) OR resource_id = ANY($3)", + ) + .bind(s.drive_id) + .bind(s.album_folder) + .bind(&s.files) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.files WHERE drive_id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.drives WHERE id = $1") + .bind(s.drive_id) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.folders WHERE id IN ($1, $2)") + .bind(s.album_folder) + .bind(s.root_folder) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.blobs WHERE hash = $1") + .bind(&s.blob_hash) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.users WHERE id IN ($1, $2, $3)") + .bind(s.owner) + .bind(s.recipient) + .bind(s.outsider) + .execute(pool) + .await; +} + +fn fresh_engine(pool: &Arc) -> Arc { + let folder_repo = Arc::new(FolderDbRepository::new(pool.clone())); + let backend = Arc::new(LocalBlobBackend::new(std::path::Path::new( + "/tmp/bench-thumbcascade-blobs", + ))); + let dedup = Arc::new(DedupService::new(backend, pool.clone(), pool.clone())); + let file_repo = Arc::new(FileBlobReadRepository::new( + pool.clone(), + dedup, + folder_repo.clone(), + )); + let group_repo = Arc::new(SubjectGroupPgRepository::new(pool.clone())); + Arc::new(PgAclEngine::new( + pool.clone(), + folder_repo, + file_repo, + group_repo, + )) +} + +async fn allowed(engine: &Arc, caller: Uuid, file: Uuid) -> bool { + engine + .require( + Subject::User(caller), + Permission::Read, + Resource::File(file), + ) + .await + .is_ok() +} + +#[tokio::main(flavor = "multi_thread")] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let thumbs: usize = env_or("BENCH_THUMBS", 100); + let pool_size: u32 = env_or("BENCH_POOL", 8); + + let pool = Arc::new( + PgPoolOptions::new() + .max_connections(pool_size) + .min_connections(pool_size) + .acquire_timeout(Duration::from_secs(10)) + .connect(&url) + .await + .expect("connect Postgres"), + ); + + let s = seed(&pool, thumbs).await; + + // ── Safety gate 1: recipient allowed on every file, outsider denied ── + { + let engine = fresh_engine(&pool); + for &f in &s.files { + if !allowed(&engine, s.recipient, f).await { + eprintln!("SAFETY GATE FAILED: folder-grant recipient denied a file in the album"); + cleanup(&pool, &s).await; + std::process::exit(1); + } + } + if allowed(&engine, s.outsider, s.files[0]).await { + eprintln!("SAFETY GATE FAILED: outsider was allowed"); + cleanup(&pool, &s).await; + std::process::exit(1); + } + } + + // ── Safety gate 2: revocation flushes the cache (immediate deny) ── + { + let engine = fresh_engine(&pool); + // Warm: caches (recipient, File[0], Read) → true. + assert!(allowed(&engine, s.recipient, s.files[0]).await); + // Revoke the album share through the real grant-write path. + engine + .clear_role(Subject::User(s.recipient), Resource::Folder(s.album_folder)) + .await + .expect("clear_role"); + // Next check MUST deny — a stale cached `true` here would be a hole. + if allowed(&engine, s.recipient, s.files[0]).await { + eprintln!( + "SAFETY GATE FAILED: recipient still allowed after clear_role — \ + cascade cache was not invalidated on grant revoke" + ); + cleanup(&pool, &s).await; + std::process::exit(1); + } + // Re-grant for the perf run below. + engine + .set_role( + s.owner, + Subject::User(s.recipient), + Role::Viewer, + Resource::Folder(s.album_folder), + None, + ) + .await + .expect("re-grant"); + } + + println!("\n#################################################################"); + println!("# shared-album thumbnail authz: folder-cascade query/thumb vs cache"); + println!("# thumbs={thumbs} (recipient holds a folder grant, no drive membership)"); + println!("#################################################################\n"); + println!("| {:<28} | {:>10} | {:>12} |", "arm", "wall ms", "µs/thumb"); + + // BEFORE: no cache — a fresh engine per thumbnail forces the cascade query + // every time (models the pre-round-8 per-request behaviour). + { + let t = Instant::now(); + for &f in &s.files { + let engine = fresh_engine(&pool); + std::hint::black_box(allowed(&engine, s.recipient, f).await); + } + let el = t.elapsed(); + println!( + "| {:<28} | {:>10.2} | {:>12.2} |", + "BEFORE (query/thumb)", + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / thumbs as f64 + ); + } + + // AFTER cold: one persistent engine — the first grid view queries once per + // distinct file (cache misses populate). + let engine = fresh_engine(&pool); + { + let t = Instant::now(); + for &f in &s.files { + std::hint::black_box(allowed(&engine, s.recipient, f).await); + } + let el = t.elapsed(); + println!( + "| {:<28} | {:>10.2} | {:>12.2} |", + "AFTER cold (first view)", + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / thumbs as f64 + ); + } + + // AFTER warm: revalidation re-checks the same files — all cache hits, the + // "navigate away and back" / constant If-None-Match revalidation case. + { + let t = Instant::now(); + for &f in &s.files { + std::hint::black_box(allowed(&engine, s.recipient, f).await); + } + let el = t.elapsed(); + println!( + "| {:<28} | {:>10.2} | {:>12.2} |", + "AFTER warm (revalidation)", + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / thumbs as f64 + ); + } + + cleanup(&pool, &s).await; + println!("\n(The check is never skipped — authz still runs on every thumbnail; only"); + println!(" the folder-cascade DECISION is memoised. BEFORE re-queries per request;"); + println!(" AFTER warm serves revalidations from memory. Safety gates verified:"); + println!(" recipient allowed, outsider denied, and a clear_role revoke denies"); + println!(" immediately — the grant write flushed the cache.)"); +} diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index 64973a5c..ecc679ea 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -103,6 +103,19 @@ const DRIVE_POLICIES_CACHE_CAPACITY: u64 = 100_000; /// effective within a minute on the hot path. const DRIVE_POLICIES_CACHE_TTL: Duration = Duration::from_secs(30); +/// `cascade_grant_cache` bound: entries are +/// `((Subject, Resource, Permission), bool)` — a few tens of bytes each. A +/// shared photo album is one folder grant serving hundreds of file checks, so +/// 100k comfortably covers the working set of active shared-resource viewers. +const CASCADE_GRANT_CACHE_CAPACITY: u64 = 100_000; +/// `cascade_grant_cache` TTL. Direct grant mutations on the file/folder +/// (`set_role` / `clear_role`) explicitly invalidate the whole cache, so the +/// TTL is the self-heal net for the *indirect* paths — a group-membership +/// change, a resource move, or a grant's `expires_at` passing — exactly as +/// `drive_role_cache` leans on its TTL for group changes "rather than a deep +/// invalidation tree". Short enough that any such change takes effect in <1 min. +const CASCADE_GRANT_CACHE_TTL: Duration = Duration::from_secs(30); + pub struct PgAclEngine { pool: Arc, folder_repo: Arc, @@ -160,6 +173,35 @@ pub struct PgAclEngine { /// returns, so the next check sees the fresh values. Short 30 s TTL /// as the self-heal net for direct-SQL edits and migration backfills. drive_policies_cache: Cache, + + /// Memoise the File/Folder **grant-cascade** decision + /// `(subject, resource, permission) → bool` — the result of the + /// `role_grants` + folder-ancestor (`lpath @>`) cascade that + /// `check_inner` falls through to when the drive-role precheck doesn't + /// cover the caller. This is the per-request query a shared-album + /// recipient (a grant on the containing folder, no drive membership) pays + /// for **every thumbnail** — and browsers revalidate immutable thumbnails + /// constantly, so the same `(subject, file, Read)` decision is recomputed + /// again and again. Cached here it costs one query then in-memory hits. + /// + /// Only reached AFTER the drive-role precheck fails, so a caller who is a + /// drive member short-circuits above and never populates a (possibly + /// negative) entry here — a later drive grant can't be shadowed by a stale + /// cascade `false`. + /// + /// **Invalidation**: explicit `invalidate_all` on every File/Folder + /// `set_role` / `clear_role` (the direct share/revoke path — infrequent + /// relative to thumbnail reads, so a full flush is cheap and keeps + /// revocation immediate). The indirect paths — group-membership changes, + /// resource moves that change ancestry, grant `expires_at` expiry — are + /// caught by the 30 s TTL, matching `drive_role_cache`'s documented + /// convention. + /// + /// **Safety**: the check still runs on every request (the ordering is + /// unchanged — authz is never skipped); only its *result* is memoised, and + /// only positively-or-negatively for at most the TTL. A revoke via + /// `clear_role` flushes immediately; anything missed self-heals in ≤30 s. + cascade_grant_cache: Cache<(Subject, Resource, Permission), bool>, } impl PgAclEngine { @@ -197,6 +239,10 @@ impl PgAclEngine { .max_capacity(DRIVE_POLICIES_CACHE_CAPACITY) .time_to_live(DRIVE_POLICIES_CACHE_TTL) .build(), + cascade_grant_cache: Cache::builder() + .max_capacity(CASCADE_GRANT_CACHE_CAPACITY) + .time_to_live(CASCADE_GRANT_CACHE_TTL) + .build(), } } @@ -267,6 +313,10 @@ impl PgAclEngine { .max_capacity(1) .time_to_live(Duration::from_secs(1)) .build(), + cascade_grant_cache: Cache::builder() + .max_capacity(1) + .time_to_live(Duration::from_secs(1)) + .build(), } } @@ -358,6 +408,20 @@ impl PgAclEngine { self.owner_cache.invalidate_all(); } + /// Flush the entire `cascade_grant_cache`. Called on every File/Folder + /// `set_role` / `clear_role` — the direct share/revoke path. A resource + /// grant can widen (or, via ancestry, narrow) the cascade decision for an + /// unbounded set of descendant files, and the cache is keyed by the + /// decision — not the grant — so we can't target the affected entries + /// without walking the subtree. A full flush is correct and cheap here: + /// grant mutations are rare next to the thumbnail reads the cache serves, + /// and it keeps a revoke immediate. Indirect changes (group membership, + /// resource moves, grant expiry) are left to the 30 s TTL, mirroring + /// `drive_role_cache`. + pub async fn invalidate_cascade_grant_cache_all(&self) { + self.cascade_grant_cache.invalidate_all(); + } + /// Sibling of [`Self::invalidate_drive_role_cache_for_drive`] keyed by /// subject rather than drive. Used by the user-deleted lifecycle hook /// to reap every cached "user X → drive Y = role R" entry after the @@ -709,6 +773,63 @@ impl PgAclEngine { Ok(exists.is_some()) } + /// Cache-aware wrapper over the File/Folder grant cascade. Serves the + /// memoised `(subject, resource, permission)` decision when warm; on a + /// miss it expands the subject set (itself cached) and runs the matching + /// cascade query, then stores the result. Only invoked after the drive-role + /// precheck fails, so it never caches a decision a drive grant would have + /// satisfied — a later drive grant short-circuits above this cache. + /// + /// The result is a pure function of the subject's group expansion + the + /// resource's grants + folder ancestry; `invalidate_cascade_grant_cache_all` + /// (on File/Folder grant writes) and the 30 s TTL (indirect changes) keep + /// it fresh. See the `cascade_grant_cache` field doc. + async fn cascade_grant_cached( + &self, + subject: Subject, + resource: Resource, + permission: Permission, + counters: &QueryCounters, + ) -> Result { + if let Some(allowed) = self + .cascade_grant_cache + .get(&(subject, resource, permission)) + .await + { + counters.cache_hit.fetch_add(1, Ordering::Relaxed); + return Ok(allowed); + } + let (subject_types, subject_ids) = self.subject_match_set(subject, counters).await?; + let allowed = match resource { + Resource::Folder(id) => { + self.folder_cascade_grant_exists( + &subject_types, + &subject_ids, + permission, + id, + counters, + ) + .await? + } + Resource::File(id) => { + self.file_cascade_grant_exists( + &subject_types, + &subject_ids, + permission, + id, + counters, + ) + .await? + } + // Only File/Folder reach this helper (see `check_inner`). + _ => return Ok(false), + }; + self.cascade_grant_cache + .insert((subject, resource, permission), allowed) + .await; + Ok(allowed) + } + /// Cached resolution of `(subject, drive_id) → Option` — the /// strongest role the subject holds on the drive (direct + transitive /// group grants collapsed). `None` means no qualifying grant; cached @@ -972,32 +1093,15 @@ impl PgAclEngine { } match resource { - // File/Folder dispatch falls through to the cascade query — - // expand the subject set lazily here (it's cached) so the - // Drive branch below never pays for an expansion it doesn't need. - Resource::Folder(id) => { - let (subject_types, subject_ids) = - self.subject_match_set(subject, counters).await?; - self.folder_cascade_grant_exists( - &subject_types, - &subject_ids, - permission, - id, - counters, - ) - .await - } - Resource::File(id) => { - let (subject_types, subject_ids) = - self.subject_match_set(subject, counters).await?; - self.file_cascade_grant_exists( - &subject_types, - &subject_ids, - permission, - id, - counters, - ) - .await + // File/Folder dispatch falls through to the cascade query, now + // memoised: a shared-album recipient (folder grant, no drive + // membership) reaches this per thumbnail, and browsers revalidate + // thumbnails constantly, so the same decision is recomputed over + // and over. `cascade_grant_cached` serves it from memory after the + // first query; the check is unchanged (never skipped), only cached. + Resource::Folder(_) | Resource::File(_) => { + self.cascade_grant_cached(subject, resource, permission, counters) + .await } Resource::Drive(id) => { // Same read_only gate as the File/Folder branch: a frozen @@ -2413,6 +2517,12 @@ impl AuthorizationEngine for PgAclEngine { if let Resource::Drive(drive_id) = resource { self.invalidate_drive_role_cache_for_drive(drive_id).await; } + // File/Folder grant write — a new share can widen the cascade + // decision for descendant files; flush the cascade cache so the next + // thumbnail/read check sees it immediately. + if matches!(resource, Resource::File(_) | Resource::Folder(_)) { + self.invalidate_cascade_grant_cache_all().await; + } Self::row_to_grant(row) } @@ -2437,6 +2547,11 @@ impl AuthorizationEngine for PgAclEngine { if let Resource::Drive(drive_id) = resource { self.invalidate_drive_role_cache_for_drive(drive_id).await; } + // Revoking a File/Folder share must stop passing the cascade check + // now, not in ≤30 s — flush the cascade cache (see `set_role`). + if matches!(resource, Resource::File(_) | Resource::Folder(_)) { + self.invalidate_cascade_grant_cache_all().await; + } Ok(()) }