From 0fdb3ff37d62e35d8673882e4ff42b96acabe020 Mon Sep 17 00:00:00 2001 From: Khushal Bhardwaj Date: Tue, 20 Oct 2026 07:44:38 +0000 Subject: [PATCH 1/2] perf(desktop-gpui): bound capped screenshot scans like recordings The tray recents and the main window's screenshots panel parsed every bundle's recording-meta.json before truncating to 9 / 20 items. Screenshot listing now stats each bundle's PNG first (original.png, falling back to a directory search for legacy bundles), sorts by that timestamp, and parses meta lazily, so capped callers only parse the bundles they keep. The full listing used by the settings page is unchanged. --- apps/desktop-gpui/src/library.rs | 227 +++++++++++++++++++++------ apps/desktop-gpui/src/main_window.rs | 4 +- 2 files changed, 182 insertions(+), 49 deletions(-) diff --git a/apps/desktop-gpui/src/library.rs b/apps/desktop-gpui/src/library.rs index dc57098a7ab..fbc81b40cf2 100644 --- a/apps/desktop-gpui/src/library.rs +++ b/apps/desktop-gpui/src/library.rs @@ -72,9 +72,11 @@ pub struct RecentItem { /// `media_sort_time_millis` (`lib.rs:3966-3972`): the filesystem `created()` /// time of the path, falling back to `modified()`, then to the epoch. fn media_sort_time_millis(path: &Path) -> f64 { - let Ok(metadata) = path.metadata() else { - return 0.0; - }; + path.metadata() + .map_or(0.0, |metadata| metadata_sort_time_millis(&metadata)) +} + +fn metadata_sort_time_millis(metadata: &std::fs::Metadata) -> f64 { metadata .created() .or_else(|_| metadata.modified()) @@ -672,44 +674,61 @@ pub struct ScreenshotItem { /// `list_screenshots` (`lib.rs:4055-4091`): `*.cap` directories only, and the /// sort key is the PNG's timestamp rather than the directory's. pub fn list_screenshots_in(dir: &Path) -> Vec { - let Ok(entries) = std::fs::read_dir(dir) else { - return Vec::new(); - }; + screenshot_items_in(dir).collect() +} - let mut out = Vec::new(); - for entry in entries.flatten() { - let bundle = entry.path(); - if !bundle.is_dir() || bundle.extension().and_then(|ext| ext.to_str()) != Some("cap") { - continue; +fn screenshot_items_in(dir: &Path) -> impl Iterator { + let mut bundles = Vec::new(); + if let Ok(entries) = std::fs::read_dir(dir) { + for entry in entries.flatten() { + let bundle = entry.path(); + if bundle.extension().and_then(|ext| ext.to_str()) != Some("cap") || !bundle.is_dir() { + continue; + } + if let Some((png, sort_time_millis)) = screenshot_png(&bundle) { + bundles.push((bundle, png, sort_time_millis)); + } } - let Ok(meta) = RecordingMeta::load_for_project(&bundle) else { - continue; - }; - let Some(png) = std::fs::read_dir(&bundle).ok().and_then(|entries| { - entries - .flatten() - .map(|entry| entry.path()) - .find(|path| path.extension().and_then(|ext| ext.to_str()) == Some("png")) - }) else { - continue; - }; + } + bundles.sort_by(|(_, _, a), (_, _, b)| b.total_cmp(a)); + bundles + .into_iter() + .filter_map(|(bundle, png, sort_time_millis)| { + let meta = RecordingMeta::load_for_project(&bundle).ok()?; + Some(ScreenshotItem { + path: png.clone(), + pretty_name: meta.pretty_name, + sort_time_millis, + thumbnail: Some(png), + bundle, + }) + }) +} - out.push(ScreenshotItem { - path: png.clone(), - pretty_name: meta.pretty_name, - sort_time_millis: media_sort_time_millis(&png), - thumbnail: Some(png), - bundle, - }); +fn screenshot_png(bundle: &Path) -> Option<(PathBuf, f64)> { + let original = bundle.join("original.png"); + if let Ok(metadata) = original.metadata() { + return Some((original, metadata_sort_time_millis(&metadata))); } - out.sort_by(|a, b| b.sort_time_millis.total_cmp(&a.sort_time_millis)); - out + let png = std::fs::read_dir(bundle) + .ok()? + .flatten() + .map(|entry| entry.path()) + .find(|path| path.extension().and_then(|ext| ext.to_str()) == Some("png"))?; + let sort_time_millis = media_sort_time_millis(&png); + Some((png, sort_time_millis)) } pub fn list_screenshots() -> Vec { list_screenshots_in(&screenshots_dir()) } +pub fn list_screenshots_limited(limit: usize) -> Vec { + screenshot_items_in(&screenshots_dir()) + .take(limit) + .collect() +} + /// Delete the `.cap` bundle that owns `path` (the PNG or the directory). pub fn delete_screenshot(path: &Path) -> Result<(), String> { let bundle = if path.is_dir() { @@ -750,16 +769,20 @@ pub fn delete_screenshot(path: &Path) -> Result<(), String> { } fn scan_screenshots(dir: &Path, out: &mut Vec) { - out.extend(list_screenshots_in(dir).into_iter().map(|item| RecentItem { - kind: MediaKind::Screenshot, - bundle: item.bundle, - pretty_name: item.pretty_name, - clip_count: 1, - sort_time_millis: item.sort_time_millis, - thumbnail_version: item.thumbnail.as_deref().and_then(source_mtime_nanos), - thumbnail: item.thumbnail, - sharing: None, - })); + out.extend( + screenshot_items_in(dir) + .take(RECENT_MEDIA_LIMIT) + .map(|item| RecentItem { + kind: MediaKind::Screenshot, + bundle: item.bundle, + pretty_name: item.pretty_name, + clip_count: 1, + sort_time_millis: item.sort_time_millis, + thumbnail_version: item.thumbnail.as_deref().and_then(source_mtime_nanos), + thumbnail: item.thumbnail, + sharing: None, + }), + ); } fn newest_first(items: &mut [RecentItem]) { @@ -777,13 +800,8 @@ pub fn recent_media_in(recording_dirs: &[PathBuf], screenshots_dir: &Path) -> Ve let mut recordings = Vec::new(); scan_recordings(recording_dirs, &mut recordings); - let mut screenshots = Vec::new(); - scan_screenshots(screenshots_dir, &mut screenshots); - newest_first(&mut screenshots); - screenshots.truncate(RECENT_MEDIA_LIMIT); - let mut merged = recordings; - merged.extend(screenshots); + scan_screenshots(screenshots_dir, &mut merged); newest_first(&mut merged); merged.truncate(RECENT_MEDIA_LIMIT); merged @@ -1481,6 +1499,121 @@ mod tests { std::fs::remove_dir_all(root).unwrap(); } + #[test] + fn bounded_screenshots_match_the_full_listing_and_parse_lazily() { + fn eager_listing(dir: &Path) -> Vec { + let mut out = Vec::new(); + for entry in std::fs::read_dir(dir).unwrap().flatten() { + let bundle = entry.path(); + if !bundle.is_dir() + || bundle.extension().and_then(|ext| ext.to_str()) != Some("cap") + { + continue; + } + let Ok(meta) = RecordingMeta::load_for_project(&bundle) else { + continue; + }; + let Some(png) = std::fs::read_dir(&bundle) + .unwrap() + .flatten() + .find_map(|entry| { + let path = entry.path(); + (path.extension().and_then(|ext| ext.to_str()) == Some("png")) + .then_some(path) + }) + else { + continue; + }; + out.push(ScreenshotItem { + path: png.clone(), + pretty_name: meta.pretty_name, + sort_time_millis: media_sort_time_millis(&png), + thumbnail: Some(png), + bundle, + }); + } + out.sort_by(|a, b| b.sort_time_millis.total_cmp(&a.sort_time_millis)); + out + } + + fn write_shot(dir: &Path, name: &str) -> PathBuf { + write_bundle( + dir, + name, + &format!(r#"{{"pretty_name":"{name}","sharing":null,"fps":0}}"#), + ) + } + + let root = temp_dir("bounded-screenshots"); + let legacy = write_shot(&root, "legacy"); + std::fs::write(legacy.join("shot.png"), b"png").unwrap(); + write_shot(&root, "no-png"); + let not_a_bundle = root.join("not-a-bundle"); + std::fs::create_dir_all(¬_a_bundle).unwrap(); + std::fs::write(not_a_bundle.join("original.png"), b"png").unwrap(); + std::fs::write(root.join("loose.cap"), b"").unwrap(); + for i in 0..30 { + std::thread::sleep(std::time::Duration::from_millis(6)); + let bundle = write_shot(&root, &format!("shot-{:02}", (i * 7) % 30)); + std::fs::write(bundle.join("original.png"), b"png").unwrap(); + } + std::thread::sleep(std::time::Duration::from_millis(6)); + let tie_a = write_shot(&root, "tie-a"); + std::fs::write(tie_a.join("original.png"), b"png").unwrap(); + let tie_b = write_shot(&root, "tie-b"); + std::fs::hard_link(tie_a.join("original.png"), tie_b.join("original.png")).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(6)); + let corrupt = write_bundle(&root, "corrupt", "{ not json "); + std::fs::write(corrupt.join("original.png"), b"png").unwrap(); + + let full = list_screenshots_in(&root); + assert_eq!(full, eager_listing(&root)); + assert_eq!(full.len(), 33); + assert!(full.iter().all(|item| item.bundle != corrupt)); + let legacy_item = full.iter().find(|item| item.bundle == legacy).unwrap(); + assert_eq!(legacy_item.path, legacy.join("shot.png")); + assert_eq!(full.last().unwrap().bundle, legacy); + + assert_eq!(full[0].sort_time_millis, full[1].sort_time_millis); + let read_dir_order: Vec = std::fs::read_dir(&root) + .unwrap() + .flatten() + .map(|entry| entry.path()) + .filter(|path| *path == tie_a || *path == tie_b) + .collect(); + assert_eq!( + [full[0].bundle.clone(), full[1].bundle.clone()], + read_dir_order[..], + "equal timestamps keep directory order" + ); + assert!( + full[1..] + .windows(2) + .all(|pair| pair[0].sort_time_millis > pair[1].sort_time_millis) + ); + + for limit in [0, 1, 9, 20, 100] { + assert_eq!( + screenshot_items_in(&root).take(limit).collect::>(), + full[..limit.min(full.len())], + "limit {limit}" + ); + } + + let mut lazy = screenshot_items_in(&root); + let head: Vec<_> = lazy.by_ref().take(RECENT_MEDIA_LIMIT).collect(); + assert_eq!(head, full[..RECENT_MEDIA_LIMIT]); + for item in &full[RECENT_MEDIA_LIMIT..] { + std::fs::remove_file(item.bundle.join("recording-meta.json")).unwrap(); + } + assert!( + lazy.next().is_none(), + "metas past the consumed prefix are read on demand" + ); + + std::fs::remove_dir_all(&root).unwrap(); + } + #[test] #[ignore = "manual filesystem benchmark; set CAP_GPUI_LIBRARY_BENCH_DIR and CAP_GPUI_LIBRARY_BENCH_MODE"] fn recording_scan_benchmark() { diff --git a/apps/desktop-gpui/src/main_window.rs b/apps/desktop-gpui/src/main_window.rs index cabac8bdd14..249628cee51 100644 --- a/apps/desktop-gpui/src/main_window.rs +++ b/apps/desktop-gpui/src/main_window.rs @@ -4131,8 +4131,8 @@ impl MainWindow { (Some(items), None) } LibraryKind::Screenshots => { - let mut items = library::list_screenshots(); - items.truncate(library::LIBRARY_PANEL_LIMIT); + let items = + library::list_screenshots_limited(library::LIBRARY_PANEL_LIMIT); (None, Some(items)) } } From 34c4f1c9f3f594f745ff2000f3f53e0095f306b6 Mon Sep 17 00:00:00 2001 From: Khushal Bhardwaj Date: Tue, 20 Oct 2026 11:50:35 +0000 Subject: [PATCH 2/2] test(desktop-gpui): make the bounded screenshot test tolerate timestamp ties --- apps/desktop-gpui/src/library.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/apps/desktop-gpui/src/library.rs b/apps/desktop-gpui/src/library.rs index fbc81b40cf2..ad940c057b9 100644 --- a/apps/desktop-gpui/src/library.rs +++ b/apps/desktop-gpui/src/library.rs @@ -1572,24 +1572,32 @@ mod tests { assert!(full.iter().all(|item| item.bundle != corrupt)); let legacy_item = full.iter().find(|item| item.bundle == legacy).unwrap(); assert_eq!(legacy_item.path, legacy.join("shot.png")); - assert_eq!(full.last().unwrap().bundle, legacy); - assert_eq!(full[0].sort_time_millis, full[1].sort_time_millis); let read_dir_order: Vec = std::fs::read_dir(&root) .unwrap() .flatten() .map(|entry| entry.path()) - .filter(|path| *path == tie_a || *path == tie_b) .collect(); - assert_eq!( - [full[0].bundle.clone(), full[1].bundle.clone()], - read_dir_order[..], - "equal timestamps keep directory order" - ); + let read_dir_position = |bundle: &Path| { + read_dir_order + .iter() + .position(|path| path == bundle) + .unwrap() + }; + let tie_time = |bundle: &Path| { + full.iter() + .find(|item| item.bundle == bundle) + .unwrap() + .sort_time_millis + }; + assert_eq!(tie_time(&tie_a), tie_time(&tie_b)); assert!( - full[1..] - .windows(2) - .all(|pair| pair[0].sort_time_millis > pair[1].sort_time_millis) + full.windows(2).all(|pair| { + pair[0].sort_time_millis > pair[1].sort_time_millis + || (pair[0].sort_time_millis == pair[1].sort_time_millis + && read_dir_position(&pair[0].bundle) < read_dir_position(&pair[1].bundle)) + }), + "newest first, equal timestamps keep directory order" ); for limit in [0, 1, 9, 20, 100] {