Page the grid along an index instead of sorting the library each time
Scrolling jittered, and this was the largest single reason. Every window the grid loads is `ORDER BY ... LIMIT n OFFSET k`, and neither half of that was being answered the cheap way. **The sort.** `GRID_ORDER` leads with `captured_at IS NULL`, so undated frames fall to the end. No ordinary index answers that — the leading term is an expression, not a column — so SQLite sorted the whole library into a temp b-tree on every window read, then threw away the first `k` rows of it. Schema V7 indexes the expression exactly as the query writes it, partial on the same `shadowed_by IS NULL AND trashed_at IS NULL` the grid filters by, so the read becomes a walk along the index. **The join.** `LEFT JOIN remote` was paged *after* it was joined, so reading 280 cells at offset 20,000 first seeked into `remote` for all 24,000 rows and then discarded 23,720 of them. The file ids are now fetched for the 280 rows that survived — the shape the badge and rating reads already use, one query for the window rather than one per cell. Measured together on 24,000 images at offset 20,000: **15.2 ms → 0.36 ms**, inside a scroll handler that has 16.7 ms to draw a frame. The test asserts on the query plan rather than on a duration, because there is no other symptom. A `GRID_ORDER` edited out of step with the index, or a column added back that drags `remote` in again, both still return exactly the right cells — just after sorting the library — and the jitter would come back with nothing to point at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+168
-35
@@ -2936,10 +2936,8 @@ pub fn read_cells_scoped(
|
||||
.join(",");
|
||||
let rated = filter.sql();
|
||||
let sql = format!(
|
||||
"SELECT i.id, i.source_ref, r.file_id, i.file_size,
|
||||
i.metadata_state, i.captured_at
|
||||
"SELECT {CELL_COLUMNS}
|
||||
FROM images i
|
||||
LEFT JOIN remote r ON r.image_id = i.id
|
||||
WHERE {VISIBLE}{rated}
|
||||
AND i.id IN (SELECT image_id FROM collection_members
|
||||
WHERE collection_id IN ({placeholders}))
|
||||
@@ -2954,10 +2952,14 @@ pub fn read_cells_scoped(
|
||||
params.push(rusqlite::types::Value::Integer(limit as i64));
|
||||
params.push(rusqlite::types::Value::Integer(offset as i64));
|
||||
|
||||
let mut stmt = catalog.connection().prepare(&sql)?;
|
||||
let rows = stmt
|
||||
.query_map(rusqlite::params_from_iter(params.iter()), row_to_cell)?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let mut rows = {
|
||||
let mut stmt = catalog.connection().prepare(&sql)?;
|
||||
let read = stmt
|
||||
.query_map(rusqlite::params_from_iter(params.iter()), row_to_cell)?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
read
|
||||
};
|
||||
attach_file_ids(catalog, &mut rows);
|
||||
Ok(rows)
|
||||
}
|
||||
|
||||
@@ -2968,18 +2970,20 @@ fn read_cells_all(
|
||||
limit: usize,
|
||||
) -> Result<Vec<LibraryCell>, dr_catalog::CatalogError> {
|
||||
let rated = filter.sql();
|
||||
let mut stmt = catalog.connection().prepare(&format!(
|
||||
"SELECT i.id, i.source_ref, r.file_id, i.file_size,
|
||||
i.metadata_state, i.captured_at
|
||||
FROM images i
|
||||
LEFT JOIN remote r ON r.image_id = i.id
|
||||
WHERE {VISIBLE}{rated}
|
||||
{GRID_ORDER}
|
||||
LIMIT ?1 OFFSET ?2"
|
||||
))?;
|
||||
let rows = stmt
|
||||
.query_map(rusqlite::params![limit as i64, offset as i64], row_to_cell)?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let mut rows = {
|
||||
let mut stmt = catalog.connection().prepare(&format!(
|
||||
"SELECT {CELL_COLUMNS}
|
||||
FROM images i
|
||||
WHERE {VISIBLE}{rated}
|
||||
{GRID_ORDER}
|
||||
LIMIT ?1 OFFSET ?2"
|
||||
))?;
|
||||
let read = stmt
|
||||
.query_map(rusqlite::params![limit as i64, offset as i64], row_to_cell)?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
read
|
||||
};
|
||||
attach_file_ids(catalog, &mut rows);
|
||||
Ok(rows)
|
||||
}
|
||||
|
||||
@@ -3000,18 +3004,20 @@ pub fn read_trashed_cells(
|
||||
offset: usize,
|
||||
limit: usize,
|
||||
) -> Result<Vec<LibraryCell>, dr_catalog::CatalogError> {
|
||||
let mut stmt = catalog.connection().prepare(&format!(
|
||||
"SELECT i.id, i.source_ref, r.file_id, i.file_size,
|
||||
i.metadata_state, i.captured_at
|
||||
FROM images i
|
||||
LEFT JOIN remote r ON r.image_id = i.id
|
||||
WHERE {TRASHED}
|
||||
{TRASH_ORDER}
|
||||
LIMIT ?1 OFFSET ?2"
|
||||
))?;
|
||||
let rows = stmt
|
||||
.query_map(rusqlite::params![limit as i64, offset as i64], row_to_cell)?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let mut rows = {
|
||||
let mut stmt = catalog.connection().prepare(&format!(
|
||||
"SELECT {CELL_COLUMNS}
|
||||
FROM images i
|
||||
WHERE {TRASHED}
|
||||
{TRASH_ORDER}
|
||||
LIMIT ?1 OFFSET ?2"
|
||||
))?;
|
||||
let read = stmt
|
||||
.query_map(rusqlite::params![limit as i64, offset as i64], row_to_cell)?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
read
|
||||
};
|
||||
attach_file_ids(catalog, &mut rows);
|
||||
Ok(rows)
|
||||
}
|
||||
|
||||
@@ -3091,20 +3097,90 @@ pub fn read_ids_span(
|
||||
Ok(ids)
|
||||
}
|
||||
|
||||
/// The columns every windowed read selects, in the order [`row_to_cell`] reads
|
||||
/// them.
|
||||
///
|
||||
/// Named rather than repeated so the three readers cannot drift — and so that
|
||||
/// the one column that is *not* here stays conspicuous. See
|
||||
/// [`attach_file_ids`] for why the server's file id is fetched separately.
|
||||
const CELL_COLUMNS: &str =
|
||||
"i.id, i.source_ref, i.file_size, i.metadata_state, i.captured_at";
|
||||
|
||||
/// Shared row mapping, so the scoped and unscoped queries cannot drift.
|
||||
///
|
||||
/// `file_id` is left empty here and filled by [`attach_file_ids`].
|
||||
fn row_to_cell(r: &rusqlite::Row) -> rusqlite::Result<LibraryCell> {
|
||||
let path: String = r.get(1)?;
|
||||
Ok(LibraryCell {
|
||||
image_id: r.get(0)?,
|
||||
name: path.rsplit(['/', ':']).next().unwrap_or(&path).to_string(),
|
||||
remote_path: path,
|
||||
file_id: r.get::<_, Option<i64>>(2)?.map(|v| v as u64),
|
||||
size: r.get::<_, Option<i64>>(3)?.unwrap_or(0) as u64,
|
||||
metadata_state: r.get::<_, i64>(4)? as u8,
|
||||
captured_at: r.get(5)?,
|
||||
file_id: None,
|
||||
size: r.get::<_, Option<i64>>(2)?.unwrap_or(0) as u64,
|
||||
metadata_state: r.get::<_, i64>(3)? as u8,
|
||||
captured_at: r.get(4)?,
|
||||
})
|
||||
}
|
||||
|
||||
/// TRACES: NFR-P5
|
||||
/// Fill in each cell's server file id, in one query for the whole window.
|
||||
///
|
||||
/// # Why this is not a `LEFT JOIN` any more
|
||||
///
|
||||
/// It was, and it was the single most expensive thing the grid did while a
|
||||
/// finger was on it. A window is `ORDER BY ... LIMIT n OFFSET k`, and SQLite
|
||||
/// answers a join like that by joining *first* and paging after — so reading
|
||||
/// 280 cells at offset 20,000 meant an index seek into `remote` for all 24,000
|
||||
/// rows, 23,720 of which were then discarded. Measured at 15.2 ms, inside the
|
||||
/// scroll handler, against a 16.7 ms frame.
|
||||
///
|
||||
/// Paging over `images` alone is 0.36 ms with `images_grid_order` (schema V7),
|
||||
/// and this fetches the ids for the 280 rows that survived. The same shape the
|
||||
/// badge and rating reads already use: one query for the window, never one per
|
||||
/// cell.
|
||||
///
|
||||
/// Silent on failure, and cells keep `file_id: None`: that is the same state a
|
||||
/// photograph the scan has not reached the server for is in, and the callers
|
||||
/// already treat it as "no cached thumbnail to key on" rather than an error.
|
||||
fn attach_file_ids(catalog: &Catalog, cells: &mut [LibraryCell]) {
|
||||
if cells.is_empty() {
|
||||
return;
|
||||
}
|
||||
let placeholders = std::iter::repeat_n("?", cells.len())
|
||||
.collect::<Vec<_>>()
|
||||
.join(",");
|
||||
let sql = format!("SELECT image_id, file_id FROM remote WHERE image_id IN ({placeholders})");
|
||||
let params: Vec<rusqlite::types::Value> = cells
|
||||
.iter()
|
||||
.map(|c| rusqlite::types::Value::Integer(c.image_id))
|
||||
.collect();
|
||||
|
||||
let mut stmt = match catalog.connection().prepare(&sql) {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
log::debug!("reading file ids for the window: {e}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let rows = stmt.query_map(rusqlite::params_from_iter(params.iter()), |r| {
|
||||
Ok((r.get::<_, i64>(0)?, r.get::<_, Option<i64>>(1)?))
|
||||
});
|
||||
let found: std::collections::HashMap<i64, Option<i64>> = match rows {
|
||||
Ok(rows) => rows.flatten().collect(),
|
||||
Err(e) => {
|
||||
log::debug!("reading file ids for the window: {e}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
for cell in cells {
|
||||
cell.file_id = found
|
||||
.get(&cell.image_id)
|
||||
.copied()
|
||||
.flatten()
|
||||
.map(|v| v as u64);
|
||||
}
|
||||
}
|
||||
|
||||
/// Total images in the catalog, or in one collection and its descendants.
|
||||
///
|
||||
/// Counts exactly what [`read_cells_scoped`] would list, filter included. The
|
||||
@@ -3819,6 +3895,63 @@ mod tests {
|
||||
assert_eq!(page[0].name, "img010.CR2");
|
||||
}
|
||||
|
||||
/// TRACES: NFR-P5
|
||||
/// The grid's window read must be answered by walking `images_grid_order`,
|
||||
/// never by sorting the library into a temp b-tree.
|
||||
///
|
||||
/// This asserts on the *query plan* rather than on a duration, because the
|
||||
/// failure has no other symptom: a `GRID_ORDER` edited out of step with the
|
||||
/// index in schema V7, or a column added back into the paging query that
|
||||
/// drags `remote` in again, both still return the right cells. They just
|
||||
/// return them after sorting 24,000 rows, inside the scroll handler — which
|
||||
/// is the jitter this pair was introduced to remove, and it would come back
|
||||
/// silently.
|
||||
#[test]
|
||||
fn the_window_read_walks_the_ordering_index() {
|
||||
let catalog = with_images(20);
|
||||
let plan: Vec<String> = catalog
|
||||
.connection()
|
||||
.prepare(&format!(
|
||||
"EXPLAIN QUERY PLAN
|
||||
SELECT {CELL_COLUMNS} FROM images i
|
||||
WHERE {VISIBLE}
|
||||
{GRID_ORDER}
|
||||
LIMIT 10 OFFSET 5"
|
||||
))
|
||||
.unwrap()
|
||||
.query_map([], |r| r.get::<_, String>(3))
|
||||
.unwrap()
|
||||
.flatten()
|
||||
.collect();
|
||||
let plan = plan.join(" | ");
|
||||
|
||||
assert!(
|
||||
plan.contains("images_grid_order"),
|
||||
"the window read is not using the ordering index: {plan}"
|
||||
);
|
||||
assert!(
|
||||
!plan.contains("TEMP B-TREE"),
|
||||
"the window read is still sorting the whole library: {plan}"
|
||||
);
|
||||
assert!(
|
||||
!plan.to_lowercase().contains("remote"),
|
||||
"the window read is joining `remote` again, which pages the whole \
|
||||
library before it discards it: {plan}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The file ids still arrive, now that they come from a second query.
|
||||
#[test]
|
||||
fn a_window_still_carries_the_server_file_ids() {
|
||||
let catalog = with_images(20);
|
||||
let page = read_cells(&catalog, 5, 4).unwrap();
|
||||
assert_eq!(page.len(), 4);
|
||||
assert!(
|
||||
page.iter().all(|c| c.file_id.is_some()),
|
||||
"a cell lost its file id when the join was split out"
|
||||
);
|
||||
}
|
||||
|
||||
/// A catalog with `n` images, ready to file into collections.
|
||||
fn with_images(n: usize) -> Catalog {
|
||||
let catalog = Catalog::in_memory().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user