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.
166 lines
9.1 KiB
Markdown
166 lines
9.1 KiB
Markdown
# 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 `405`s 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.
|