diff --git a/.jules/bolt.md b/.jules/bolt.md index 541c0ab51..af606a123 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -17,3 +17,7 @@ ## 2025-05-19 - File Discovery Allocations **Learning:** In `discover_importing_files`, `WalkBuilder` results were being converted to `PathBuf` via `.map(|e| e.into_path())` *before* filtering. This caused allocations for every single file in the workspace (including excluded files and directories). **Action:** Filter `ignore::DirEntry` directly using `entry.file_type()` and `entry.path()` before mapping to `PathBuf`. This avoids allocations for non-matching files. + +## 2025-10-24 - Directory Traversal is_file() +**Learning:** When using directory traversal crates like `ignore` or `walkdir` in Rust, calling `entry.path().is_file()` on directory entries performs an unnecessary `stat` syscall. Using `entry.file_type().is_some_and(|ft| ft.is_file())` utilizes the metadata already retrieved during the `readdir` operation, significantly improving performance. +**Action:** Avoid calling `entry.path().is_file()` on `DirEntry` values from traversal libraries. Use `entry.file_type().is_some_and(|ft| ft.is_file())` instead. diff --git a/crates/mill-handlers/src/handlers/common/checksums.rs b/crates/mill-handlers/src/handlers/common/checksums.rs index 79a48a327..3e61c434f 100644 --- a/crates/mill-handlers/src/handlers/common/checksums.rs +++ b/crates/mill-handlers/src/handlers/common/checksums.rs @@ -79,7 +79,9 @@ pub async fn calculate_checksums_for_directory_rename( .build(); for entry in walker.flatten() { - if entry.path().is_file() { + // Avoid `entry.path().is_file()` to prevent unnecessary `stat` syscalls, + // relying on the cached metadata from the `readdir` operation. + if entry.file_type().is_some_and(|ft| ft.is_file()) { if let Ok(content) = context.app_state.file_service.read_file(entry.path()).await { file_checksums.insert( entry.path().to_string_lossy().to_string(), diff --git a/crates/mill-handlers/src/handlers/prune_ops.rs b/crates/mill-handlers/src/handlers/prune_ops.rs index 2f9ca3a91..f95d68732 100644 --- a/crates/mill-handlers/src/handlers/prune_ops.rs +++ b/crates/mill-handlers/src/handlers/prune_ops.rs @@ -665,7 +665,9 @@ impl PrunePlanner { let walker = ignore::WalkBuilder::new(&abs_dir).hidden(false).build(); let files = walker .flatten() - .filter(|entry| entry.path().is_file()) + // Avoid `entry.path().is_file()` to prevent unnecessary `stat` syscalls, + // relying on the cached metadata from the `readdir` operation. + .filter(|entry| entry.file_type().is_some_and(|ft| ft.is_file())) .map(|entry| entry.path().to_path_buf()) .collect(); Ok((files, abs_dir)) diff --git a/crates/mill-handlers/src/handlers/rename_ops/directory_rename.rs b/crates/mill-handlers/src/handlers/rename_ops/directory_rename.rs index 68aead607..43fb84d67 100644 --- a/crates/mill-handlers/src/handlers/rename_ops/directory_rename.rs +++ b/crates/mill-handlers/src/handlers/rename_ops/directory_rename.rs @@ -287,7 +287,9 @@ impl RenameService { let mut files_to_move = 0; let walker = ignore::WalkBuilder::new(&old_path).hidden(false).build(); for entry in walker.flatten() { - if entry.path().is_file() { + // Avoid `entry.path().is_file()` to prevent unnecessary `stat` syscalls, + // relying on the cached metadata from the `readdir` operation. + if entry.file_type().is_some_and(|ft| ft.is_file()) { files_to_move += 1; } } diff --git a/crates/mill-services/src/services/filesystem/file_service/rename.rs b/crates/mill-services/src/services/filesystem/file_service/rename.rs index b8b364ffa..c3c53f803 100644 --- a/crates/mill-services/src/services/filesystem/file_service/rename.rs +++ b/crates/mill-services/src/services/filesystem/file_service/rename.rs @@ -469,7 +469,9 @@ impl FileService { let mut files = Vec::new(); let walker = ignore::WalkBuilder::new(dir).hidden(false).build(); for entry in walker.flatten() { - if entry.path().is_file() { + // Avoid `entry.path().is_file()` to prevent unnecessary `stat` syscalls, + // relying on the cached metadata from the `readdir` operation. + if entry.file_type().is_some_and(|ft| ft.is_file()) { files.push(entry.path().to_path_buf()); } }