List a person's photographs once instead of asking per image
Filtering the grid by one person under "All of them" took 38 s on the reference library (24k images, 19k faces): a click on Catherine, 775 photographs, spent 13 s counting, 20 s drawing the timeline and 4.4 s reading the window. The SQL took 10 ms when tried by hand, because the hand-written version used the "Any" term. The "All" term was a correlated COUNT(DISTINCT person_id) per image. With one person in the IN list SQLite drove that subquery from face_person_person, so every image walked every one of the person's faces and opened each to read its image_id: 24,000 x 775 probes, in each of the five queries a reload runs. Two people happened to plan from the image side and stayed fast, which is why only the single-person case crawled. Both modes now list the people's photographs once from face_person, the small side, and test the image's key against that list: GROUP BY image with HAVING COUNT(DISTINCT person_id) = n for "All", the plain list for "Any". On a copy of the reference catalog the grid window goes from 11.4 s to 23 ms and the count from 21.4 s to 1.8 ms. The eyes-open term rides inside the same list, unchanged. A test now checks the plan, not just the answer: no person or eyes term may be correlated.
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -314,27 +314,35 @@ impl RatingFilter {
|
||||
.collect::<Vec<_>>()
|
||||
.join(",");
|
||||
|
||||
terms.push(match self.people_mode {
|
||||
// `EXISTS` rather than a join, so a photograph holding three
|
||||
// faces of the same person appears once — the grid shows
|
||||
// pictures, not faces.
|
||||
PeopleMode::Any => format!(
|
||||
"EXISTS (SELECT 1 FROM faces f
|
||||
JOIN face_person fp ON fp.face_id = f.id
|
||||
WHERE f.image_id = i.id AND fp.person_id IN ({ids}){not_blinking})"
|
||||
),
|
||||
// Counting *distinct* people rather than ANDing one EXISTS per
|
||||
// person: same result, one subquery instead of n, and it does
|
||||
// not grow the statement with the selection. `DISTINCT` is
|
||||
// what makes it correct — three faces of Anna in one frame
|
||||
// must not satisfy a filter asking for Anna and Bob.
|
||||
// The people's photographs are listed once, from their faces, and
|
||||
// each image is tested against that list — not a subquery per
|
||||
// image. Spelled as a correlated `COUNT(DISTINCT ...)`, `All` with
|
||||
// one person was planned from `face_person_person`: every image
|
||||
// walked every one of that person's faces, 24,000 × 775 probes,
|
||||
// and a click on Catherine cost 38 s across the reload's five
|
||||
// queries. The list is a few hundred rows whichever way it is
|
||||
// asked, and an `IN` on the image's key is one probe.
|
||||
//
|
||||
// `IN` rather than a join, so a photograph holding three faces of
|
||||
// the same person appears once — the grid shows pictures, not
|
||||
// faces.
|
||||
let having = match self.people_mode {
|
||||
PeopleMode::Any => String::new(),
|
||||
// Counting *distinct* people rather than one `IN` per person:
|
||||
// same result, one list instead of n, and it does not grow the
|
||||
// statement with the selection. `DISTINCT` is what makes it
|
||||
// correct — three faces of Anna in one frame must not satisfy
|
||||
// a filter asking for Anna and Bob.
|
||||
PeopleMode::All => format!(
|
||||
"(SELECT COUNT(DISTINCT fp.person_id) FROM faces f
|
||||
JOIN face_person fp ON fp.face_id = f.id
|
||||
WHERE f.image_id = i.id AND fp.person_id IN ({ids}){not_blinking}) = {}",
|
||||
" GROUP BY f.image_id HAVING COUNT(DISTINCT fp.person_id) = {}",
|
||||
self.people.len()
|
||||
),
|
||||
});
|
||||
};
|
||||
terms.push(format!(
|
||||
"i.id IN (SELECT f.image_id FROM face_person fp
|
||||
JOIN faces f ON f.id = fp.face_id
|
||||
WHERE fp.person_id IN ({ids}){not_blinking}{having})"
|
||||
));
|
||||
} else if self.eyes_open {
|
||||
// Nobody in particular: no face in the frame may be a blink. A
|
||||
// photograph with no faces at all passes — there is no one in it
|
||||
@@ -701,6 +709,47 @@ mod tests {
|
||||
assert_eq!(total_images_scoped(&catalog, None, &f).unwrap(), 0);
|
||||
}
|
||||
|
||||
/// The people's photographs are listed once, not asked about per image.
|
||||
///
|
||||
/// A correlated person term let the planner drive it from the person's
|
||||
/// faces, once for every image in the library: 38 s for one click on the
|
||||
/// reference library under `All` with one person. The plan is what
|
||||
/// regressed, so the plan is what is checked — with the eyes term too,
|
||||
/// since it rides inside the same list.
|
||||
#[test]
|
||||
fn the_people_term_is_one_list_rather_than_a_probe_per_image() {
|
||||
let catalog = with_images(2);
|
||||
for (mode, eyes_open, people) in [
|
||||
(PeopleMode::Any, false, vec![4]),
|
||||
(PeopleMode::All, false, vec![4]),
|
||||
(PeopleMode::All, false, vec![4, 59]),
|
||||
(PeopleMode::All, true, vec![4]),
|
||||
] {
|
||||
let f = RatingFilter {
|
||||
people,
|
||||
people_mode: mode,
|
||||
eyes_open,
|
||||
..Default::default()
|
||||
};
|
||||
let mut q = catalog
|
||||
.connection()
|
||||
.prepare(&format!(
|
||||
"EXPLAIN QUERY PLAN SELECT i.id FROM images i WHERE {VISIBLE}{}",
|
||||
f.sql()
|
||||
))
|
||||
.unwrap();
|
||||
let plan: Vec<String> = q
|
||||
.query_map([], |r| r.get(3))
|
||||
.unwrap()
|
||||
.map(Result::unwrap)
|
||||
.collect();
|
||||
assert!(
|
||||
!plan.iter().any(|step| step.contains("CORRELATED")),
|
||||
"{mode:?}, eyes open {eyes_open}: {plan:#?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_people_narrows_nothing() {
|
||||
let catalog = with_images(3);
|
||||
|
||||
Reference in New Issue
Block a user