diff --git a/core/dr-catalog/src/merge.rs b/core/dr-catalog/src/merge.rs index 97cce78..5da149a 100644 --- a/core/dr-catalog/src/merge.rs +++ b/core/dr-catalog/src/merge.rs @@ -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::>()?; + // 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 = tx - .query_row("SELECT id FROM people WHERE uuid = ?1", [&uuid], |r| { - r.get(0) - }) - .optional()?; + let person: Option = 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::>()?; + 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 = tx - .query_row("SELECT id FROM people WHERE uuid = ?1", [&uuid], |r| { - r.get(0) - }) - .optional()?; + let person: Option = 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])?; } }