tidyup: no semantically meaningful changes, subtle refactor

Author: alexcu2718

README.md

@@ -132,6 +132,8 @@ To avoid issues, use --same-file-system when traversing symlinks. Both fd and fi
 
 The following function provides an elegant solution to avoid branch mispredictions/SIMD instructions during directory entry parsing (a performance-critical loop):
 
+Check source code for further explanation [in src/utils.rs](./src/utils.rs#L195)**
+
 ```rust
 // Computational complexity: O(1) - truly constant time
 // Used on Linux/Solaris/Illumos/Android systems; BSD systems/macOS store name length trivially
@@ -149,12 +151,13 @@ pub const unsafe fn dirent_const_time_strlen(drnt: *const dirent64) -> usize {
     const DIRENT_HEADER_START: usize = std::mem::offset_of!(dirent64, d_name);
     let reclen = unsafe { (*drnt).d_reclen as usize }; 
     // Access the last 8 bytes of the word (this is an aligned read due to kernel providing 8 byte aligned dirent structs!)
-    let last_word: u64 = unsafe { *(drnt.cast::<u8>()).add(reclen - 8).cast::<u64>() };
+    let last_word: u64 = unsafe { *(drnt.byte_add(reclen - 8).cast::<u64>()) };
     // reclen is always multiple of 8 so alignment is guaranteed
     let mask = MASK * ((reclen == 24) as u64); // branchless mask
     let candidate_pos = last_word | mask; //Mask out the false nulls when d_name is short 
     //The idea is to convert each 0-byte to 0x80, and each nonzero byte to 0x00
-    let zero_bit = unsafe { // Use specialised instructions (ctl_nonzero) to avoid 0 check for bitscan forward so it compiles to tzcnt on most CPU's
+    let zero_bit = unsafe { // Use specialised instructions (ctl_nonzero) 
+        //to avoid 0 check for bitscan forward so it compiles to tzcnt on most CPU's
             NonZeroU64::new_unchecked(candidate_pos.wrapping_sub(LO_U64) & !candidate_pos & HI_U64)
         };
 

benches/dirent_bench.rs

@@ -1,16 +1,8 @@
 use criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main};
 
+use core::num::NonZeroU64;
 use std::hint::black_box;
 
-#[inline]
-pub(crate) const fn repeat_u64(byte: u8) -> u64 {
-    u64::from_ne_bytes([byte; size_of::<u64>()])
-}
-
-const LO_U64: u64 = repeat_u64(0x01);
-
-const HI_U64: u64 = repeat_u64(0x80);
-
 #[inline]
 //modified version to work for this test function(copy pasted really)
 pub const unsafe fn dirent_const_time_strlen(dirent: *const LibcDirent64) -> usize {
@@ -19,7 +11,8 @@ pub const unsafe fn dirent_const_time_strlen(dirent: *const LibcDirent64) -> usi
     // Access the last field and then round up to find the minimum struct size
     const MINIMUM_DIRENT_SIZE: usize = DIRENT_HEADER_START.next_multiple_of(8);
 
-    use core::num::NonZeroU64;
+    const LO_U64: u64 = u64::from_ne_bytes([0x01; size_of::<u64>()]);
+    const HI_U64: u64 = u64::from_ne_bytes([0x80; size_of::<u64>()]);
 
     /*  Accessing `d_reclen` is safe because the struct is kernel-provided.
     / SAFETY: `dirent` is valid by precondition */
@@ -29,20 +22,13 @@ pub const unsafe fn dirent_const_time_strlen(dirent: *const LibcDirent64) -> usi
       Read the last 8 bytes of the struct as a u64.
     This works because dirents are always 8-byte aligned. */
     // SAFETY: We're indexing in bounds within the pointer (it is guaranteed aligned by the kernel)
-    let last_word: u64 = unsafe { *(dirent.cast::<u8>()).add(reclen - 8).cast::<u64>() };
+    let last_word: u64 = unsafe { *(dirent.byte_add(reclen - 8).cast::<u64>()) };
     /* Note, I don't index as a u64 with eg (reclen-8)/8 or (reclen-8)>>3 because that adds a division which is a costly operation, relatively speaking
     let last_word: u64 = unsafe { *(dirent.cast::<u64>()).add((reclen - 8)/8 (or >>3))}; //this will also work but it's less performant (MINUTELY)
     */
 
-    #[cfg(target_endian = "little")]
-    const MASK: u64 = 0x00FF_FFFFu64;
-    #[cfg(target_endian = "big")]
-    const MASK: u64 = 0xFFFF_FF00_0000_0000u64; // byte order is shifted unintuitively on big endian!
+    const MASK: u64 = u64::from_ne_bytes([0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00]);
 
-    /* When the record length is 24/`MINIMUM_DIRENT_SIZE`, the kernel may insert nulls before d_name.
-    Which will exist on index's 17/18 (or opposite, for big endian...sigh.,)
-    Mask them out to avoid false detection of a terminator.
-    Multiplying by 0 or 1 applies the mask conditionally without branching. */
     let mask: u64 = MASK * ((reclen == MINIMUM_DIRENT_SIZE) as u64);
     /*
      Apply the mask to ignore non-name bytes while preserving name bytes.
@@ -53,21 +39,12 @@ pub const unsafe fn dirent_const_time_strlen(dirent: *const LibcDirent64) -> usi
     */
     let candidate_pos: u64 = last_word | mask;
 
-    /*
-     Locate the first null byte in constant time using SWAR.
-     Subtract  the position of the index of the 0 then add 1 to compute its position relative to the start of d_name.
-
-     SAFETY: The u64 can never be all 0's post-SWAR, therefore we can make a niche optimisation
-     https://doc.rust-lang.org/std/num/struct.NonZero.html#tymethod.trailing_zeros
-     https://doc.rust-lang.org/std/num/struct.NonZero.html#tymethod.leading_zeros
-    (using ctlz_nonzero instruction which is superior to ctlz but can't handle all 0 numbers)
-    */
     let zero_bit = unsafe {
         NonZeroU64::new_unchecked(candidate_pos.wrapping_sub(LO_U64) & !candidate_pos & HI_U64)
     };
     #[cfg(target_endian = "little")]
     let byte_pos = 8 - (zero_bit.trailing_zeros() >> 3) as usize;
-    #[cfg(not(target_endian = "little"))]
+    #[cfg(target_endian = "big")]
     let byte_pos = 8 - (zero_bit.leading_zeros() >> 3) as usize;
 
     /*  Final length:

src/config.rs

@@ -2,7 +2,7 @@ use crate::BytePath as _;
 use crate::cli_helpers::SizeFilter;
 use crate::glob_to_regex;
 use crate::{DirEntry, FileType, SearchConfigError};
-use core::num::NonZeroUsize;
+use core::num::NonZeroU32;
 use regex::bytes::{Regex, RegexBuilder};
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 /// File type filter for directory traversal
@@ -152,7 +152,7 @@ pub struct SearchConfig {
     ///if this is Some, then we match against the extension of the file otherwise accept (if none)
     pub(crate) file_name_only: bool,
     ///if true, then we only match against the file name, otherwise we match against the full path when regexing
-    pub(crate) depth: Option<NonZeroUsize>,
+    pub(crate) depth: Option<NonZeroU32>,
     ///the maximum depth to search, if None then no limit
     pub(crate) follow_symlinks: bool, //if true, then we follow symlinks, otherwise we do not follow them
     /// a size filter
@@ -176,7 +176,7 @@ impl SearchConfig {
         keep_dirs: bool,
         filenameonly: bool,
         extension_match: Option<Box<[u8]>>,
-        depth: Option<NonZeroUsize>,
+        depth: Option<NonZeroU32>,
         follow_symlinks: bool,
         size_filter: Option<SizeFilter>,
         type_filter: Option<FileTypeFilter>,

src/finderbuilder.rs

@@ -1,4 +1,4 @@
-use core::num::NonZeroUsize;
+use core::num::NonZeroU32;
 
 use crate::{
     DirEntry, DirEntryFilter, FileTypeFilter, FilterType, Finder, SearchConfig, SearchConfigError,
@@ -10,8 +10,7 @@ use std::{
     ffi::{OsStr, OsString},
     fs::metadata,
     os::unix::fs::MetadataExt as _,
-    path::Path,
-    path::PathBuf,
+    path::{Path, PathBuf},
 };
 
 //Set the threadcount at compile time (backing to a minimum of 1, **this should never happen**)
@@ -34,7 +33,7 @@ pub struct FinderBuilder {
     pub(crate) keep_dirs: bool,
     pub(crate) file_name_only: bool,
     pub(crate) extension_match: Option<Box<[u8]>>,
-    pub(crate) max_depth: Option<NonZeroUsize>,
+    pub(crate) max_depth: Option<NonZeroU32>,
     pub(crate) follow_symlinks: bool,
     pub(crate) filter: Option<DirEntryFilter>,
     pub(crate) size_filter: Option<SizeFilter>,
@@ -120,11 +119,11 @@ impl FinderBuilder {
     }
     #[must_use]
     /// Set maximum search depth
-    pub const fn max_depth(mut self, max_depth: Option<usize>) -> Self {
+    pub const fn max_depth(mut self, max_depth: Option<u32>) -> Self {
         match max_depth {
             None => self,
             Some(num) => {
-                self.max_depth = core::num::NonZeroUsize::new(num);
+                self.max_depth = core::num::NonZeroU32::new(num);
                 self
             }
         }
@@ -264,9 +263,7 @@ impl FinderBuilder {
             }
         };
 
-        let inode_cache: Option<DashSet<(u64, u64)>> =
-            (self.same_filesystem || self.follow_symlinks).then(DashSet::new);
-        //Enable the cache if same file system too, this helps de-duplicate for free (since it's 1 stat call regardless)
+        let inode_cache: Option<DashSet<(u64, u64)>> = self.follow_symlinks.then(DashSet::new);
 
         Ok(Finder {
             root: resolved_root,

src/iter.rs

@@ -1,7 +1,5 @@
 #![allow(clippy::must_use_candidate)]
 use crate::FileType;
-#[cfg(any(target_os = "linux", target_os = "macos", target_os = "android"))]
-use crate::types::SyscallBuffer;
 use crate::{DirEntry, FileDes, Result};
 use crate::{dirent64, readdir64};
 use core::cell::Cell;
@@ -120,7 +118,7 @@ pub struct GetDents {
     pub(crate) fd: FileDes,
     /// Kernel buffer for batch reading directory entries via system call I/O
     /// Approximately 4.1KB in size, optimised for typical directory traversal
-    pub(crate) syscall_buffer: SyscallBuffer,
+    pub(crate) syscall_buffer: crate::types::SyscallBuffer,
     /// buffer for constructing full entry paths
     /// Reused for each entry to avoid repeated memory allocation (only constructed once per dir)
     pub(crate) path_buffer: Vec<u8>,
@@ -291,7 +289,7 @@ impl GetDents {
 
     #[inline]
     /// Provides read only access to the internal buffer that holds the bytes read from the syscall
-    pub const fn borrow_syscall_buffer(&self) -> &SyscallBuffer {
+    pub const fn borrow_syscall_buffer(&self) -> &crate::types::SyscallBuffer {
         &self.syscall_buffer
     }
 
@@ -301,7 +299,7 @@ impl GetDents {
         debug_assert!(fd.is_open(), "We expect it to always be open");
 
         let (path_buffer, path_len) = Self::init_from_direntry(dir);
-        let buffer = SyscallBuffer::new();
+        let buffer = crate::types::SyscallBuffer::new();
         Ok(Self {
             fd,
             syscall_buffer: buffer,
@@ -602,12 +600,6 @@ macro_rules! impl_iter {
     };
 }
 
-impl_iter!(ReadDir);
-#[cfg(any(target_os = "linux", target_os = "android"))]
-impl_iter!(GetDents);
-#[cfg(target_os = "macos")]
-impl_iter!(GetDirEntries);
-
 // simple repetition avoider
 macro_rules! impl_dirent_constructor {
     ($type:ty) => {
@@ -652,19 +644,24 @@ macro_rules! impl_iterator_for_dirent {
     };
 }
 
+// Common to all platforms
+impl_iter!(ReadDir);
 impl_iterator_for_dirent!(ReadDir);
+impl_dirent_constructor!(ReadDir);
 
+// Linux/Android specific
+#[cfg(any(target_os = "linux", target_os = "android"))]
+impl_iter!(GetDents);
 #[cfg(any(target_os = "linux", target_os = "android"))]
 impl_iterator_for_dirent!(GetDents);
-
-#[cfg(target_os = "macos")]
-impl_iterator_for_dirent!(GetDirEntries);
-
-impl_dirent_constructor!(ReadDir);
-
 #[cfg(any(target_os = "linux", target_os = "android"))]
 impl_dirent_constructor!(GetDents);
 
+// macOS specific
+#[cfg(target_os = "macos")]
+impl_iter!(GetDirEntries);
+#[cfg(target_os = "macos")]
+impl_iterator_for_dirent!(GetDirEntries);
 #[cfg(target_os = "macos")]
 impl_dirent_constructor!(GetDirEntries);
 
@@ -684,7 +681,7 @@ pub struct GetDirEntries {
     pub(crate) fd: FileDes,
     /// Kernel buffer for batch reading directory entries via system call I/O
     /// ~32kb in size (8*4096, matching macos readdir semantics) in size, optimised for typical directory traversal
-    pub(crate) syscall_buffer: SyscallBuffer,
+    pub(crate) syscall_buffer: crate::types::SyscallBuffer,
     /// buffer for constructing full entry paths
     /// Reused for each entry to avoid repeated memory allocation (only constructed once per dir)
     pub(crate) path_buffer: Vec<u8>,
@@ -782,7 +779,7 @@ impl GetDirEntries {
         debug_assert!(fd.is_open(), "We expect it to always be open");
 
         let (path_buffer, path_len) = Self::init_from_direntry(dir);
-        let buffer = SyscallBuffer::new();
+        let buffer = crate::types::SyscallBuffer::new();
         Ok(Self {
             fd,
             syscall_buffer: buffer,