Release 0.24.1

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.

Cargo.lock

@@ -1265,7 +1265,7 @@ checksum = "f27ae1dd37df86211c42e150270f82743308803d90a6f6e6651cd730d5e1732f"
 
 [[package]]
 name = "darkroom-android"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "android_logger",
  "dr-plat",
@@ -1278,7 +1278,7 @@ dependencies = [
 
 [[package]]
 name = "darkroom-desktop"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "anyhow",
  "dr-plat",
@@ -1454,7 +1454,7 @@ checksum = "d8b14ccef22fc6f5a8f4d7d768562a182c04ce9a3b3157b91390b52ddfdf1a76"
 
 [[package]]
 name = "dr-bench"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "anyhow",
  "dr-catalog",
@@ -1471,7 +1471,7 @@ dependencies = [
 
 [[package]]
 name = "dr-catalog"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-face",
  "dr-plat",
@@ -1486,7 +1486,7 @@ dependencies = [
 
 [[package]]
 name = "dr-decode"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-types",
  "env_logger",
@@ -1500,7 +1500,7 @@ dependencies = [
 
 [[package]]
 name = "dr-denoise"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-decode",
  "dr-gpu",
@@ -1517,7 +1517,7 @@ dependencies = [
 
 [[package]]
 name = "dr-export"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-decode",
  "dr-gpu",
@@ -1536,7 +1536,7 @@ dependencies = [
 
 [[package]]
 name = "dr-face"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-inference-engine",
  "env_logger",
@@ -1549,7 +1549,7 @@ dependencies = [
 
 [[package]]
 name = "dr-film"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "log",
  "serde",
@@ -1558,7 +1558,7 @@ dependencies = [
 
 [[package]]
 name = "dr-gpu"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "bytemuck",
  "dr-decode",
@@ -1576,7 +1576,7 @@ dependencies = [
 
 [[package]]
 name = "dr-inference-engine"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "env_logger",
  "libloading",
@@ -1591,7 +1591,7 @@ dependencies = [
 
 [[package]]
 name = "dr-ingest"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-plat",
  "dr-types",
@@ -1603,7 +1603,7 @@ dependencies = [
 
 [[package]]
 name = "dr-lens"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "lensfun",
  "log",
@@ -1611,7 +1611,7 @@ dependencies = [
 
 [[package]]
 name = "dr-pano"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-decode",
  "dr-inference-engine",
@@ -1625,7 +1625,7 @@ dependencies = [
 
 [[package]]
 name = "dr-pipeline"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-types",
  "log",
@@ -1634,7 +1634,7 @@ dependencies = [
 
 [[package]]
 name = "dr-plat"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "android-native-keyring-store",
  "dr-types",
@@ -1650,7 +1650,7 @@ dependencies = [
 
 [[package]]
 name = "dr-preset-xmp"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-pipeline",
  "log",
@@ -1660,7 +1660,7 @@ dependencies = [
 
 [[package]]
 name = "dr-segment"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-inference-engine",
  "env_logger",
@@ -1673,7 +1673,7 @@ dependencies = [
 
 [[package]]
 name = "dr-sync"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "async-trait",
  "dr-plat",
@@ -1687,7 +1687,7 @@ dependencies = [
 
 [[package]]
 name = "dr-sync-folder"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "async-trait",
  "dr-sync",
@@ -1699,7 +1699,7 @@ dependencies = [
 
 [[package]]
 name = "dr-sync-nextcloud"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "async-trait",
  "dr-decode",
@@ -1721,7 +1721,7 @@ dependencies = [
 
 [[package]]
 name = "dr-thumbs"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-types",
  "jpeg-encoder",
@@ -1733,7 +1733,7 @@ dependencies = [
 
 [[package]]
 name = "dr-types"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "serde",
  "serde_json",
@@ -1742,7 +1742,7 @@ dependencies = [
 
 [[package]]
 name = "dr-ui"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -1792,7 +1792,7 @@ dependencies = [
 
 [[package]]
 name = "dr-xmp"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "dr-types",
  "log",
@@ -7126,7 +7126,7 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3"
 
 [[package]]
 name = "traceability"
-version = "0.24.0"
+version = "0.24.1"
 dependencies = [
  "anyhow",
  "proc-macro2",

Cargo.toml

@@ -33,7 +33,7 @@ members = [
 exclude = ["third_party"]
 
 [workspace.package]
-version = "0.24.0"
+version = "0.24.1"
 edition = "2021"
 rust-version = "1.92"
 license = "GPL-3.0-or-later"

README.md

@@ -201,7 +201,7 @@ controls, its place in the chain and its tests.
 
 ## Where it stands
 
-**0.24.0**, thirty-nine tagged releases in. 193 numbered requirements in
+**0.24.1**, forty tagged releases in. 193 numbered requirements in
 scope, 85% of them claimed by code and [traced to it](docs/dev/traceability.md);
 the rest are written down rather than merely absent.
 

packaging/PKGBUILD

@@ -4,7 +4,7 @@
 # makes `makepkg -si` in this directory install what you are actually working
 # on. Swap `source` for a tagged tarball when there is something to release.
 pkgname=darkroom
-pkgver=0.24.0
+pkgver=0.24.1
 # Back to 1 with the version: a new pkgver is a new archive name, so there is
 # nothing for makepkg to reuse and nothing for a release number to disambiguate.
 pkgrel=1

ui/dr-ui/src/library/filters.rs

@@ -314,27 +314,35 @@ impl RatingFilter {
                 .collect::<Vec<_>>()
                 .join(",");
 
-            terms.push(match self.people_mode {
+            // The people's photographs are listed once, from their faces, and
-                // `EXISTS` rather than a join, so a photograph holding three
+            // each image is tested against that list — not a subquery per
-                // faces of the same person appears once — the grid shows
+            // image. Spelled as a correlated `COUNT(DISTINCT ...)`, `All` with
-                // pictures, not faces.
+            // one person was planned from `face_person_person`: every image
-                PeopleMode::Any => format!(
+            // walked every one of that person's faces, 24,000 × 775 probes,
-                    "EXISTS (SELECT 1 FROM faces f
+            // and a click on Catherine cost 38 s across the reload's five
-                               JOIN face_person fp ON fp.face_id = f.id
+            // queries. The list is a few hundred rows whichever way it is
-                              WHERE f.image_id = i.id AND fp.person_id IN ({ids}){not_blinking})"
+            // asked, and an `IN` on the image's key is one probe.
-                ),
+            //
-                // Counting *distinct* people rather than ANDing one EXISTS per
+            // `IN` rather than a join, so a photograph holding three faces of
-                // person: same result, one subquery instead of n, and it does
+            // the same person appears once — the grid shows pictures, not
-                // not grow the statement with the selection. `DISTINCT` is
+            // faces.
-                // what makes it correct — three faces of Anna in one frame
+            let having = match self.people_mode {
-                // must not satisfy a filter asking for Anna and Bob.
+                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
+                    " GROUP BY f.image_id HAVING COUNT(DISTINCT fp.person_id) = {}",
-                        JOIN face_person fp ON fp.face_id = f.id
-                       WHERE f.image_id = i.id AND fp.person_id IN ({ids}){not_blinking}) = {}",
                     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);