feat(cp): optimize directory copy by caching file checks and refactoring calls (#8805)

* feat: optimize directory copy by caching file checks and refactoring calls

- Add `target_is_file` field to Context to avoid repeated `stat` calls on target
- Refactor `copy_direntry` to accept `&Entry` and additional bool params for symlink/directory handling
- Use cached value and local variables for cleaner access to entry properties
- Pass `created_parent_dirs` to `copy_file` for improved directory creation tracking

This reduces filesystem overhead in cp operations when copying directories.

src/uu/cp/src/copydir.rs

@@ -99,6 +99,9 @@ struct Context<'a> {
     /// The target path to which the directory will be copied.
     target: &'a Path,
 
+    /// Whether the target is an existing file. Cached to avoid repeated `stat` calls.
+    target_is_file: bool,
+
     /// The source path from which the directory will be copied.
     root: &'a Path,
 }
@@ -107,6 +110,7 @@ impl<'a> Context<'a> {
     fn new(root: &'a Path, target: &'a Path) -> io::Result<Self> {
         let current_dir = env::current_dir()?;
         let root_path = current_dir.join(root);
+        let target_is_file = target.is_file();
         let root_parent = if target.exists() && !root.to_str().unwrap().ends_with("/.") {
             root_path.parent().map(|p| p.to_path_buf())
         } else if root == Path::new(".") && target.is_dir() {
@@ -122,6 +126,7 @@ impl<'a> Context<'a> {
             current_dir,
             root_parent,
             target,
+            target_is_file,
             root,
         })
     }
@@ -213,7 +218,7 @@ impl Entry {
         }
 
         let local_to_target = context.target.join(descendant);
-        let target_is_file = context.target.is_file();
+        let target_is_file = context.target_is_file;
         Ok(Self {
             source_absolute,
             source_relative,
@@ -227,54 +232,73 @@ impl Entry {
 /// Copy a single entry during a directory traversal.
 fn copy_direntry(
     progress_bar: Option<&ProgressBar>,
-    entry: Entry,
+    entry: &Entry,
+    entry_is_symlink: bool,
+    entry_is_dir_no_follow: bool,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
     preserve_hard_links: bool,
     copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
+    created_parent_dirs: &mut HashSet<PathBuf>,
 ) -> CopyResult<()> {
-    let Entry {
-        source_absolute,
-        source_relative,
-        local_to_target,
-        target_is_file,
-    } = entry;
+    let source_is_symlink = entry_is_symlink;
+    let source_is_dir = if source_is_symlink && !options.dereference {
+        false
+    } else if source_is_symlink {
+        entry.source_absolute.is_dir()
+    } else {
+        entry_is_dir_no_follow
+    };
 
     // If the source is a symbolic link and the options tell us not to
     // dereference the link, then copy the link object itself.
-    if source_absolute.is_symlink() && !options.dereference {
-        return copy_link(&source_absolute, &local_to_target, symlinked_files, options);
+    if source_is_symlink && !options.dereference {
+        return copy_link(
+            &entry.source_absolute,
+            &entry.local_to_target,
+            symlinked_files,
+            options,
+        );
     }
 
     // If the source is a directory and the destination does not
     // exist, ...
-    if source_absolute.is_dir() && !local_to_target.exists() {
-        return if target_is_file {
+    if source_is_dir && !entry.local_to_target.exists() {
+        return if entry.target_is_file {
             Err(translate!("cp-error-cannot-overwrite-non-directory-with-directory").into())
         } else {
-            build_dir(&local_to_target, false, options, Some(&source_absolute))?;
+            build_dir(
+                &entry.local_to_target,
+                false,
+                options,
+                Some(&entry.source_absolute),
+            )?;
             if options.verbose {
-                println!("{}", context_for(&source_relative, &local_to_target));
+                println!(
+                    "{}",
+                    context_for(&entry.source_relative, &entry.local_to_target)
+                );
             }
             Ok(())
         };
     }
 
     // If the source is not a directory, then we need to copy the file.
-    if !source_absolute.is_dir() {
+    if !source_is_dir {
         if let Err(err) = copy_file(
             progress_bar,
-            &source_absolute,
-            local_to_target.as_path(),
+            &entry.source_absolute,
+            entry.local_to_target.as_path(),
             options,
             symlinked_files,
             copied_destinations,
             copied_files,
+            created_parent_dirs,
             false,
         ) {
             if preserve_hard_links {
-                if !source_absolute.is_symlink() {
+                if !source_is_symlink {
                     return Err(err);
                 }
                 // silent the error with a symlink
@@ -292,7 +316,10 @@ fn copy_direntry(
                         show!(uio_error!(
                             e,
                             "{}",
-                            translate!("cp-error-cannot-open-for-reading", "source" => source_relative.quote()),
+                            translate!(
+                                "cp-error-cannot-open-for-reading",
+                                "source" => entry.source_relative.quote()
+                            ),
                         ));
                     }
                     e => return Err(e),
@@ -320,6 +347,7 @@ pub(crate) fn copy_directory(
     symlinked_files: &mut HashSet<FileInformation>,
     copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
+    created_parent_dirs: &mut HashSet<PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
     // if no-dereference is enabled and this is a symlink, copy it as a file
@@ -332,6 +360,7 @@ pub(crate) fn copy_directory(
             symlinked_files,
             copied_destinations,
             copied_files,
+            created_parent_dirs,
             source_in_command_line,
         );
     }
@@ -405,16 +434,29 @@ pub(crate) fn copy_directory(
     {
         match direntry_result {
             Ok(direntry) => {
-                let entry = Entry::new(&context, direntry.path(), options.no_target_dir)?;
+                let direntry_type = direntry.file_type();
+                let direntry_path = direntry.path();
+                let (entry_is_symlink, entry_is_dir_no_follow) =
+                    match direntry_path.symlink_metadata() {
+                        Ok(metadata) => {
+                            let file_type = metadata.file_type();
+                            (file_type.is_symlink(), file_type.is_dir())
+                        }
+                        Err(_) => (direntry_type.is_symlink(), direntry_type.is_dir()),
+                    };
+                let entry = Entry::new(&context, direntry_path, options.no_target_dir)?;
 
                 copy_direntry(
                     progress_bar,
-                    entry,
+                    &entry,
+                    entry_is_symlink,
+                    entry_is_dir_no_follow,
                     options,
                     symlinked_files,
                     preserve_hard_links,
                     copied_destinations,
                     copied_files,
+                    created_parent_dirs,
                 )?;
 
                 // We omit certain permissions when creating directories
@@ -428,19 +470,17 @@ pub(crate) fn copy_directory(
                 // (Note that there can be more than one! We might step out of
                 // `./a/b/c` into `./a/`, in which case we'll need to fix the
                 // permissions of both `./a/b/c` and `./a/b`, in that order.)
-                if direntry.file_type().is_dir() {
+                let is_dir_for_permissions =
+                    entry_is_dir_no_follow || (options.dereference && direntry_path.is_dir());
+                if is_dir_for_permissions {
                     // Add this directory to our list for permission fixing later
-                    let entry_for_tracking =
-                        Entry::new(&context, direntry.path(), options.no_target_dir)?;
-                    dirs_needing_permissions.push((
-                        entry_for_tracking.source_absolute,
-                        entry_for_tracking.local_to_target,
-                    ));
+                    dirs_needing_permissions
+                        .push((entry.source_absolute.clone(), entry.local_to_target.clone()));
 
                     // If true, last_iter is not a parent of this iter.
                     // The means we just exited a directory.
                     let went_up = if let Some(last_iter) = &last_iter {
-                        last_iter.path().strip_prefix(direntry.path()).is_ok()
+                        last_iter.path().strip_prefix(direntry_path).is_ok()
                     } else {
                         false
                     };
@@ -454,14 +494,14 @@ pub(crate) fn copy_directory(
                         //
                         // All the unwraps() here are unreachable.
                         let last_iter = last_iter.as_ref().unwrap();
-                        let diff = last_iter.path().strip_prefix(direntry.path()).unwrap();
+                        let diff = last_iter.path().strip_prefix(direntry_path).unwrap();
 
                         // Fix permissions for every entry in `diff`, inside-out.
                         // We skip the last directory (which will be `.`) because
                         // its permissions will be fixed when we walk _out_ of it.
                         // (at this point, we might not be done copying `.`!)
                         for p in skip_last(diff.ancestors()) {
-                            let src = direntry.path().join(p);
+                            let src = direntry_path.join(p);
                             let entry = Entry::new(&context, &src, options.no_target_dir)?;
 
                             copy_attributes(

src/uu/cp/src/cp.rs

@@ -1335,6 +1335,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
     // remember the copied destinations for further usage.
     // we can't use copied_files as it is because the key is the source file's information.
     let mut copied_destinations: HashSet<PathBuf> = HashSet::with_capacity(sources.len());
+    let mut created_parent_dirs: HashSet<PathBuf> = HashSet::new();
 
     let progress_bar = if options.progress_bar {
         let pb = ProgressBar::new(disk_usage(sources, options.recursive)?)
@@ -1390,6 +1391,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
                 &mut symlinked_files,
                 &copied_destinations,
                 &mut copied_files,
+                &mut created_parent_dirs,
             ) {
                 show_error_if_needed(&error);
                 if !matches!(error, CpError::Skipped(false)) {
@@ -1464,6 +1466,7 @@ fn copy_source(
     symlinked_files: &mut HashSet<FileInformation>,
     copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
+    created_parent_dirs: &mut HashSet<PathBuf>,
 ) -> CopyResult<()> {
     let source_path = Path::new(&source);
     if source_path.is_dir() && (options.dereference || !source_path.is_symlink()) {
@@ -1476,6 +1479,7 @@ fn copy_source(
             symlinked_files,
             copied_destinations,
             copied_files,
+            created_parent_dirs,
             true,
         )
     } else {
@@ -1489,6 +1493,7 @@ fn copy_source(
             symlinked_files,
             copied_destinations,
             copied_files,
+            created_parent_dirs,
             true,
         );
         if options.parents {
@@ -1970,21 +1975,18 @@ fn delete_dest_if_needed_and_allowed(
     };
 
     if delete_dest {
-        fs::remove_file(dest)?;
+        match fs::remove_file(dest) {
+            Ok(()) => {}
+            Err(err) if err.kind() == io::ErrorKind::NotFound => {
+                // target could have been deleted earlier (e.g. same-file with --remove-destination)
+            }
+            Err(err) => return Err(err.into()),
+        }
     }
 
     Ok(())
 }
 
-/// Decide whether the given path exists.
-fn file_or_link_exists(path: &Path) -> bool {
-    // Using `Path.exists()` or `Path.try_exists()` is not sufficient,
-    // because if `path` is a symbolic link and there are too many
-    // levels of symbolic link, then those methods will return false
-    // or an OS error.
-    path.symlink_metadata().is_ok()
-}
-
 /// Zip the ancestors of a source path and destination path.
 ///
 /// # Examples
@@ -2082,6 +2084,7 @@ fn handle_copy_mode(
     source_in_command_line: bool,
     source_is_fifo: bool,
     source_is_socket: bool,
+    created_parent_dirs: &mut HashSet<PathBuf>,
     #[cfg(unix)] source_is_stream: bool,
 ) -> CopyResult<PerformedAction> {
     let source_is_symlink = source_metadata.is_symlink();
@@ -2123,6 +2126,7 @@ fn handle_copy_mode(
                 source_is_fifo,
                 source_is_socket,
                 symlinked_files,
+                created_parent_dirs,
                 #[cfg(unix)]
                 source_is_stream,
             )?;
@@ -2146,6 +2150,7 @@ fn handle_copy_mode(
                             source_is_fifo,
                             source_is_socket,
                             symlinked_files,
+                            created_parent_dirs,
                             #[cfg(unix)]
                             source_is_stream,
                         )?;
@@ -2182,6 +2187,7 @@ fn handle_copy_mode(
                             source_is_fifo,
                             source_is_socket,
                             symlinked_files,
+                            created_parent_dirs,
                             #[cfg(unix)]
                             source_is_stream,
                         )?;
@@ -2197,6 +2203,7 @@ fn handle_copy_mode(
                     source_is_fifo,
                     source_is_socket,
                     symlinked_files,
+                    created_parent_dirs,
                     #[cfg(unix)]
                     source_is_stream,
                 )?;
@@ -2228,16 +2235,14 @@ fn handle_copy_mode(
 // Allow unused variables for Windows (on options)
 #[allow(unused_variables)]
 fn calculate_dest_permissions(
+    dest_metadata: Option<&Metadata>,
     dest: &Path,
     source_metadata: &Metadata,
     options: &Options,
     context: &str,
 ) -> CopyResult<Permissions> {
-    if dest.exists() {
-        Ok(dest
-            .symlink_metadata()
-            .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?
-            .permissions())
+    if let Some(metadata) = dest_metadata {
+        Ok(metadata.permissions())
     } else {
         #[cfg(unix)]
         {
@@ -2277,10 +2282,16 @@ fn copy_file(
     symlinked_files: &mut HashSet<FileInformation>,
     copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
+    created_parent_dirs: &mut HashSet<PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
     let source_is_symlink = source.is_symlink();
-    let dest_is_symlink = dest.is_symlink();
+    let initial_dest_metadata = dest.symlink_metadata().ok();
+    let dest_is_symlink = initial_dest_metadata
+        .as_ref()
+        .map(|md| md.file_type().is_symlink())
+        .unwrap_or(false);
+    let dest_target_exists = dest.try_exists().unwrap_or(false);
     // Fail if dest is a dangling symlink or a symlink this program created previously
     if dest_is_symlink {
         if FileInformation::from_path(dest, false)
@@ -2302,7 +2313,7 @@ fn copy_file(
 
         let copy_contents = options.dereference(source_in_command_line) || !source_is_symlink;
         if copy_contents
-            && !dest.exists()
+            && !dest_target_exists
             && !matches!(
                 options.overwrite,
                 OverwriteMode::Clobber(ClobberMode::RemoveDestination)
@@ -2335,7 +2346,7 @@ fn copy_file(
         fs::remove_file(dest)?;
     }
 
-    if file_or_link_exists(dest)
+    if initial_dest_metadata.is_some()
         && (!options.attributes_only
             || matches!(
                 options.overwrite,
@@ -2418,7 +2429,15 @@ fn copy_file(
         })?
     };
 
-    let dest_permissions = calculate_dest_permissions(dest, &source_metadata, options, context)?;
+    let dest_metadata = dest.symlink_metadata().ok();
+
+    let dest_permissions = calculate_dest_permissions(
+        dest_metadata.as_ref(),
+        dest,
+        &source_metadata,
+        options,
+        context,
+    )?;
 
     #[cfg(unix)]
     let source_is_fifo = source_metadata.file_type().is_fifo();
@@ -2441,6 +2460,7 @@ fn copy_file(
         source_in_command_line,
         source_is_fifo,
         source_is_socket,
+        created_parent_dirs,
         #[cfg(unix)]
         source_is_stream,
     )?;
@@ -2493,7 +2513,7 @@ fn copy_file(
     );
 
     if let Some(progress_bar) = progress_bar {
-        progress_bar.inc(fs::metadata(source)?.len());
+        progress_bar.inc(source_metadata.len());
     }
 
     Ok(())
@@ -2569,11 +2589,14 @@ fn copy_helper(
     source_is_fifo: bool,
     source_is_socket: bool,
     symlinked_files: &mut HashSet<FileInformation>,
+    created_parent_dirs: &mut HashSet<PathBuf>,
     #[cfg(unix)] source_is_stream: bool,
 ) -> CopyResult<()> {
     if options.parents {
         let parent = dest.parent().unwrap_or(dest);
-        fs::create_dir_all(parent)?;
+        if created_parent_dirs.insert(parent.to_path_buf()) {
+            fs::create_dir_all(parent)?;
+        }
     }
 
     if path_ends_with_terminator(dest) && !dest.is_dir() {