Benchmarks / CPU and I/O (per commit) (push) Failing after 6m20s
Benchmarks / Frame budget (on demand) (push) Skipped
Build and test / Desktop (Linux) (push) Failing after 45s
Build and test / Layer separation (push) Successful in 26s
Traceability / Requirement traces (push) Failing after 46s
🐳 Android image / Build and push (push) Successful in 1s
Build and test / android-image (push) Successful in 1s
🐳 Windows image / Build and push (push) Successful in 1s
Build and test / windows-image (push) Successful in 1s
Build and test / Android (aarch64) (push) Failing after 2m19s
Build and test / Windows (x86_64, cross) (push) Failing after 3m2s
Measured on a Radeon RX 7900 XT against Arch's onnxruntime-rocm 1.29 (docs/inference.md §1.3): MIGraphX fp16 runs the detectors at 2.4–3.4 ms against 10–58 ms on the CPU provider, the inpainter at 8 ms against 514, with a 15–135 s compile per graph the first time and under a second from its cache after. A compiling rung on TensorRT's terms, wired the same way. The ROCm execution provider is gone (removed in ONNX Runtime 1.23), so the AMD ladder is MIGraphX then the CPU, with no non-compiling rung between. MIGraphX is registered through the runtime's generic key/value entry point rather than ort's builder: 1.29 reads the legacy options struct for its precision flags only, and the compiled-program cache directory (`migraphx_model_cache_dir`) only travels the generic way. The provider's cache key omits the precision, so f32 and fp16 programs get their own directories. The probe fingerprint now includes the provider libraries beside the runtime and the ROCm version, since a distribution's CPU and ROCm builds are the same file at the same path. `status().failed` reports only the rungs above the selection, so an AMD desktop's About line says why MIGraphX won rather than that the NVIDIA providers are not in the build. Two examples: `ep_probe` times each provider cold and from cache, and `ladder` drives `init` as the app does to watch the first-run sequence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
133 lines
7.1 KiB
Markdown
133 lines
7.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/requirements.md`; this file is
|
|
about habits, not features.
|
|
|
|
## Catalog reads: work is proportional to what changed, never to library size
|
|
|
|
`docs/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.
|
|
|
|
**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.
|
|
|
|
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.
|