Files
DarkRoom/CLAUDE.md
T
dtourolle ce72fe49a0 Record the catalog lessons of the 2026-09-25 performance pass
Three habits the pass found broken, in the style of the 2026-09-19 notes:
SQL text passed to `execute`/`query_row` in a loop is a prepare per row
(the merge, `persist` and the shard sync all paid it), a case-insensitive
`LIKE` cannot use the `source_ref` key where a range can, and the backfill
inside `Catalog::open` is paid by every worker thread, the develop view's
fetches included. And where the two new benches are and how to run them.
2026-09-25 22:06:58 -04:00

9.1 KiB

Working in this repository

Notes for anyone — person or agent — changing this code. They record what went wrong once and what the fix looked like, so the same shape is not written again. Requirements live in docs/dev/requirements.md; this file is about habits, not features.

Catalog reads: work is proportional to what changed, never to library size

docs/dev/catalog.md §1 states the rule. These are the ways it was broken on the Identity screen, found when every confirm click cost half a second on a 24k-image library (2026-09-19), and what each fix looked like.

A redraw must know what changed. A click handler that calls "refresh everything" pays for everything. identity_ui::refresh takes a Changed: a confirm re-reads the rail and the grid and not the coverage line, because moving a face between people cannot alter how many images are indexed. Before adding a read to a shared refresh, ask which events can change its answer, and gate it on those.

Count with COUNT(*), never with .len() on a list you then drop. repairs::counts used to build every repair's work list — a Target with its path per row, sorted into visiting order — to report its length. Six repairs, 350 ms, nothing kept. If the caller wants a number, the query returns a number.

One query, not one per row. ThumbStore::contains in a filter over 5,000 rows is 5,000 prepared statements; ThumbStore::held(size) reads the index once into a set. The same applies to any query_row inside a loop over a result set — including deep_count per sidebar row, which is fine at sidebar scale and would not be at grid scale. Aggregate in one statement and look up in memory.

SQL text in a loop is a prepare in a loop. rusqlite's execute and query_row compile their statement on every call. A loop that calls them per row pays a prepare per row even when each query is a primary-key seek: the merge of a synced catalog prepared four statements for each of 13,000 incoming faces (450 ms of a pass that changed nothing), persist four per photograph a scan listed (1.5 s for a first scan), the shard sync one per image each way. Hoist the statement, use prepare_cached, or — better, when the loop asks the same table about every row — read that table once into a map. And do not rewrite a row with what it already holds: an upsert of identical values still dirties the page.

A LIKE is case-insensitive, and no index here serves that. source_ref LIKE 'stem.%' read every name of the root per sidecar a pull took in. When the check that decides is exact, spell the prefix as a range (>= 'stem.' AND < 'stem/', / being the byte after .), which the (root_id, source_ref) key answers with a seek.

Catalog::open is not free, and every worker thread calls it. The backfill runs on every open, and the develop view opens a catalog to fetch each original and again for each neighbour it prefetches. Keep each backfill step's no-op case to a read of the small side — the unpaired JPEGs, not every RAW; the distinct keywords, not every assignment — and measure an open with catalog_bench after adding one.

Filter and aggregate in SQL, and aggregate the small side first. faces::people read 19,000 rows, grouped, sorted them by name, and the screen threw 17,000 away (empty unnamed groups). people_in_use filters in the WHERE, and joins people to a pre-aggregated face_person (2,000 groups) rather than grouping after a LEFT JOIN over every person. The sort then sees only the rows that will be drawn.

Wide rows make "just check one column" a table scan. A faces row is ~8 KB (a 1 KB embedding and a ~5 KB crop, then the columns added later). Any predicate that reads quality, crop or an eye column for every face reads every row. V17 learned this for the eye filter; V19 applies it to the repair counts with partial indexes (faces_owed_*) that hold only the rows still owing, keyed on what the predicate joins on and carrying model_id because the predicate reads it. Two things to know about them:

  • Drive the count from the small side. SQLite uses a partial index when the query starts from faces (repairs::count, Needs::Face) and ignores it inside a correlated EXISTS (... WHERE f.image_id = i.id ...). That is why Needs::Face carries the per-face fragment and spells it two ways.
  • Spell the predicate as the index's WHERE is spelled. NEEDS_EYES is (f.eye_right IS NULL OR f.landmarks_dense IS NULL) because faces_owed_eyes is WHERE eye_right IS NULL OR landmarks_dense IS NULL. Change one, change both, and counts_are_the_sizes_of_the_lists will tell you if they drift.

Check a query's plan with EXPLAIN QUERY PLAN against a copy of a real catalog before trusting an index exists for it: "SEARCH ... USING COVERING INDEX" is the answer you want, "SEARCH f USING INDEX faces_image" on a wide table means every probe opens a row.

Catalog writes: one transaction per user action

faces::confirm opens a transaction. Calling it in a loop over a group is a commit per face; faces::confirm_all is two statements and one commit, faces::reassign one transaction for a whole split. When a UI action touches N rows, give the catalog a function that takes the N, not a loop that calls the one-row function N times — unchecked_transaction cannot nest, so this has to be designed in at the catalog layer, not wrapped from above.

Screens: keep what is already decoded

identity::load_faces takes the crops the grid is currently showing and hands them back into the new cells. Before that, a click re-read 4 MB of crop blobs and decoded 700 JPEGs to produce the pixels already on screen. When a redraw replaces a model, the expensive parts of the old model — a decoded image, a cut portrait — are the first thing to reuse; only the row that changed needs new work. Drain the old cells rather than cloning them.

Remote calls: one round trip per file, not one per ancestor

NextcloudBackend::move_to guaranteed its destination's parent with a MKCOL per ancestor from the account root, on every file of a batch — three 405s before each MOVE. The backend now remembers the collections it has confirmed (known_dirs) for its lifetime, which is one job. When a per-file operation has a per-batch precondition, satisfy it once.

Providers: read the runtime's source for the version on disk, not the binding

Two things the MIGraphX rung (2026-09-20) got wrong before it was measured right, both because ort's builder was trusted to mean what its method names say.

A binding's option builder may fill a struct the runtime no longer reads. ep::MIGraphX::with_save_model sets fields of the legacy OrtMIGraphXProviderOptions; ONNX Runtime 1.29 reads that struct for the precision flags and ignores the rest, so every session compiled for 40 s and the cache directory went nowhere. The option that works (migraphx_model_cache_dir) exists only in the generic key/value registration, which session::migraphx calls on the API table directly. Before wiring a provider option, fetch the provider's source at the runtime's exact version and find where the option is read.

A provider's cache key may leave out what you are varying. MIGraphX keys a compiled program on graph, GPU and its own version — not precision. The first fp16 measurement built in 0.3 s and matched f32 to the tenth of a millisecond, because it had loaded the f32 program. A "from cache" build that is suspiciously fast on the first run of a new configuration is a key collision, not a fast provider; give each precision its own directory (the engine does) and check the cache directory gained a file.

Measuring

cargo run --release -p dr-ui --example identity_bench -- CATALOG THUMBS times what one click on the Identity screen reads and what the batch operations write. Run it against a copy of a real catalog (it writes), never the library's own file; sqlite3 catalog.sqlite ".backup copy.sqlite" takes a consistent one while the app runs. Compare the cpu column when other builds are running on the machine — the wall clock doubles under load, the CPU figure does not. Keep the binary from before the change and run both back to back rather than trusting numbers taken an hour apart.

cargo run --release -p dr-catalog --example catalog_bench -- CATALOG [FACES_DIR] does the same for opening the catalog (the backfill step by step), the upload snapshot, a merge, and the face shard export and import; persist_bench, an ignored test in dr-ui's scan module, replays a scan's persist and a sidecar pull (DR_BENCH_CATALOG=copy.sqlite cargo test --release -p dr-ui --lib persist_bench -- --ignored --nocapture --test-threads=1). Both take copies; hand catalog_bench a copy of the face store directory too.

Reference figures from the 2026-09-19 fixes, largest person (754 faces), 24k images, 19k faces, before → after. What one click read: load_people 22 ms → 12 ms, load_faces 316 ms → 2.4 ms, audit 190 ms → not run (66 ms when it is, on open and at the end of a sweep). What one click wrote: confirm_all 16 ms → 2 ms, split_off 23 ms → 4.5 ms. A click on the face grid went from ~530 ms of catalog work to ~15 ms.