Merge pull request #1214 from arnav-hiwarkar/perf/project-size-sort

perf(project): read file sizes once per sort

crates/ui-egui/src/panels/project.rs

@@ -84,11 +84,14 @@ pub const COLUMNS: [(&str, &str, f32); 6] = [
 
 /// Sort siblings by the Project panel's sort column; ties fall back to the name.
 pub fn sort_items(items: &mut [&Item], col: &str, desc: bool) {
+    if col == "size" {
+        sort_by_size(items, desc, file_size);
+        return;
+    }
     let name = |i: &Item| i.name.to_lowercase();
     items.sort_by(|a, b| {
         let o = match col {
             "type" => a.type_name().cmp(b.type_name()),
-            "size" => file_size(a).cmp(&file_size(b)),
             "fps" => a.frame_rate().map(|r| r.as_f64()).partial_cmp(&b.frame_rate().map(|r| r.as_f64())).unwrap_or(std::cmp::Ordering::Equal),
             "duration" => a.duration().cmp(&b.duration()),
             "path" => file_path(a).cmp(file_path(b)),
@@ -101,6 +104,15 @@ pub fn sort_items(items: &mut [&Item], col: &str, desc: bool) {
     });
 }
 
+fn sort_by_size(items: &mut [&Item], desc: bool, mut size_of: impl FnMut(&Item) -> Option<u64>) {
+    let mut key = |it: &&Item| (size_of(it), it.name.to_lowercase());
+    if desc {
+        items.sort_by_cached_key(|it| std::cmp::Reverse(key(it)));
+    } else {
+        items.sort_by_cached_key(key);
+    }
+}
+
 /// Where a drop at a row lands: into a folder row, else into the row's folder; `None` row = root.
 pub fn drop_folder(project: &effectcraft_engine::project::Project, row: Option<ItemId>) -> Option<ItemId> {
     let it = project.item(row?)?;
@@ -1113,6 +1125,136 @@ pub fn set_column(cols: &mut Vec<String>, key: &str, on: bool) {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use effectcraft_engine::project::{Footage, Project};
+
+    struct SizeFiles(std::path::PathBuf);
+
+    impl SizeFiles {
+        fn new() -> Self {
+            static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
+            let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+            let path = std::env::temp_dir().join(format!("effectcraft-size-{}-{n}", std::process::id()));
+            std::fs::create_dir(&path).unwrap();
+            Self(path)
+        }
+
+        fn add(&self, project: &mut Project, name: &str, bytes: Option<u64>) {
+            let path = self.0.join(name);
+            if let Some(bytes) = bytes {
+                std::fs::File::create(&path).unwrap().set_len(bytes).unwrap();
+            }
+            project.add_item(name, Label::Aqua, None, ItemKind::Footage(Footage { path: path.to_string_lossy().into_owned(), ..Default::default() }));
+        }
+    }
+
+    impl Drop for SizeFiles {
+        fn drop(&mut self) {
+            let _ = std::fs::remove_dir_all(&self.0);
+        }
+    }
+
+    #[test]
+    fn size_keys_are_sampled_once_with_stable_ties() {
+        let mut project = Project::default();
+        for name in ["Zeta", "alpha", "ALPHA", "Missing", "Zero"] {
+            project.add_item(name, Label::Aqua, None, ItemKind::Folder);
+        }
+        for (desc, expected) in [(false, ["Missing", "Zero", "alpha", "ALPHA", "Zeta"]), (true, ["Zeta", "alpha", "ALPHA", "Zero", "Missing"])] {
+            let mut reads = std::collections::BTreeMap::new();
+            let mut rows = project.children(None);
+            sort_by_size(&mut rows, desc, |item| {
+                *reads.entry(item.id).or_insert(0) += 1;
+                match item.name.as_str() {
+                    "Zeta" => Some(30),
+                    "alpha" | "ALPHA" => Some(10),
+                    "Zero" => Some(0),
+                    _ => None,
+                }
+            });
+            assert_eq!(rows.iter().map(|item| item.name.as_str()).collect::<Vec<_>>(), expected);
+            assert_eq!(reads.len(), 5);
+            assert!(reads.values().all(|&count| count == 1));
+        }
+    }
+
+    #[test]
+    fn size_sort_reads_files_again_after_they_change() {
+        let files = SizeFiles::new();
+        let mut project = Project::default();
+        project.add_item("Folder", Label::Aqua, None, ItemKind::Folder);
+        files.add(&mut project, "Missing", None);
+        files.add(&mut project, "Zero", Some(0));
+        files.add(&mut project, "Small", Some(10));
+        files.add(&mut project, "Large", Some(20));
+        let mut rows = project.children(None);
+        sort_items(&mut rows, "size", false);
+        assert_eq!(rows.iter().map(|item| item.name.as_str()).collect::<Vec<_>>(), ["Folder", "Missing", "Zero", "Small", "Large"]);
+        sort_items(&mut rows, "size", true);
+        assert_eq!(rows.iter().map(|item| item.name.as_str()).collect::<Vec<_>>(), ["Large", "Small", "Zero", "Missing", "Folder"]);
+        std::fs::OpenOptions::new().write(true).open(files.0.join("Small")).unwrap().set_len(30).unwrap();
+        sort_items(&mut rows, "size", false);
+        assert_eq!(rows.iter().map(|item| item.name.as_str()).collect::<Vec<_>>(), ["Folder", "Missing", "Zero", "Large", "Small"]);
+        std::fs::remove_file(files.0.join("Large")).unwrap();
+        sort_items(&mut rows, "size", false);
+        assert_eq!(rows.iter().map(|item| item.name.as_str()).collect::<Vec<_>>(), ["Folder", "Large", "Missing", "Zero", "Small"]);
+    }
+
+    #[test]
+    fn size_sort_accepts_empty_and_singleton_lists() {
+        let mut project = Project::default();
+        let id = project.add_item("Only", Label::Aqua, None, ItemKind::Folder);
+        for desc in [false, true] {
+            sort_items(&mut [], "size", desc);
+            let mut rows = project.children(None);
+            sort_items(&mut rows, "size", desc);
+            assert_eq!(rows.iter().map(|item| item.id).collect::<Vec<_>>(), [id]);
+        }
+    }
+
+    #[test]
+    #[ignore = "filesystem benchmark; run with --ignored --nocapture"]
+    fn size_sort_benchmark() {
+        let files = SizeFiles::new();
+        let mut project = Project::default();
+        for i in 0..1000 {
+            files.add(&mut project, &format!("Clip {i:04}"), Some((i * 137) % 1000 + 1));
+        }
+        let original = project.children(None);
+        for item in &original {
+            assert!(file_size(item).is_some());
+        }
+        let mut before_times = Vec::new();
+        let mut after_times = Vec::new();
+        let mut before_reads = 0;
+        let mut after_reads = 0;
+        for _ in 0..5 {
+            let mut before = original.clone();
+            before_reads = 0;
+            let start = std::time::Instant::now();
+            before.sort_by(|a, b| {
+                before_reads += 2;
+                file_size(a).cmp(&file_size(b)).then_with(|| a.name.to_lowercase().cmp(&b.name.to_lowercase()))
+            });
+            before_times.push(start.elapsed());
+            let mut after = original.clone();
+            after_reads = 0;
+            let start = std::time::Instant::now();
+            sort_by_size(&mut after, false, |item| {
+                after_reads += 1;
+                file_size(item)
+            });
+            after_times.push(start.elapsed());
+            assert_eq!(before.iter().map(|item| item.id).collect::<Vec<_>>(), after.iter().map(|item| item.id).collect::<Vec<_>>());
+            assert_eq!(after_reads, 1000);
+            assert!(before_reads > after_reads);
+        }
+        before_times.sort();
+        after_times.sort();
+        println!(
+            "1,000 footage files; warmed metadata; five paired sorts\nmetadata reads/sort: {before_reads} -> {after_reads}\nmedian sort time: {:?} -> {:?}",
+            before_times[2], after_times[2]
+        );
+    }
 
     #[test]
     fn ae_duration_and_size_formats() {