Merge a synced catalog's face assignments without re-preparing per face

A sync pass that brought nothing new cost 450-540 ms of CPU in
`merge_remote_catalog` on the reference library (24k images, 19k faces),
measured by catalog_bench merging a copy of the catalog with itself.

Most of it was the loop over the other device's confirmed faces and the
faces under its ignored groups -- 13,000 rows. For each one it prepared
three statements from scratch (`query_row`/`execute` with a SQL string
compile the statement every call) and then rewrote the `face_person` row
with the values it already held, dirtying a page per face on every pass.
The rejection loop prepared three more per row.

The statements are now `prepare_cached`, the local assignment is read once
per face (whether it is confirmed, and what it holds, come from the same
row), and the upsert is skipped when the row already says exactly that.
`faces_assigned` is still counted for those rows, so the report is the one
the old code gave, and nothing else reads the difference: the row is
byte-for-byte what the upsert would have written.

After: 279 ms (best of 5, CPU), with every catalog table identical after
the run to the old build's.
This commit is contained in:
2026-09-25 22:06:58 -04:00
parent 454375243c
commit 9cff677392
+51 -42
View File
@@ -931,16 +931,36 @@ fn merge_people_within(tx: &Connection, report: &mut MergeReport) -> Result<(),
.query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?)))?
.collect::<Result<_, _>>()?;
// Cached, not prepared per row: a steady-state pass walks every
// confirmed and every ignored face the other device holds -- 13,000 on
// the reference library -- and preparing four statements for each was
// most of the 780 ms a merge that changed nothing cost.
let mut person_of = tx.prepare_cached("SELECT id FROM people WHERE uuid = ?1")?;
let mut current = tx.prepare_cached(
"SELECT person_id, probability, confirmed FROM face_person WHERE face_id = ?1",
)?;
let mut rejected_q = tx.prepare_cached(
"SELECT EXISTS(SELECT 1 FROM face_person_rejected
WHERE face_id = ?1 AND person_id = ?2)",
)?;
let mut assign = tx.prepare_cached(
"INSERT INTO face_person (face_id, person_id, probability, confirmed)
VALUES (?1, ?2, ?3, ?4)
ON CONFLICT(face_id) DO UPDATE SET
person_id = excluded.person_id,
probability = excluded.probability,
confirmed = excluded.confirmed",
)?;
for (remote_face, uuid, probability, confirmed) in incoming {
let Some(&local_face) = face_map.get(&remote_face) else {
continue;
};
let person: Option<i64> = tx
.query_row("SELECT id FROM people WHERE uuid = ?1", [&uuid], |r| {
r.get(0)
})
.optional()?;
let person: Option<i64> = person_of.query_row([&uuid], |r| r.get(0)).optional()?;
let Some(person) = person else { continue };
let held: Option<(i64, f64, i64)> = current
.query_row([local_face], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)))
.optional()?;
// A local confirmation is never overwritten, in either direction.
// Two devices confirming the same face as different people is a
@@ -948,13 +968,7 @@ fn merge_people_within(tx: &Connection, report: &mut MergeReport) -> Result<(),
// settle it with; silently taking the remote's answer would let a
// sync undo something the user did here. It stays as it is, and the
// user can change it on the device they are looking at.
let locally_confirmed: bool = tx.query_row(
"SELECT EXISTS(SELECT 1 FROM face_person
WHERE face_id = ?1 AND confirmed = 1)",
[local_face],
|r| r.get(0),
)?;
if locally_confirmed {
if held.is_some_and(|(_, _, confirmed)| confirmed == 1) {
report.faces_kept_local += 1;
continue;
}
@@ -963,25 +977,22 @@ fn merge_people_within(tx: &Connection, report: &mut MergeReport) -> Result<(),
// this user's judgement about this pair, and re-suggesting what
// they pushed away is the behaviour that makes the feature feel
// broken.
let rejected: bool = tx.query_row(
"SELECT EXISTS(SELECT 1 FROM face_person_rejected
WHERE face_id = ?1 AND person_id = ?2)",
[local_face, person],
|r| r.get(0),
)?;
let rejected: bool = rejected_q.query_row([local_face, person], |r| r.get(0))?;
if rejected {
continue;
}
tx.execute(
"INSERT INTO face_person (face_id, person_id, probability, confirmed)
VALUES (?1, ?2, ?3, ?4)
ON CONFLICT(face_id) DO UPDATE SET
person_id = excluded.person_id,
probability = excluded.probability,
confirmed = excluded.confirmed",
rusqlite::params![local_face, person, probability, confirmed],
)?;
// Written only when it differs. Rewriting a row with the values it
// already holds dirtied a page per face, every pass, for nothing;
// the report still counts it, as it always has.
if held != Some((person, probability, i64::from(confirmed))) {
assign.execute(rusqlite::params![
local_face,
person,
probability,
confirmed
])?;
}
report.faces_assigned += 1;
}
}
@@ -997,32 +1008,30 @@ fn merge_people_within(tx: &Connection, report: &mut MergeReport) -> Result<(),
.query_map([], |r| Ok((r.get(0)?, r.get(1)?)))?
.collect::<Result<_, _>>()?;
let mut person_of = tx.prepare_cached("SELECT id FROM people WHERE uuid = ?1")?;
let mut reject = tx.prepare_cached(
"INSERT OR IGNORE INTO face_person_rejected (face_id, person_id)
VALUES (?1, ?2)",
)?;
let mut unsuggest = tx.prepare_cached(
"DELETE FROM face_person
WHERE face_id = ?1 AND person_id = ?2 AND confirmed = 0",
)?;
for (remote_face, uuid) in incoming {
let Some(&local_face) = face_map.get(&remote_face) else {
continue;
};
let person: Option<i64> = tx
.query_row("SELECT id FROM people WHERE uuid = ?1", [&uuid], |r| {
r.get(0)
})
.optional()?;
let person: Option<i64> = person_of.query_row([&uuid], |r| r.get(0)).optional()?;
let Some(person) = person else { continue };
let n = tx.execute(
"INSERT OR IGNORE INTO face_person_rejected (face_id, person_id)
VALUES (?1, ?2)",
[local_face, person],
)?;
let n = reject.execute([local_face, person])?;
report.faces_rejected += n;
// A rejection that lands on a face currently *suggested* to be
// that person has to take the suggestion with it, or the screen
// keeps offering exactly what the other device just refused.
tx.execute(
"DELETE FROM face_person
WHERE face_id = ?1 AND person_id = ?2 AND confirmed = 0",
[local_face, person],
)?;
unsuggest.execute([local_face, person])?;
}
}