Confirm a group, and split one, in one transaction
`confirm_all` called `faces::confirm` per face, and `split_off` called `reject` then `confirm` per face: each opens and commits its own transaction, so a click on a group of several hundred was several hundred commits. `faces::confirm_all` is two statements — clear the rejections the confirmations override, then flip the rows — and `faces::reassign` does a split's reject-and-confirm for every face under one commit. 16 ms → 2 ms and 22 ms → 4 ms on the largest group.
This commit is contained in:
@@ -1057,6 +1057,72 @@ pub fn confirm(conn: &Connection, face: FaceId, person: PersonId) -> Result<(),
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The user says every suggested face of this person is right.
|
||||||
|
///
|
||||||
|
/// What "Confirm all" runs, and the reason it is not a loop over [`confirm`]:
|
||||||
|
/// that is a transaction per face, and on a group of several hundred it was
|
||||||
|
/// several hundred commits for one click. Two statements, one commit, and the
|
||||||
|
/// same two rules `confirm` applies face by face — an earlier rejection of
|
||||||
|
/// the pair is overridden, and a face already confirmed is left alone.
|
||||||
|
///
|
||||||
|
/// Returns how many suggestions became confirmations.
|
||||||
|
pub fn confirm_all(conn: &Connection, person: PersonId) -> Result<u64, CatalogError> {
|
||||||
|
let tx = conn.unchecked_transaction()?;
|
||||||
|
tx.execute(
|
||||||
|
"DELETE FROM face_person_rejected
|
||||||
|
WHERE person_id = ?1
|
||||||
|
AND face_id IN (SELECT face_id FROM face_person
|
||||||
|
WHERE person_id = ?1 AND confirmed = 0)",
|
||||||
|
[person.0 as i64],
|
||||||
|
)?;
|
||||||
|
let n = tx.execute(
|
||||||
|
"UPDATE face_person SET confirmed = 1, probability = 1.0
|
||||||
|
WHERE person_id = ?1 AND confirmed = 0",
|
||||||
|
[person.0 as i64],
|
||||||
|
)?;
|
||||||
|
tx.commit()?;
|
||||||
|
Ok(n as u64)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The user says these faces are `to`, not `from`.
|
||||||
|
///
|
||||||
|
/// [`reject`] from one and [`confirm`] onto the other, for every face, in one
|
||||||
|
/// transaction — what a split commits. Rejecting first is what stops the
|
||||||
|
/// split being undone: without it the next pass sees a face that looks like
|
||||||
|
/// `from` and suggests it straight back. Confirmed rather than suggested on
|
||||||
|
/// `to`, because the user has just asserted these belong together.
|
||||||
|
pub fn reassign(
|
||||||
|
conn: &Connection,
|
||||||
|
faces: &[FaceId],
|
||||||
|
from: PersonId,
|
||||||
|
to: PersonId,
|
||||||
|
) -> Result<(), CatalogError> {
|
||||||
|
let tx = conn.unchecked_transaction()?;
|
||||||
|
{
|
||||||
|
let mut reject = tx.prepare(
|
||||||
|
"INSERT OR IGNORE INTO face_person_rejected (face_id, person_id)
|
||||||
|
VALUES (?1, ?2)",
|
||||||
|
)?;
|
||||||
|
let mut unrejected =
|
||||||
|
tx.prepare("DELETE FROM face_person_rejected WHERE face_id = ?1 AND person_id = ?2")?;
|
||||||
|
let mut confirm = tx.prepare(
|
||||||
|
"INSERT INTO face_person (face_id, person_id, probability, confirmed)
|
||||||
|
VALUES (?1, ?2, 1.0, 1)
|
||||||
|
ON CONFLICT(face_id) DO UPDATE SET
|
||||||
|
person_id = excluded.person_id,
|
||||||
|
probability = 1.0,
|
||||||
|
confirmed = 1",
|
||||||
|
)?;
|
||||||
|
for face in faces {
|
||||||
|
reject.execute(rusqlite::params![face.0 as i64, from.0 as i64])?;
|
||||||
|
unrejected.execute(rusqlite::params![face.0 as i64, to.0 as i64])?;
|
||||||
|
confirm.execute(rusqlite::params![face.0 as i64, to.0 as i64])?;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
tx.commit()?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// The user says this face is **not** this person.
|
/// The user says this face is **not** this person.
|
||||||
///
|
///
|
||||||
/// Stored rather than implied by removal, so the next clustering pass does not
|
/// Stored rather than implied by removal, so the next clustering pass does not
|
||||||
|
|||||||
@@ -580,15 +580,7 @@ pub fn rename(
|
|||||||
/// right — the common case for a well-photographed person — should cost one
|
/// right — the common case for a well-photographed person — should cost one
|
||||||
/// click, not forty.
|
/// click, not forty.
|
||||||
pub fn confirm_all(catalog: &Catalog, person: PersonId) -> Result<usize, dr_catalog::CatalogError> {
|
pub fn confirm_all(catalog: &Catalog, person: PersonId) -> Result<usize, dr_catalog::CatalogError> {
|
||||||
let conn = catalog.connection();
|
faces::confirm_all(catalog.connection(), person).map(|n| n as usize)
|
||||||
let mut n = 0;
|
|
||||||
for f in faces::for_person(conn, person, true)? {
|
|
||||||
if !f.confirmed {
|
|
||||||
faces::confirm(conn, f.id, person)?;
|
|
||||||
n += 1;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Ok(n)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The user says this face is this person.
|
/// The user says this face is this person.
|
||||||
@@ -733,13 +725,9 @@ pub fn split_off(
|
|||||||
) -> Result<PersonId, dr_catalog::CatalogError> {
|
) -> Result<PersonId, dr_catalog::CatalogError> {
|
||||||
let conn = catalog.connection();
|
let conn = catalog.connection();
|
||||||
let new_person = faces::create_person(conn, name.trim())?;
|
let new_person = faces::create_person(conn, name.trim())?;
|
||||||
for &face in members {
|
// Rejected from `from` and confirmed onto the new person, in one
|
||||||
// Rejecting first is what stops the split being undone: without it the
|
// transaction rather than two per face: `faces::reassign` says why both.
|
||||||
// next pass sees a face that looks like `from` and suggests it straight
|
faces::reassign(conn, members, from, new_person)?;
|
||||||
// back, and the user's correction becomes an argument they keep having.
|
|
||||||
faces::reject(conn, face, from)?;
|
|
||||||
faces::confirm(conn, face, new_person)?;
|
|
||||||
}
|
|
||||||
Ok(new_person)
|
Ok(new_person)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user