dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Do not recount face coverage on every confirm, and count it without listing

Browse Source 
Every click on the Identity screen's face grid — confirm, reject, split,
rename, merge — redrew the whole screen, and the redraw recomputed the
coverage line. That line lists every repair's outstanding images to count
them: six scans of the images table with a correlated EXISTS over the
8 KB face rows, an ORDER BY the job's visiting order, a Target with its
path per row, and a thumbnail-index query per image with faces. On the
reference library (24k images, 19k faces) that was ~200 ms of the
~540 ms each click cost, spent computing a figure a confirm cannot change.

`refresh` now takes what changed: `Changed::Identities` re-reads the rail
and the grid and leaves the coverage line alone; `Changed::Library` — an
open, a sweep ending or stopped, the face data deleted — re-reads it too.

For the times it does run, `repairs::counts` counts instead of building
and dropping the lists, and the thumbnail store is read once
(`ThumbStore::held`) rather than probed once per image in the audit, the
outstanding list and the proxy repair.

`identity_bench` is the measurement: the reads a click performs and the
batch writes, timed against a copy of a real catalog.
This commit is contained in:
dtourolle
2026-09-20 10:56:36 +02:00
parent d790961b28
commit e0e193efb4
6 changed files with 291 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/faces.rs
+17 -2
View File
@@ -116,6 +116,12 @@ pub fn faces_outstanding(
ORDER BY i.id",
faces::embedder_sql("fi.model_id")
))?;
// The store's index in one read rather than a probe per image; see
// `audit`, which splits the same list the same way.
let held = store.held(FACE_TIER).unwrap_or_else(|e| {
log::warn!("faces: reading the thumbnail index: {e}");
Default::default()
});
let rows = stmt
.query_map([faces::embedder_of(model_id)], |r| {
Ok(FaceRequest {
@@ -134,7 +140,7 @@ pub fn faces_outstanding(
// a whole-library button that could only reach photographs the user had
// personally zoomed into. `repairs::spawn` is that
// requirement implemented; this one is the local-only variant.
.filter(|req| store.contains(req.file_id, FACE_TIER))
.filter(|req| held.contains(&req.file_id))
.collect();
Ok(rows)
}
@@ -270,12 +276,21 @@ pub fn audit(
)",
faces::embedder_sql("fi.model_id")
))?;
// One read of the store's index, not one probe per outstanding image:
// `contains` answers the same question, and asked four thousand times
// it cost more than every query above put together. A store whose index
// cannot be read is treated as holding nothing, which is what `contains`
// reports for it too.
let held = store.held(FACE_TIER).unwrap_or_else(|e| {
log::warn!("faces: reading the thumbnail index: {e}");
Default::default()
});
let (mut ready, mut awaiting) = (0u64, 0u64);
for file_id in stmt
.query_map([faces::embedder_of(model_id)], |r| r.get::<_, i64>(0))?
.filter_map(Result::ok)
{
if store.contains(file_id as u64, FACE_TIER) {
if held.contains(&(file_id as u64)) {
ready += 1;
} else {
awaiting += 1;
ui/dr-ui/src/identity_ui.rs
+42 -14
View File
@@ -138,6 +138,26 @@ impl IdentityController {
}
}
/// What a reload has to re-read, named by what just changed.
///
/// The coverage line is the expensive half of a redraw: it lists every
/// repair's outstanding images to count them, which is several scans of the
/// whole `images` table (`crate::repairs::counts`). A confirm, a reject, a
/// rename or a merge moves faces between people and cannot change how many
/// images have been indexed, so a redraw for one of those must not pay for
/// it — that was 200 ms of the half-second every click on the face grid used
/// to cost on the reference library.
#[derive(Clone, Copy, PartialEq, Eq)]
pub enum Changed {
/// Who the faces belong to. The rail and the grid are re-read; the
/// coverage line is left as it was.
Identities,
/// Which faces exist: a sweep finished or was stopped, the face data was
/// deleted, the screen was opened onto a catalog another device may have
/// indexed. Everything is re-read, the coverage line included.
Library,
}
/// Push the people rail and the face grid into the window.
///
/// **Draws with the portraits it already has and cuts the rest afterwards.**
@@ -154,6 +174,7 @@ pub fn refresh(
store: Option<Rc<ThumbStore>>,
model_id: &str,
eyes: bool,
changed: Changed,
) {
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else {
@@ -274,7 +295,9 @@ pub fn refresh(
window.set_identity_picked(ctl.picked.borrow().len() as i32);
drop(borrow);
refresh_coverage(window, catalog, store.as_deref(), model_id, eyes);
if changed == Changed::Library {
refresh_coverage(window, catalog, store.as_deref(), model_id, eyes);
}
// Last, so a portrait cannot delay anything above it.
if let Some(store) = store {
@@ -628,8 +651,10 @@ pub fn wire<S, M, P>(
// The availability closure is an argument rather than named in the
// body: macro hygiene would bind the name to this scope's `Rc`, which the
// first `move` closure would then take with it.
// The last argument is a `Changed` variant, named bare so the call
// stays on one line.
macro_rules! reload {
($w:expr, $ctl:expr, $catalog:expr, $store:expr, $settings:expr, $eyes:expr) => {
($w:expr, $ctl:expr, $catalog:expr, $store:expr, $settings:expr, $eyes:expr, $changed:ident) => {
refresh(
&$w,
&$ctl,
@@ -637,6 +662,7 @@ pub fn wire<S, M, P>(
$store(),
&model_id(&$settings),
$eyes(),
Changed::$changed,
)
};
}
@@ -662,7 +688,7 @@ pub fn wire<S, M, P>(
// A fact about the filesystem, so it is re-checked on every open
// rather than cached: the user may have just put the models there.
w.set_identity_model_missing(models_present().is_none());
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Library);
});
}
@@ -702,7 +728,7 @@ pub fn wire<S, M, P>(
// The offer was about the person being navigated away from. Left
// up, its "Merge" would fold whoever is selected *now*.
clear_merge_offer(&w, &ctl);
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
reset_name_field(&w);
});
}
@@ -748,7 +774,7 @@ pub fn wire<S, M, P>(
Err(e) => log::warn!("identity: looking for a namesake: {e}"),
}
}
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
// `rename` trims; the field should show what was actually stored
// rather than the spacing the user happened to type.
reset_name_field(&w);
@@ -786,7 +812,7 @@ pub fn wire<S, M, P>(
}
}
clear_merge_offer(&w, &ctl);
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
reset_name_field(&w);
});
}
@@ -832,7 +858,7 @@ pub fn wire<S, M, P>(
log::warn!("identity: confirm: {e}");
}
}
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
@@ -853,7 +879,7 @@ pub fn wire<S, M, P>(
log::warn!("identity: reject: {e}");
}
}
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
@@ -897,7 +923,7 @@ pub fn wire<S, M, P>(
Err(e) => log::warn!("identity: confirm all: {e}"),
}
}
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
@@ -931,7 +957,7 @@ pub fn wire<S, M, P>(
}
}
ctl.clear_picks();
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
@@ -1091,6 +1117,7 @@ pub fn wire<S, M, P>(
store_tick(),
&model_id(&settings_tick),
eyes_tick(),
Changed::Identities,
);
}
},
@@ -1268,6 +1295,7 @@ pub fn wire<S, M, P>(
store_tick(),
&model_id(&settings_tick),
eyes_tick(),
Changed::Library,
);
}
},
@@ -1306,7 +1334,7 @@ pub fn wire<S, M, P>(
a.finish("stopped");
}
w.set_identity_indexing(false);
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Library);
});
}
@@ -1351,7 +1379,7 @@ pub fn wire<S, M, P>(
ctl.selected.set(None);
ctl.clear_picks();
}
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
@@ -1365,7 +1393,7 @@ pub fn wire<S, M, P>(
window.on_identity_toggle_show_ignored(move || {
let Some(w) = weak.upgrade() else { return };
ctl.show_ignored.set(!ctl.show_ignored.get());
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Identities);
});
}
@@ -1387,7 +1415,7 @@ pub fn wire<S, M, P>(
ctl.selected.set(None);
ctl.clear_picks();
ctl.covers.borrow_mut().clear();
reload!(w, ctl, catalog, store, settings, eyes_available);
reload!(w, ctl, catalog, store, settings, eyes_available, Library);
});
}
}
ui/dr-ui/src/repairs.rs
+59 -3
View File
@@ -732,12 +732,19 @@ fn faces_without_proxy(
WHERE r.file_id IS NOT NULL AND {VISIBLE} AND {} = ?1",
faces::embedder_sql("f.model_id"),
))?;
// The index once, not a probe per image with faces — see
// `ThumbStore::held`. An unreadable index reads as empty, as `contains`
// would have reported it.
let held = store.held(dr_thumbs::ThumbSize::Large).unwrap_or_else(|e| {
log::warn!("repairs: reading the thumbnail index: {e}");
Default::default()
});
let rows = stmt
.query_map([embedder], |r| {
Ok((r.get::<_, i64>(0)?, r.get::<_, i64>(1)?))
})?
.filter_map(Result::ok)
.filter(|(_, file_id)| !store.contains(*file_id as u64, dr_thumbs::ThumbSize::Large))
.filter(|(_, file_id)| !held.contains(&(*file_id as u64)))
.map(|(id, _)| id)
.collect();
Ok(rows)
@@ -802,7 +809,14 @@ fn still_owed(
}
}
/// How many images each repair still lists, for the settings line.
/// How many images each repair still lists, for the settings line and the
/// coverage line.
///
/// Counted, not listed. [`listed`] builds a `Target` per image — its path,
/// its size — and sorts the faces-first order the job visits them in, none of
/// which a count reads; asked for six repairs on a 24,000-image library that
/// was 350 ms of `source_ref` strings built to be dropped. A `COUNT(*)` over
/// the same predicate is the same number in a tenth of the time.
pub fn counts(
catalog: &Catalog,
store: &ThumbStore,
@@ -810,10 +824,52 @@ pub fn counts(
) -> Result<Vec<(&'static str, u64)>, dr_catalog::CatalogError> {
repairs
.iter()
.map(|r| Ok((r.label, listed(catalog, store, r)?.len() as u64)))
.map(|r| Ok((r.label, count(catalog, store, r)?)))
.collect()
}
/// How many images one repair lists — the size of [`listed`]'s answer,
/// without building it.
fn count(
catalog: &Catalog,
store: &ThumbStore,
repair: &Repair,
) -> Result<u64, dr_catalog::CatalogError> {
match &repair.needs {
Needs::Sql(sql) => {
let n: i64 = catalog.connection().query_row(
&format!(
"SELECT COUNT(*)
FROM images i
JOIN remote r ON r.image_id = i.id
WHERE r.file_id IS NOT NULL AND {VISIBLE} AND ({sql})"
),
[],
|r| r.get(0),
)?;
Ok(n as u64)
}
// The set is built from its own query and may name images `listed`
// would not visit, so it is intersected with the same base rather
// than trusted for its size.
Needs::Set(f) => {
let set = f(catalog, store)?;
let mut stmt = catalog.connection().prepare(&format!(
"SELECT i.id
FROM images i
JOIN remote r ON r.image_id = i.id
WHERE r.file_id IS NOT NULL AND {VISIBLE}"
))?;
let n = stmt
.query_map([], |r| r.get::<_, i64>(0))?
.filter_map(Result::ok)
.filter(|id| set.contains(id))
.count();
Ok(n as u64)
}
}
}
/// One image on the work list, with the most any repair claiming it asks
/// for.
struct Planned {