From 973963111299d024f3f792b1572f5e41a6e5df5a Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 00:08:42 +0800 Subject: [PATCH 01/26] feat(path): add interned UstrPath type backed by ustr --- Cargo.lock | 23 ++++++ Cargo.toml | 4 + deny.toml | 3 +- src/lib.rs | 2 + src/ustr_path.rs | 205 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 src/ustr_path.rs diff --git a/Cargo.lock b/Cargo.lock index 649c7d0e..6aef94cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -861,6 +861,16 @@ version = "11.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + [[package]] name = "parking_lot_core" version = "0.9.12" @@ -1051,6 +1061,7 @@ dependencies = [ "thiserror", "tokio", "tracing", + "ustr-fxhash", "vfs", ] @@ -1326,6 +1337,18 @@ version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f6ccf251212114b54433ec949fd6a7841275f9ada20dddd2f29e9ceea4501493" +[[package]] +name = "ustr-fxhash" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4d2224cabd887261556e0d3a483a7e9f87dcbaab3b820df7d748305a8d5cbc8" +dependencies = [ + "byteorder", + "lazy_static", + "parking_lot", + "rustc-hash", +] + [[package]] name = "value-trait" version = "0.12.1" diff --git a/Cargo.toml b/Cargo.toml index f3fd2a17..a6fb06c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,6 +91,10 @@ indexmap = { version = "2.14.0", features = ["serde"] } json-strip-comments = "3.1.1" rustc-hash = { version = "2.1.2", default-features = false, features = ["std"] } thiserror = "2.0.18" +# Global string interner. MUST stay byte-identical to rspack's workspace +# declaration — a version split creates a second interner static and silently +# destroys the "one copy per path" guarantee that `UstrPath` exists for. +ustr = { package = "ustr-fxhash", version = "1.0.1", default-features = false } pnp = { version = "0.12.9", optional = true } diff --git a/deny.toml b/deny.toml index 1c4e9974..6170711a 100644 --- a/deny.toml +++ b/deny.toml @@ -92,7 +92,8 @@ allow = [ "MIT", "Apache-2.0", "BSD-2-Clause", - "Unicode-3.0", + "BSD-2-Clause-Patent", + "Unicode-3.0", "Zlib" #"Apache-2.0 WITH LLVM-exception", ] diff --git a/src/lib.rs b/src/lib.rs index 76e188f9..979b3cd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,6 +60,7 @@ mod resolution; mod resolver_path; mod specifier; mod tsconfig; +mod ustr_path; #[cfg(test)] mod tests; @@ -97,6 +98,7 @@ pub use crate::{ package_json::{JSONValue, ModuleType, PackageJson}, resolution::Resolution, resolver_path::ResolverPath, + ustr_path::{IdentityHasher, UstrPath, UstrPathSet}, }; type ResolveResult = Result, ResolveError>; diff --git a/src/ustr_path.rs b/src/ustr_path.rs new file mode 100644 index 00000000..ff07ee33 --- /dev/null +++ b/src/ustr_path.rs @@ -0,0 +1,205 @@ +use std::{ + collections::HashSet, + fmt, + hash::{BuildHasherDefault, Hash, Hasher}, + ops::Deref, + path::Path, +}; + +use camino::Utf8Path; +use ustr::Ustr; + +/// A globally interned UTF-8 path. +/// +/// 8 bytes, `Copy`, and `'static`: every distinct path string exists exactly +/// once process-wide, so the same path handed to many consumers costs one +/// pointer each instead of one heap allocation each. +/// +/// Equality degenerates to a pointer comparison (interning guarantees one +/// pointer per string) and hashing to a single `u64` load from the interner +/// entry header, so `UstrPathSet` lookups cost one `write_u64`. +/// +/// The interner is shared with rspack — both crates depend on the same +/// `ustr-fxhash` version, hence the same static — so a path interned here is +/// already interned for rspack. +/// +/// # Lifetime +/// +/// Interned strings are **never freed**. See `CLAUDE_USTR_PATH_DESIGN.md` §4.3. +#[derive(Clone, Copy, PartialEq, Eq)] +#[repr(transparent)] +pub struct UstrPath(Ustr); + +impl UstrPath { + /// Intern `path` and return a handle to it. + #[inline] + pub fn new(path: &str) -> Self { + Self(Ustr::from(path)) + } + + #[inline] + pub fn as_str(&self) -> &'static str { + self.0.as_str() + } + + #[inline] + pub fn as_utf8_path(&self) -> &'static Utf8Path { + Utf8Path::new(self.0.as_str()) + } + + #[inline] + pub fn as_std_path(&self) -> &'static Path { + Path::new(self.0.as_str()) + } + + /// The `FxHash` of the path bytes, precomputed by the interner. + /// + /// Reading it is a single load from the entry header (`char_ptr - 16`). + #[inline] + pub fn precomputed_hash(&self) -> u64 { + self.0.precomputed_hash() + } +} + +impl Hash for UstrPath { + #[inline] + fn hash(&self, state: &mut H) { + state.write_u64(self.0.precomputed_hash()); + } +} + +impl Deref for UstrPath { + type Target = Utf8Path; + + #[inline] + fn deref(&self) -> &Self::Target { + self.as_utf8_path() + } +} + +impl AsRef for UstrPath { + #[inline] + fn as_ref(&self) -> &Utf8Path { + self.as_utf8_path() + } +} + +impl AsRef for UstrPath { + #[inline] + fn as_ref(&self) -> &Path { + self.as_std_path() + } +} + +impl AsRef for UstrPath { + #[inline] + fn as_ref(&self) -> &str { + self.as_str() + } +} + +impl fmt::Debug for UstrPath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_utf8_path().fmt(f) + } +} + +impl fmt::Display for UstrPath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +/// A `HashSet` keyed by the interner's precomputed hash. +/// +/// Uses `ustr::IdentityHasher` rather than the private `IdentityHasher` in +/// `crate::cache` so rspack's `ArcPathSet` is the same concrete type and can +/// take these sets by `mem::take` instead of re-bucketing them. +pub type UstrPathSet = HashSet>; + +/// Re-exported so downstream crates can spell the same concrete set type +/// without taking a direct `ustr` dependency (and without risking a version +/// split — see the note on the `ustr` dependency in `Cargo.toml`). +pub use ustr::IdentityHasher; + +#[cfg(test)] +mod tests { + use std::{ + collections::HashSet, + hash::{Hash, Hasher}, + }; + + use camino::Utf8Path; + + use super::*; + + #[test] + fn same_string_is_same_pointer() { + let a = UstrPath::new("/a/b/c.js"); + let b = UstrPath::new("/a/b/c.js"); + assert_eq!(a.as_str().as_ptr(), b.as_str().as_ptr()); + assert_eq!(a, b); + } + + #[test] + fn different_strings_are_not_equal() { + assert_ne!(UstrPath::new("/a/b"), UstrPath::new("/a/c")); + } + + #[test] + fn hash_is_the_precomputed_hash() { + let p = UstrPath::new("/x/y"); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + p.hash(&mut hasher); + // The value written into the hasher is the precomputed one, not a + // re-hash of the bytes. + let mut expected = std::collections::hash_map::DefaultHasher::new(); + expected.write_u64(p.precomputed_hash()); + assert_eq!(hasher.finish(), expected.finish()); + } + + #[test] + fn works_in_an_identity_hashed_set() { + let mut set: UstrPathSet = HashSet::default(); + set.insert(UstrPath::new("/a/b")); + assert!(set.contains(&UstrPath::new("/a/b"))); + assert!(!set.contains(&UstrPath::new("/a/c"))); + } + + #[test] + fn derefs_to_utf8_path() { + let p = UstrPath::new("/a/b/c.js"); + assert_eq!(p.file_name(), Some("c.js")); + assert_eq!(p.parent(), Some(Utf8Path::new("/a/b"))); + assert_eq!(p.extension(), Some("js")); + assert_eq!(p.join("d.ts"), Utf8Path::new("/a/b/c.js/d.ts")); + } + + #[test] + fn debug_prints_the_path_not_the_ustr_wrapper() { + let p = UstrPath::new("/a/b"); + assert_eq!(format!("{p:?}"), "\"/a/b\""); + assert_eq!(format!("{p}"), "/a/b"); + } + + #[test] + fn as_ref_targets_compile_and_agree() { + let p = UstrPath::new("/a/b"); + let as_utf8: &Utf8Path = p.as_ref(); + let as_std: &std::path::Path = p.as_ref(); + let as_str: &str = p.as_ref(); + assert_eq!(as_utf8.as_str(), as_str); + assert_eq!(as_std, std::path::Path::new("/a/b")); + } + + #[test] + fn identity_hasher_receives_eight_bytes() { + // `ustr::IdentityHasher::write` silently produces a 0 hash unless it gets + // exactly 8 bytes. `UstrPath::hash` goes through the default `write_u64`, + // which forwards `u64::to_ne_bytes()` — exactly 8. Guard that invariant so + // a future change to `Hash` cannot silently collapse every key to bucket 0. + let mut hasher = ustr::IdentityHasher::default(); + UstrPath::new("/some/long/path/that/is/not/eight/bytes").hash(&mut hasher); + assert_ne!(hasher.finish(), 0); + } +} From 9b8aafa4f7c95aaaa58a46646883ddc53cea70a0 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 00:28:08 +0800 Subject: [PATCH 02/26] fix(path): rename similar bindings in UstrPath test to satisfy clippy pedantic under --all-targets --- src/ustr_path.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ustr_path.rs b/src/ustr_path.rs index ff07ee33..fa46e249 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -186,10 +186,10 @@ mod tests { fn as_ref_targets_compile_and_agree() { let p = UstrPath::new("/a/b"); let as_utf8: &Utf8Path = p.as_ref(); - let as_std: &std::path::Path = p.as_ref(); - let as_str: &str = p.as_ref(); - assert_eq!(as_utf8.as_str(), as_str); - assert_eq!(as_std, std::path::Path::new("/a/b")); + let std_path: &std::path::Path = p.as_ref(); + let str_ref: &str = p.as_ref(); + assert_eq!(as_utf8.as_str(), str_ref); + assert_eq!(std_path, std::path::Path::new("/a/b")); } #[test] From 64600553f55ca469d6e319931fcb00e1a39d8e85 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 00:37:24 +0800 Subject: [PATCH 03/26] feat(path): normalize windows path spellings at intern time --- src/ustr_path.rs | 158 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/src/ustr_path.rs b/src/ustr_path.rs index fa46e249..cdcc8ad4 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -30,10 +30,94 @@ use ustr::Ustr; #[repr(transparent)] pub struct UstrPath(Ustr); +#[inline] +const fn is_sep(c: u8) -> bool { + c == b'/' || c == b'\\' +} + +/// Rewrite `s` into canonical Windows form: `\` separators, no repeated +/// separators, no trailing separator (roots excepted). +/// +/// Returns `None` when `s` is already canonical — the common case — so the +/// caller interns the original `&str` without allocating. +/// +/// Paths beginning with two separators (UNC and verbatim, e.g. `\\?\C:\x` or +/// `\\server\share`) pass through unchanged: std's `Path` disables separator +/// normalization behind a verbatim prefix, and the `dunce` dependency already +/// keeps the resolver off UNC paths. +/// +/// Platform-independent on purpose. camino's `Utf8Path::components()` picks its +/// separator set at compile time, so it cannot express Windows semantics on a +/// unix host — this would otherwise be untestable off Windows. +// Referenced by `UstrPath::new` only on Windows, but compiled and tested +// everywhere so the normalization rules stay verifiable on a unix host. Also +// excluded under `cfg(test)`: the test module below calls it directly, so on +// a non-Windows test build it is not actually dead and the expectation would +// go unfulfilled. +#[cfg_attr( + not(any(windows, test)), + expect(dead_code, reason = "windows-only caller; tested on all platforms") +)] +fn normalize_windows_separators(s: &str) -> Option { + let bytes = s.as_bytes(); + + if bytes.len() >= 2 && is_sep(bytes[0]) && is_sep(bytes[1]) { + return None; + } + + let mut needs_rewrite = false; + let mut prev_was_sep = false; + for (i, &c) in bytes.iter().enumerate() { + if is_sep(c) { + // A trailing separator only counts as canonical when it is the root + // itself (`\`) or a drive root (`C:\`). + if c == b'/' || prev_was_sep || (i + 1 == bytes.len() && i > 0 && bytes[i - 1] != b':') { + needs_rewrite = true; + break; + } + prev_was_sep = true; + } else { + prev_was_sep = false; + } + } + if !needs_rewrite { + return None; + } + + let mut out = String::with_capacity(s.len()); + let mut prev_was_sep = false; + for ch in s.chars() { + if ch == '/' || ch == '\\' { + if !prev_was_sep { + out.push('\\'); + } + prev_was_sep = true; + } else { + out.push(ch); + prev_was_sep = false; + } + } + + if out.len() > 1 && out.ends_with('\\') && !out.ends_with(":\\") { + out.pop(); + } + + Some(out) +} + impl UstrPath { /// Intern `path` and return a handle to it. + /// + /// On Windows the path is first rewritten into canonical form so that every + /// spelling of the same path (`C:/a/b`, `C:\a\b`, `C:\a\\b`, `C:\a\b\`) + /// interns to one pointer — preserving the dedup semantics `Path`'s + /// component-wise `Hash`/`Eq` used to provide. #[inline] pub fn new(path: &str) -> Self { + #[cfg(windows)] + if let Some(normalized) = normalize_windows_separators(path) { + return Self(Ustr::from(&normalized)); + } Self(Ustr::from(path)) } @@ -202,4 +286,78 @@ mod tests { UstrPath::new("/some/long/path/that/is/not/eight/bytes").hash(&mut hasher); assert_ne!(hasher.finish(), 0); } + + #[test] + fn already_canonical_windows_paths_need_no_rewrite() { + assert_eq!(normalize_windows_separators(r"C:\a\b"), None); + assert_eq!(normalize_windows_separators(r"C:\"), None); + assert_eq!(normalize_windows_separators(r"\"), None); + assert_eq!(normalize_windows_separators("a"), None); + } + + #[test] + fn forward_slashes_become_backslashes() { + assert_eq!( + normalize_windows_separators("C:/a/b").as_deref(), + Some(r"C:\a\b") + ); + assert_eq!( + normalize_windows_separators(r"C:/a\b").as_deref(), + Some(r"C:\a\b") + ); + } + + #[test] + fn repeated_separators_collapse() { + assert_eq!( + normalize_windows_separators(r"C:\a\\b").as_deref(), + Some(r"C:\a\b") + ); + assert_eq!( + normalize_windows_separators("C://a//b").as_deref(), + Some(r"C:\a\b") + ); + } + + #[test] + fn trailing_separator_is_dropped_but_roots_survive() { + assert_eq!( + normalize_windows_separators(r"C:\a\b\").as_deref(), + Some(r"C:\a\b") + ); + assert_eq!(normalize_windows_separators("C:/").as_deref(), Some(r"C:\")); + assert_eq!(normalize_windows_separators(r"C:\"), None); + } + + #[test] + fn unc_and_verbatim_paths_pass_through_untouched() { + // std disables separator normalization for verbatim prefixes, and `dunce` + // already keeps the resolver off UNC paths — leaving both alone is both + // correct and the conservative choice. + assert_eq!(normalize_windows_separators(r"\\?\C:\a/b"), None); + assert_eq!(normalize_windows_separators("//?/C:/a/b"), None); + assert_eq!(normalize_windows_separators(r"\\server\share\a"), None); + } + + #[test] + fn non_ascii_segments_survive_normalization() { + assert_eq!( + normalize_windows_separators("C:/项目/源码").as_deref(), + Some(r"C:\项目\源码") + ); + } + + #[cfg(windows)] + #[test] + fn equivalent_windows_spellings_intern_to_one_pointer() { + let canonical = UstrPath::new(r"C:\a\b"); + for spelling in [r"C:\a\b", "C:/a/b", r"C:/a\b", r"C:\a\\b", r"C:\a\b\"] { + let p = UstrPath::new(spelling); + assert_eq!( + p.as_str().as_ptr(), + canonical.as_str().as_ptr(), + "spelling {spelling:?} should intern to the canonical pointer" + ); + } + } } From 2cba2e10915ed83a20910c5787ab9a19cfe09ee6 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 00:53:56 +0800 Subject: [PATCH 04/26] fix(path): fold windows drive letter case at intern time std's Path hash/eq folds the drive letter to uppercase (Prefix::Disk), so c:\a\b and C:\a\b are one dependency today. Byte-wise interning without this fold would split that into two paths, defeating the dedup this task exists to preserve. --- src/ustr_path.rs | 56 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/src/ustr_path.rs b/src/ustr_path.rs index cdcc8ad4..bda63aae 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -36,15 +36,20 @@ const fn is_sep(c: u8) -> bool { } /// Rewrite `s` into canonical Windows form: `\` separators, no repeated -/// separators, no trailing separator (roots excepted). +/// separators, no trailing separator (roots excepted), and an uppercase +/// drive letter — matching the folding std's `Path` applies via +/// `Prefix::Disk` before comparing paths in `Hash`/`Eq`. /// /// Returns `None` when `s` is already canonical — the common case — so the /// caller interns the original `&str` without allocating. /// /// Paths beginning with two separators (UNC and verbatim, e.g. `\\?\C:\x` or -/// `\\server\share`) pass through unchanged: std's `Path` disables separator -/// normalization behind a verbatim prefix, and the `dunce` dependency already -/// keeps the resolver off UNC paths. +/// `\\server\share`) pass through untouched, including the verbatim disk +/// prefix's drive-letter case: std folds that case too (`VerbatimDisk` goes +/// through the same `parse_drive`), so leaving it alone here is a known, +/// accepted divergence from `Path`'s `Hash`/`Eq` for verbatim paths — the +/// `dunce` dependency already keeps the resolver off UNC/verbatim paths, so +/// this code path isn't reached in practice. /// /// Platform-independent on purpose. camino's `Utf8Path::components()` picks its /// separator set at compile time, so it cannot express Windows semantics on a @@ -65,7 +70,10 @@ fn normalize_windows_separators(s: &str) -> Option { return None; } - let mut needs_rewrite = false; + // A lowercase drive letter (`c:\...`) is not canonical on its own — std + // folds it to uppercase (`Prefix::Disk`) before comparing paths — even + // when every separator is already fine. + let mut needs_rewrite = bytes.len() >= 2 && bytes[0].is_ascii_lowercase() && bytes[1] == b':'; let mut prev_was_sep = false; for (i, &c) in bytes.iter().enumerate() { if is_sep(c) { @@ -84,14 +92,18 @@ fn normalize_windows_separators(s: &str) -> Option { return None; } + let fold_drive = bytes.len() >= 2 && bytes[1] == b':'; let mut out = String::with_capacity(s.len()); let mut prev_was_sep = false; - for ch in s.chars() { + for (i, ch) in s.chars().enumerate() { if ch == '/' || ch == '\\' { if !prev_was_sep { out.push('\\'); } prev_was_sep = true; + } else if i == 0 && fold_drive && ch.is_ascii_lowercase() { + out.push(ch.to_ascii_uppercase()); + prev_was_sep = false; } else { out.push(ch); prev_was_sep = false; @@ -347,11 +359,41 @@ mod tests { ); } + #[test] + fn lowercase_drive_letter_is_folded_to_uppercase() { + assert_eq!( + normalize_windows_separators(r"c:\a\b").as_deref(), + Some(r"C:\a\b") + ); + assert_eq!( + normalize_windows_separators("c:/a/b").as_deref(), + Some(r"C:\a\b") + ); + assert_eq!( + normalize_windows_separators(r"c:\").as_deref(), + Some(r"C:\") + ); + // Already uppercase and otherwise canonical: still no rewrite. + assert_eq!(normalize_windows_separators(r"C:\a\b"), None); + } + + #[test] + fn case_is_folded_only_on_the_drive_letter() { + assert_eq!( + normalize_windows_separators(r"c:\Foo\BAR.ts").as_deref(), + Some(r"C:\Foo\BAR.ts") + ); + // No drive letter at all — nothing to fold. + assert_eq!(normalize_windows_separators(r"relative\Path"), None); + } + #[cfg(windows)] #[test] fn equivalent_windows_spellings_intern_to_one_pointer() { let canonical = UstrPath::new(r"C:\a\b"); - for spelling in [r"C:\a\b", "C:/a/b", r"C:/a\b", r"C:\a\\b", r"C:\a\b\"] { + for spelling in [ + r"C:\a\b", "C:/a/b", r"C:/a\b", r"C:\a\\b", r"C:\a\b\", "c:/a/b", r"c:\a\b", + ] { let p = UstrPath::new(spelling); assert_eq!( p.as_str().as_ptr(), From bd61675728eecd585614b6e6d7502fd088f2d53b Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 01:17:15 +0800 Subject: [PATCH 05/26] feat(path): add ToUstrPath conversion trait --- src/lib.rs | 2 +- src/ustr_path.rs | 119 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 979b3cd0..dd69401e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -98,7 +98,7 @@ pub use crate::{ package_json::{JSONValue, ModuleType, PackageJson}, resolution::Resolution, resolver_path::ResolverPath, - ustr_path::{IdentityHasher, UstrPath, UstrPathSet}, + ustr_path::{IdentityHasher, ToUstrPath, UstrPath, UstrPathSet}, }; type ResolveResult = Result, ResolveError>; diff --git a/src/ustr_path.rs b/src/ustr_path.rs index bda63aae..aac14202 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -3,10 +3,10 @@ use std::{ fmt, hash::{BuildHasherDefault, Hash, Hasher}, ops::Deref, - path::Path, + path::{Path, PathBuf}, }; -use camino::Utf8Path; +use camino::{Utf8Path, Utf8PathBuf}; use ustr::Ustr; /// A globally interned UTF-8 path. @@ -218,6 +218,85 @@ pub type UstrPathSet = HashSet Utf8Path` boundary (see the +/// `expect("path should be UTF-8")` calls in `lib.rs` and `cache.rs`). +pub trait ToUstrPath { + fn to_ustr_path(&self) -> UstrPath; +} + +impl ToUstrPath for str { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + UstrPath::new(self) + } +} + +impl ToUstrPath for String { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + UstrPath::new(self) + } +} + +impl ToUstrPath for Utf8Path { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + UstrPath::new(self.as_str()) + } +} + +impl ToUstrPath for Utf8PathBuf { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + UstrPath::new(self.as_str()) + } +} + +impl ToUstrPath for Path { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + UstrPath::new(self.to_str().expect("path should be UTF-8")) + } +} + +impl ToUstrPath for PathBuf { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + self.as_path().to_ustr_path() + } +} + +impl ToUstrPath for UstrPath { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + *self + } +} + +impl From<&T> for UstrPath { + #[inline] + fn from(value: &T) -> Self { + value.to_ustr_path() + } +} + +impl From for UstrPath { + #[inline] + fn from(value: Utf8PathBuf) -> Self { + value.to_ustr_path() + } +} + +impl From for UstrPath { + #[inline] + fn from(value: PathBuf) -> Self { + value.to_ustr_path() + } +} + #[cfg(test)] mod tests { use std::{ @@ -402,4 +481,40 @@ mod tests { ); } } + + #[test] + fn to_ustr_path_accepts_every_path_flavor() { + use camino::Utf8PathBuf; + + let expected = UstrPath::new("/a/b"); + assert_eq!("/a/b".to_ustr_path(), expected); + assert_eq!(String::from("/a/b").to_ustr_path(), expected); + assert_eq!(Utf8Path::new("/a/b").to_ustr_path(), expected); + assert_eq!(Utf8PathBuf::from("/a/b").to_ustr_path(), expected); + assert_eq!(std::path::Path::new("/a/b").to_ustr_path(), expected); + assert_eq!(std::path::PathBuf::from("/a/b").to_ustr_path(), expected); + assert_eq!(expected.to_ustr_path(), expected); + } + + #[test] + fn from_impls_match_to_ustr_path() { + use camino::Utf8PathBuf; + + let expected = UstrPath::new("/a/b"); + assert_eq!(UstrPath::from("/a/b"), expected); + assert_eq!(UstrPath::from(Utf8Path::new("/a/b")), expected); + assert_eq!(UstrPath::from(Utf8PathBuf::from("/a/b")), expected); + assert_eq!(UstrPath::from(std::path::Path::new("/a/b")), expected); + assert_eq!(UstrPath::from(std::path::PathBuf::from("/a/b")), expected); + } + + #[test] + #[should_panic(expected = "path should be UTF-8")] + #[cfg(unix)] + fn non_utf8_std_path_panics_like_the_rest_of_the_resolver() { + use std::{ffi::OsStr, os::unix::ffi::OsStrExt}; + + let bad = std::path::Path::new(OsStr::from_bytes(b"/a/\xff/b")); + let _ = bad.to_ustr_path(); + } } From 908cb324fac861e675831cb2cbe6e89ddc903cfb Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 01:42:09 +0800 Subject: [PATCH 06/26] refactor(path): replace ResolverPath with interned UstrPath Swap ResolveContext's file/missing dependency collections, all add_file_dependency/add_missing_dependency call sites, and CachedPathImpl's output channel from the Arc-backed ResolverPath to the interned UstrPath. ResolverPath and its hash_path helper are gone; the CachedPath DashSet's hash helper moves into cache.rs as the private hash_utf8_path, unrelated to interning. This is a pure type swap with no memoization yet (that's next): every add_*_dependency call does a naive to_ustr_path() intern lookup. --- examples/resolver.rs | 4 +- src/cache.rs | 56 +++--- src/context.rs | 28 +-- src/lib.rs | 12 +- src/resolver_path.rs | 216 ----------------------- src/tests/alias.rs | 16 +- src/tests/dependencies.rs | 12 +- src/tests/extensions.rs | 20 +-- src/tests/incorrect_description_file.rs | 11 +- src/tests/missing.rs | 24 ++- src/tests/pnp.rs | 4 +- src/tests/symlink.rs | 4 +- src/tests/tsconfig_project_references.rs | 8 +- 13 files changed, 114 insertions(+), 301 deletions(-) delete mode 100644 src/resolver_path.rs diff --git a/examples/resolver.rs b/examples/resolver.rs index 21684121..ad9b5e2f 100644 --- a/examples/resolver.rs +++ b/examples/resolver.rs @@ -40,10 +40,10 @@ async fn main() { }; let mut sorted_file_deps = ctx.file_dependencies.iter().collect::>(); - sorted_file_deps.sort_by_key(|p| p.as_path()); + sorted_file_deps.sort_by_key(|p| p.as_std_path()); println!("file_deps: {:#?}", sorted_file_deps); let mut sorted_missing = ctx.missing_dependencies.iter().collect::>(); - sorted_missing.sort_by_key(|p| p.as_path()); + sorted_missing.sort_by_key(|p| p.as_std_path()); println!("missing_deps: {:#?}", sorted_missing); } diff --git a/src/cache.rs b/src/cache.rs index 14acda2b..5cc5db06 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -18,7 +18,7 @@ use tokio::sync::OnceCell as OnceLock; use crate::{ context::ResolveContext as Ctx, package_json::{off_to_location, PackageJson}, - resolver_path::{hash_path, ResolverPath}, + ustr_path::{ToUstrPath, UstrPath}, FileMetadata, FileSystem, JSONError, ResolveError, ResolveOptions, TsConfig, }; @@ -44,7 +44,7 @@ impl Cache { } pub fn value(&self, path: &Utf8Path) -> CachedPath { - let hash = hash_path(path.as_std_path()); + let hash = hash_utf8_path(path); if let Some(cache_entry) = self.paths.get((hash, path).borrow() as &dyn CacheKey) { return cache_entry.clone(); } @@ -161,18 +161,16 @@ pub struct CachedPathImpl { canonicalized: OnceLock>, node_modules: OnceLock>, package_json: OnceLock>>, - /// Memoized `/package.json` `ResolverPath` for the - /// `missing_dependencies` push that fires on every `package_json` cache-hit - /// `None` (~97% of `package_json` calls in dep-tracking workloads). - package_json_dep_path: std::sync::OnceLock, + /// Memoized `/package.json` for the `missing_dependencies` push + /// that fires on every `package_json` cache-hit `None` (~97% of + /// `package_json` calls in dep-tracking workloads). + package_json_dep_path: std::sync::OnceLock, } -impl From<&CachedPathImpl> for ResolverPath { - /// Reuse the cache-side `FxHash` (already computed in `Cache::value`); the - /// only remaining work is one `Arc::from(&Path)` to materialize the shared - /// path buffer for the `ResolveContext` sink. - fn from(cached: &CachedPathImpl) -> Self { - Self::from_parts(cached.hash, Arc::from(cached.path.as_std_path())) +impl ToUstrPath for CachedPathImpl { + #[inline] + fn to_ustr_path(&self) -> UstrPath { + self.path.to_ustr_path() } } @@ -192,11 +190,10 @@ impl CachedPathImpl { /// Without this cache, each `None` cache-hit on `package_json` would /// re-`join` + re-allocate the `Arc` + re-hash on every push. - fn package_json_dep_path(&self) -> ResolverPath { - self + fn package_json_dep_path(&self) -> UstrPath { + *self .package_json_dep_path - .get_or_init(|| self.path.join("package.json").into()) - .clone() + .get_or_init(|| self.path.join("package.json").to_ustr_path()) } pub fn path(&self) -> &Utf8Path { @@ -225,10 +222,10 @@ impl CachedPathImpl { pub async fn is_file(&self, fs: &Fs, ctx: &mut Ctx) -> bool { if let Some(meta) = self.meta(fs).await { - ctx.add_file_dependency(self); + ctx.add_file_dependency(self.to_ustr_path()); meta.is_file } else { - ctx.add_missing_dependency(self); + ctx.add_missing_dependency(self.to_ustr_path()); false } } @@ -236,7 +233,7 @@ impl CachedPathImpl { pub async fn is_dir(&self, fs: &Fs, ctx: &mut Ctx) -> bool { self.meta(fs).await.map_or_else( || { - ctx.add_missing_dependency(self); + ctx.add_missing_dependency(self.to_ustr_path()); false }, |meta| meta.is_dir, @@ -371,7 +368,7 @@ impl CachedPathImpl { if let Some(pkg) = self.package_json.get() { // Preserve ctx dependency tracking on cache hit. match pkg { - Some(package_json) => ctx.add_file_dependency(&package_json.path), + Some(package_json) => ctx.add_file_dependency(package_json.path.to_ustr_path()), None => { if ctx.missing_dependencies.is_some() { ctx.add_missing_dependency(self.package_json_dep_path()); @@ -436,7 +433,7 @@ impl CachedPathImpl { // https://github.com/webpack/enhanced-resolve/blob/58464fc7cb56673c9aa849e68e6300239601e615/lib/DescriptionFileUtils.js#L68-L82 match &result { Ok(Some(package_json)) => { - ctx.add_file_dependency(&package_json.path); + ctx.add_file_dependency(package_json.path.to_ustr_path()); } Ok(None) => { // Avoid an allocation by making this lazy @@ -503,3 +500,20 @@ impl Hasher for IdentityHasher { self.0 } } + +/// Hash a path for the `CachedPath` `DashSet` key. +/// +/// Bulk-writes the raw bytes on unix — the std `Path::hash` impl walks +/// components (utf8 split + per-segment write), which is materially more +/// expensive on this, the hottest lookup in the crate. On other platforms it +/// goes through `Path` so `pack1/foo` and `pack1\foo` stay one entry, matching +/// `CachedPath`'s `PartialEq`. +#[inline] +fn hash_utf8_path(path: &Utf8Path) -> u64 { + let mut hasher = FxHasher::default(); + #[cfg(unix)] + hasher.write(path.as_str().as_bytes()); + #[cfg(not(unix))] + path.as_std_path().hash(&mut hasher); + hasher.finish() +} diff --git a/src/context.rs b/src/context.rs index 24b38196..37bc025c 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,6 +1,9 @@ use std::ops::{Deref, DerefMut}; -use crate::{error::ResolveError, resolver_path::ResolverPath}; +use crate::{ + error::ResolveError, + ustr_path::{ToUstrPath, UstrPath}, +}; #[derive(Debug, Default, Clone)] pub struct ResolveContext(ResolveContextImpl); @@ -14,10 +17,10 @@ pub struct ResolveContextImpl { pub fragment: Option, /// Files that were found on file system - pub file_dependencies: Option>, + pub file_dependencies: Option>, /// Files that were not found on file system - pub missing_dependencies: Option>, + pub missing_dependencies: Option>, /// The current resolving alias for bailing recursion alias. pub resolving_alias: Option, @@ -59,19 +62,22 @@ impl ResolveContext { self.missing_dependencies.replace(vec![]); } - // Accepts anything convertible to `ResolverPath`. The conversion (which - // includes the `Arc` allocation for `&Path` / `PathBuf` callers, or - // hash reuse for `&CachedPathImpl`) only runs inside the `Some` branch, so - // `resolve()` calls without a context still pay zero. - pub fn add_file_dependency>(&mut self, dep: P) { + // Accepts anything path-shaped. The interning only runs inside the `Some` + // branch, so `resolve()` calls without a context still pay zero. + // `ToUstrPath::to_ustr_path` only ever borrows `dep`, so clippy sees the + // by-value `P` as needless; kept by value anyway so callers can pass owned + // or borrowed path types through the same generic call site. + #[allow(clippy::needless_pass_by_value)] + pub fn add_file_dependency(&mut self, dep: P) { if let Some(deps) = &mut self.file_dependencies { - deps.push(dep.into()); + deps.push(dep.to_ustr_path()); } } - pub fn add_missing_dependency>(&mut self, dep: P) { + #[allow(clippy::needless_pass_by_value)] + pub fn add_missing_dependency(&mut self, dep: P) { if let Some(deps) = &mut self.missing_dependencies { - deps.push(dep.into()); + deps.push(dep.to_ustr_path()); } } diff --git a/src/lib.rs b/src/lib.rs index dd69401e..4e168df7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,7 +57,6 @@ mod options; mod package_json; mod path; mod resolution; -mod resolver_path; mod specifier; mod tsconfig; mod ustr_path; @@ -97,7 +96,6 @@ pub use crate::{ }, package_json::{JSONValue, ModuleType, PackageJson}, resolution::Resolution, - resolver_path::ResolverPath, ustr_path::{IdentityHasher, ToUstrPath, UstrPath, UstrPathSet}, }; @@ -107,10 +105,10 @@ type ResolveResult = Result, ResolveError>; #[derive(Debug, Default, Clone)] pub struct ResolveContext { /// Files that were found on file system - pub file_dependencies: FxHashSet, + pub file_dependencies: UstrPathSet, /// Dependencies that were not found on file system - pub missing_dependencies: FxHashSet, + pub missing_dependencies: UstrPathSet, } /// Resolver with the current operating system as the file system @@ -758,7 +756,7 @@ impl ResolverGeneric { .realpath(&self.cache.fs) .await .map(|path| { - ctx.add_file_dependency(path.as_path()); + ctx.add_file_dependency(path.to_ustr_path()); path }) .map_err(ResolveError::from) @@ -1059,7 +1057,7 @@ impl ResolverGeneric { return Ok(None); }; let (manifest_path, manifest) = pnp_data.as_ref(); - ctx.add_file_dependency(manifest_path); + ctx.add_file_dependency(manifest_path.to_ustr_path()); let mut path = cached_path.to_path_buf(); path.push(""); @@ -1502,7 +1500,7 @@ impl ResolverGeneric { ) .await?; for dependency in &tsconfig.file_dependencies { - ctx.add_file_dependency(dependency.as_path()); + ctx.add_file_dependency(dependency.to_ustr_path()); } let paths = tsconfig.resolve(cached_path.path(), specifier); for path in paths { diff --git a/src/resolver_path.rs b/src/resolver_path.rs deleted file mode 100644 index 45e3883f..00000000 --- a/src/resolver_path.rs +++ /dev/null @@ -1,216 +0,0 @@ -#[cfg(unix)] -use std::os::unix::ffi::OsStrExt; -use std::{ - fmt, - hash::{Hash, Hasher}, - ops::Deref, - path::{Path, PathBuf}, - sync::Arc, -}; - -use camino::{Utf8Path, Utf8PathBuf}; -use rustc_hash::FxHasher; - -/// A path returned in [`crate::ResolveContext`] dependencies, paired with a -/// precomputed `FxHash` of the path bytes. -/// -/// Downstream consumers (rspack) place these into hash collections keyed by -/// the precomputed hash, avoiding repeated hashing of long absolute paths on -/// every insert and lookup. -/// -/// Hash and equality are kept aligned per platform so the standard -/// `a == b ⇒ hash(a) == hash(b)` contract holds: -/// - On Unix, both hash and equality compare the raw `OsStr` bytes (fast bulk -/// `write`; matches the resolver's cache-side `Cache::value` hash). -/// - On other platforms, both go through `Path` (component-walked hash and -/// normalized equality), so e.g. `pack1/foo` and `pack1\\foo` are treated as -/// the same dependency on Windows — same behavior `PathBuf` had before. -#[derive(Clone)] -pub struct ResolverPath { - hash: u64, - path: Arc, -} - -impl ResolverPath { - pub fn new(path: Arc) -> Self { - let hash = hash_path(&path); - Self { hash, path } - } - - /// Construct without recomputing the hash. - /// - /// # Precondition - /// `hash` MUST equal `hash_path(path)`. Violating this breaks `HashSet`'s - /// bucketing invariant — entries become unfindable and deduplication stops - /// working. Not `unsafe` because the failure mode is a logic bug rather - /// than UB. - #[inline] - pub(crate) fn from_parts(hash: u64, path: Arc) -> Self { - Self { hash, path } - } - - #[inline] - pub fn as_path(&self) -> &Path { - &self.path - } - - #[inline] - pub fn as_arc(&self) -> &Arc { - &self.path - } - - #[inline] - pub fn into_arc(self) -> Arc { - self.path - } - - /// The precomputed `FxHash` of the path bytes. - #[inline] - pub fn precomputed_hash(&self) -> u64 { - self.hash - } -} - -/// Hash a path with `FxHasher`, matching the bytes-on-unix optimization used by -/// the resolver's internal cache so [`ResolverPath`] values constructed from a -/// `CachedPath` produce the same `u64` as values constructed from a `&Path`. -#[inline] -pub fn hash_path(path: &Path) -> u64 { - let mut hasher = FxHasher::default(); - // The std `Path::hash` impl walks components (utf8 split + per-segment - // write); a single bulk `write` of the raw bytes is materially cheaper on - // the resolver hot path. - #[cfg(unix)] - hasher.write(path.as_os_str().as_bytes()); - #[cfg(not(unix))] - path.hash(&mut hasher); - hasher.finish() -} - -impl Hash for ResolverPath { - #[inline] - fn hash(&self, state: &mut H) { - state.write_u64(self.hash); - } -} - -impl PartialEq for ResolverPath { - /// Mirror `hash_path`'s per-platform scheme so the `a == b ⇒ hash(a) == - /// hash(b)` invariant holds: raw `OsStr` bytes on Unix (matches the - /// bulk-byte hash), component-normalized `Path::eq` elsewhere (matches - /// `Path::hash`). - fn eq(&self, other: &Self) -> bool { - #[cfg(unix)] - { - self.path.as_os_str() == other.path.as_os_str() - } - #[cfg(not(unix))] - { - self.path == other.path - } - } -} - -impl Eq for ResolverPath {} - -impl Deref for ResolverPath { - type Target = Path; - - fn deref(&self) -> &Self::Target { - &self.path - } -} - -impl AsRef for ResolverPath { - fn as_ref(&self) -> &Path { - &self.path - } -} - -impl From for ResolverPath { - fn from(path: PathBuf) -> Self { - Self::new(Arc::from(path)) - } -} - -impl From<&Path> for ResolverPath { - fn from(path: &Path) -> Self { - Self::new(Arc::from(path)) - } -} - -impl From<&PathBuf> for ResolverPath { - fn from(path: &PathBuf) -> Self { - Self::new(Arc::from(path.as_path())) - } -} - -impl From> for ResolverPath { - fn from(path: Arc) -> Self { - Self::new(path) - } -} - -// The resolver stores paths internally as UTF-8 (`camino`); these accept the -// internal representation directly while keeping the stored buffer an -// `Arc` so the `as_arc`/`into_arc` output contract stays unchanged. -impl From for ResolverPath { - fn from(path: Utf8PathBuf) -> Self { - Self::new(Arc::from(path.into_std_path_buf())) - } -} - -impl From<&Utf8Path> for ResolverPath { - fn from(path: &Utf8Path) -> Self { - Self::new(Arc::from(path.as_std_path())) - } -} - -impl fmt::Debug for ResolverPath { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.path.fmt(f) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn hash_is_path_byte_hash() { - let p: &Path = Path::new("/a/b/c.js"); - let rp = ResolverPath::from(p); - assert_eq!(rp.precomputed_hash(), hash_path(p)); - } - - #[test] - fn equal_paths_have_equal_hashes() { - let a = ResolverPath::from(PathBuf::from("/x/y")); - let b = ResolverPath::from(Path::new("/x/y")); - assert_eq!(a, b); - assert_eq!(a.precomputed_hash(), b.precomputed_hash()); - } - - #[test] - fn writes_u64_into_hasher() { - use std::{collections::HashSet, hash::BuildHasherDefault}; - - #[derive(Default)] - struct IdHasher(u64); - impl Hasher for IdHasher { - fn write(&mut self, _: &[u8]) { - unreachable!() - } - fn write_u64(&mut self, n: u64) { - self.0 = n; - } - fn finish(&self) -> u64 { - self.0 - } - } - - let mut set: HashSet> = HashSet::default(); - set.insert(ResolverPath::from(Path::new("/a/b"))); - assert!(set.contains(&ResolverPath::from(PathBuf::from("/a/b")))); - } -} diff --git a/src/tests/alias.rs b/src/tests/alias.rs index d42922a1..947e145b 100644 --- a/src/tests/alias.rs +++ b/src/tests/alias.rs @@ -237,13 +237,21 @@ async fn alias_is_full_path() { } for path in ctx.file_dependencies { - assert_eq!(path.as_path(), path.normalize(), "{path:?}"); - check_slash(&path); + assert_eq!( + path.as_std_path(), + path.as_std_path().normalize(), + "{path:?}" + ); + check_slash(path.as_std_path()); } for path in ctx.missing_dependencies { - assert_eq!(path.as_path(), path.normalize(), "{path:?}"); - check_slash(&path); + assert_eq!( + path.as_std_path(), + path.as_std_path().normalize(), + "{path:?}" + ); + check_slash(path.as_std_path()); if let Some(path) = path.parent() { assert!(!path.is_file(), "{path:?} must not be a file"); } diff --git a/src/tests/dependencies.rs b/src/tests/dependencies.rs index be9fe04f..46a00216 100644 --- a/src/tests/dependencies.rs +++ b/src/tests/dependencies.rs @@ -59,10 +59,8 @@ mod warm_cache_missing_dependencies { mod windows { use std::path::PathBuf; - use rustc_hash::FxHashSet; - use super::super::memory_fs::MemoryFS; - use crate::{ResolveContext, ResolveOptions, ResolverGeneric, ResolverPath}; + use crate::{ResolveContext, ResolveOptions, ResolverGeneric, UstrPath, UstrPathSet}; fn file_system() -> MemoryFS { MemoryFS::new(&[ @@ -159,13 +157,13 @@ mod windows { .await .map(|r| r.full_path()); assert_eq!(resolved, Ok(PathBuf::from(result))); - let file_dependencies: FxHashSet = file_dependencies + let file_dependencies: UstrPathSet = file_dependencies .iter() - .map(|p| ResolverPath::from(PathBuf::from(p))) + .map(|p| UstrPath::from(PathBuf::from(p))) .collect(); - let missing_dependencies: FxHashSet = missing_dependencies + let missing_dependencies: UstrPathSet = missing_dependencies .iter() - .map(|p| ResolverPath::from(PathBuf::from(p))) + .map(|p| UstrPath::from(PathBuf::from(p))) .collect(); assert_eq!(ctx.file_dependencies, file_dependencies, "{name}"); assert_eq!(ctx.missing_dependencies, missing_dependencies, "{name}"); diff --git a/src/tests/extensions.rs b/src/tests/extensions.rs index 66e971ed..b98df3b4 100644 --- a/src/tests/extensions.rs +++ b/src/tests/extensions.rs @@ -1,10 +1,8 @@ //! -use rustc_hash::FxHashSet; - use crate::{ - EnforceExtension, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, - ResolverPath, + EnforceExtension, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, UstrPath, + UstrPathSet, }; #[tokio::test] @@ -67,9 +65,9 @@ async fn default_enforce_extension() { ); assert_eq!( ctx.file_dependencies, - FxHashSet::from_iter([ - ResolverPath::from(f.join("foo.ts")), - ResolverPath::from(f.join("package.json")), + UstrPathSet::from_iter([ + UstrPath::from(f.join("foo.ts")), + UstrPath::from(f.join("package.json")), ]) ); assert!(ctx.missing_dependencies.is_empty()); @@ -95,14 +93,14 @@ async fn respect_enforce_extension() { ); assert_eq!( ctx.file_dependencies, - FxHashSet::from_iter([ - ResolverPath::from(f.join("foo.ts")), - ResolverPath::from(f.join("package.json")), + UstrPathSet::from_iter([ + UstrPath::from(f.join("foo.ts")), + UstrPath::from(f.join("package.json")), ]) ); assert_eq!( ctx.missing_dependencies, - FxHashSet::from_iter([ResolverPath::from(f.join("foo"))]) + UstrPathSet::from_iter([UstrPath::from(f.join("foo"))]) ); } diff --git a/src/tests/incorrect_description_file.rs b/src/tests/incorrect_description_file.rs index a21ae525..070567b4 100644 --- a/src/tests/incorrect_description_file.rs +++ b/src/tests/incorrect_description_file.rs @@ -1,9 +1,8 @@ //! -use rustc_hash::FxHashSet; - use crate::{ - JSONError, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, ResolverPath, + JSONError, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, UstrPath, + UstrPathSet, }; // should not resolve main in incorrect description file #1 @@ -25,9 +24,9 @@ async fn incorrect_description_file_1() { assert!(matches!(resolution, Err(ResolveError::JSON(_)))); assert_eq!( ctx.file_dependencies, - FxHashSet::from_iter([ - ResolverPath::from(f.join("pack1")), - ResolverPath::from(f.join("pack1/package.json")), + UstrPathSet::from_iter([ + UstrPath::from(f.join("pack1")), + UstrPath::from(f.join("pack1/package.json")), ]) ); assert!(!ctx.missing_dependencies.is_empty()); diff --git a/src/tests/missing.rs b/src/tests/missing.rs index c41e644e..e664c6d8 100644 --- a/src/tests/missing.rs +++ b/src/tests/missing.rs @@ -2,7 +2,7 @@ use normalize_path::NormalizePath; -use crate::{AliasValue, ResolveContext, ResolveOptions, Resolver, ResolverPath}; +use crate::{AliasValue, ResolveContext, ResolveOptions, Resolver, UstrPath}; #[tokio::test] async fn test() { @@ -59,15 +59,17 @@ async fn test() { let _ = resolver.resolve_with_context(&f, specifier, &mut ctx).await; for path in ctx.file_dependencies { - assert_eq!(path.as_path(), path.normalize(), "{path:?}"); + assert_eq!( + path.as_std_path(), + path.as_std_path().normalize(), + "{path:?}" + ); } for path in missing_dependencies { assert_eq!(path, path.normalize(), "{path:?}"); assert!( - ctx - .missing_dependencies - .contains(&ResolverPath::from(&path)), + ctx.missing_dependencies.contains(&UstrPath::from(&path)), "{specifier}: {path:?} not in {:?}", &ctx.missing_dependencies ); @@ -110,11 +112,19 @@ async fn alias_and_extensions() { let _ = resolver.resolve_with_context(&f, "react-dom/client", &mut ctx); for path in ctx.file_dependencies { - assert_eq!(path.as_path(), path.normalize(), "{path:?}"); + assert_eq!( + path.as_std_path(), + path.as_std_path().normalize(), + "{path:?}" + ); } for path in ctx.missing_dependencies { - assert_eq!(path.as_path(), path.normalize(), "{path:?}"); + assert_eq!( + path.as_std_path(), + path.as_std_path().normalize(), + "{path:?}" + ); if let Some(path) = path.parent() { assert!(!path.is_file(), "{path:?} must not be a file"); } diff --git a/src/tests/pnp.rs b/src/tests/pnp.rs index 3d55ee16..ed659145 100644 --- a/src/tests/pnp.rs +++ b/src/tests/pnp.rs @@ -6,7 +6,7 @@ use camino::Utf8Path; use crate::{ - path::PathUtil, ResolveContext, ResolveError::NotFound, ResolveOptions, Resolver, ResolverPath, + path::PathUtil, ResolveContext, ResolveError::NotFound, ResolveOptions, Resolver, UstrPath, }; #[tokio::test] @@ -100,7 +100,7 @@ async fn pnp_file_dependencies() { assert!( ctx .file_dependencies - .contains(&ResolverPath::from(fixture.join(".pnp.cjs"))), + .contains(&UstrPath::from(fixture.join(".pnp.cjs"))), ".pnp.cjs should be in file_dependencies, got: {:?}", ctx.file_dependencies ); diff --git a/src/tests/symlink.rs b/src/tests/symlink.rs index 949c3677..82210b0e 100644 --- a/src/tests/symlink.rs +++ b/src/tests/symlink.rs @@ -1,6 +1,6 @@ use std::{fs, io, path::Path}; -use crate::{ResolveOptions, Resolver, ResolverPath}; +use crate::{ResolveOptions, Resolver, UstrPath}; #[derive(Debug, Clone, Copy)] enum FileType { @@ -150,7 +150,7 @@ async fn test() -> io::Result<()> { assert!( ctx .file_dependencies - .contains(&ResolverPath::from(resolved_path.unwrap())), + .contains(&UstrPath::from(resolved_path.unwrap())), "file dependencies should contain resolved path {comment:?}" ); } diff --git a/src/tests/tsconfig_project_references.rs b/src/tests/tsconfig_project_references.rs index 6ea71e73..b977c85a 100644 --- a/src/tests/tsconfig_project_references.rs +++ b/src/tests/tsconfig_project_references.rs @@ -1,8 +1,8 @@ //! Tests for tsconfig project references use crate::{ - ResolveContext, ResolveError, ResolveOptions, Resolver, ResolverPath, TsconfigOptions, - TsconfigReferences, + ResolveContext, ResolveError, ResolveOptions, Resolver, TsconfigOptions, TsconfigReferences, + UstrPath, }; #[tokio::test] @@ -75,9 +75,7 @@ async fn tsconfig_file_as_file_dependencies() { ]; for dependency in expected_dependencies { assert!( - ctx - .file_dependencies - .contains(&ResolverPath::from(&dependency)), + ctx.file_dependencies.contains(&UstrPath::from(&dependency)), "missing tsconfig file dependency {dependency:?}: {:?}", ctx.file_dependencies ); From 86a39c03be10da906e52b9ed04d8ceae99d821eb Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 01:59:31 +0800 Subject: [PATCH 07/26] fix(path): take dependency by reference to preserve zero-cost no-context path add_file_dependency/add_missing_dependency took P by value, so ToUstrPath::to_ustr_path (and the Ustr::from intern it does under missing_dependencies/file_dependencies == None) ran unconditionally at every call site before the `if let Some(deps)` guard could skip it. Interning takes a global sharded lock and permanently allocates on miss, so this silently added lock contention and unbounded allocation to the default resolve() path, which never populates a context. Switch both methods to (&mut self, dep: &P) so the interning stays inside the Some branch, and revert the 8 call sites this affected back to passing references. Drop the needless_pass_by_value allows this shadowed. Also add a direct test for hash_utf8_path (cache.rs), which lost its only coverage when resolver_path.rs was deleted. --- src/cache.rs | 40 +++++++++++++++++++++++++++++++--------- src/context.rs | 17 ++++++++--------- src/lib.rs | 6 +++--- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 5cc5db06..b0c6a204 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -222,10 +222,10 @@ impl CachedPathImpl { pub async fn is_file(&self, fs: &Fs, ctx: &mut Ctx) -> bool { if let Some(meta) = self.meta(fs).await { - ctx.add_file_dependency(self.to_ustr_path()); + ctx.add_file_dependency(self); meta.is_file } else { - ctx.add_missing_dependency(self.to_ustr_path()); + ctx.add_missing_dependency(self); false } } @@ -233,7 +233,7 @@ impl CachedPathImpl { pub async fn is_dir(&self, fs: &Fs, ctx: &mut Ctx) -> bool { self.meta(fs).await.map_or_else( || { - ctx.add_missing_dependency(self.to_ustr_path()); + ctx.add_missing_dependency(self); false }, |meta| meta.is_dir, @@ -311,7 +311,7 @@ impl CachedPathImpl { // Replay ctx tracking from the cold path: module_directory -> is_dir calls // ctx.add_missing_dependency when node_modules doesn't exist on disk. if nm.is_none() { - ctx.add_missing_dependency(self.path.join("node_modules")); + ctx.add_missing_dependency(&self.path.join("node_modules")); } return nm.clone(); } @@ -368,10 +368,10 @@ impl CachedPathImpl { if let Some(pkg) = self.package_json.get() { // Preserve ctx dependency tracking on cache hit. match pkg { - Some(package_json) => ctx.add_file_dependency(package_json.path.to_ustr_path()), + Some(package_json) => ctx.add_file_dependency(&package_json.path), None => { if ctx.missing_dependencies.is_some() { - ctx.add_missing_dependency(self.package_json_dep_path()); + ctx.add_missing_dependency(&self.package_json_dep_path()); } } } @@ -433,17 +433,17 @@ impl CachedPathImpl { // https://github.com/webpack/enhanced-resolve/blob/58464fc7cb56673c9aa849e68e6300239601e615/lib/DescriptionFileUtils.js#L68-L82 match &result { Ok(Some(package_json)) => { - ctx.add_file_dependency(package_json.path.to_ustr_path()); + ctx.add_file_dependency(&package_json.path); } Ok(None) => { // Avoid an allocation by making this lazy if ctx.missing_dependencies.is_some() { - ctx.add_missing_dependency(self.package_json_dep_path()); + ctx.add_missing_dependency(&self.package_json_dep_path()); } } Err(_) => { if ctx.file_dependencies.is_some() { - ctx.add_file_dependency(self.path.join("package.json")); + ctx.add_file_dependency(&self.path.join("package.json")); } } } @@ -517,3 +517,25 @@ fn hash_utf8_path(path: &Utf8Path) -> u64 { path.as_std_path().hash(&mut hasher); hasher.finish() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hash_utf8_path_matches_the_platform_contract() { + let path = Utf8Path::new("/a/b/c.js"); + let mut expected = FxHasher::default(); + #[cfg(unix)] + expected.write(path.as_str().as_bytes()); + #[cfg(not(unix))] + path.as_std_path().hash(&mut expected); + assert_eq!(hash_utf8_path(path), expected.finish()); + + // Equal paths hash equally — the invariant `CachedPath`'s DashSet depends on. + assert_eq!( + hash_utf8_path(Utf8Path::new("/x/y")), + hash_utf8_path(&Utf8PathBuf::from("/x/y")) + ); + } +} diff --git a/src/context.rs b/src/context.rs index 37bc025c..eab9d215 100644 --- a/src/context.rs +++ b/src/context.rs @@ -62,20 +62,19 @@ impl ResolveContext { self.missing_dependencies.replace(vec![]); } - // Accepts anything path-shaped. The interning only runs inside the `Some` - // branch, so `resolve()` calls without a context still pay zero. - // `ToUstrPath::to_ustr_path` only ever borrows `dep`, so clippy sees the - // by-value `P` as needless; kept by value anyway so callers can pass owned - // or borrowed path types through the same generic call site. - #[allow(clippy::needless_pass_by_value)] - pub fn add_file_dependency(&mut self, dep: P) { + // Accepts anything path-shaped, by reference. Taking `&P` (not `P`) matters: + // interning is a global-lock + hashtable probe with a permanent allocation on + // miss, so it must only happen inside the `Some` branch — a by-value `dep` + // would be evaluated (and interned) before the `if let` ever runs, silently + // paying that cost on every call even when `resolve()` was invoked without a + // context and these fields are `None`. + pub fn add_file_dependency(&mut self, dep: &P) { if let Some(deps) = &mut self.file_dependencies { deps.push(dep.to_ustr_path()); } } - #[allow(clippy::needless_pass_by_value)] - pub fn add_missing_dependency(&mut self, dep: P) { + pub fn add_missing_dependency(&mut self, dep: &P) { if let Some(deps) = &mut self.missing_dependencies { deps.push(dep.to_ustr_path()); } diff --git a/src/lib.rs b/src/lib.rs index 4e168df7..19b2e727 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -756,7 +756,7 @@ impl ResolverGeneric { .realpath(&self.cache.fs) .await .map(|path| { - ctx.add_file_dependency(path.to_ustr_path()); + ctx.add_file_dependency(path.as_path()); path }) .map_err(ResolveError::from) @@ -1057,7 +1057,7 @@ impl ResolverGeneric { return Ok(None); }; let (manifest_path, manifest) = pnp_data.as_ref(); - ctx.add_file_dependency(manifest_path.to_ustr_path()); + ctx.add_file_dependency(manifest_path); let mut path = cached_path.to_path_buf(); path.push(""); @@ -1500,7 +1500,7 @@ impl ResolverGeneric { ) .await?; for dependency in &tsconfig.file_dependencies { - ctx.add_file_dependency(dependency.to_ustr_path()); + ctx.add_file_dependency(dependency.as_path()); } let paths = tsconfig.resolve(cached_path.path(), specifier); for path in paths { From 1554722be1165d4d7e70c5d43704a193f5190ca5 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 02:15:48 +0800 Subject: [PATCH 08/26] perf(cache): memoize interned dependency paths on CachedPathImpl Repeated is_file/is_dir dependency pushes for the same CachedPathImpl re-entered the global ustr interner (FxHash + sharded mutex + probe) on every call. Memoize the interned path, the node_modules dependency path, and route the package_json cache-miss error branch through the already-memoized package_json path, so repeats degrade to a Copy. The node_modules_dep_path call site (cached_node_modules's cache-hit replay) previously had no dependency-tracking guard around it, since it built a plain Utf8PathBuf that only got interned inside the guarded add_missing_dependency body. Memoizing moves the intern call to the call site itself, so it now needs its own ctx.missing_dependencies.is_some() guard to keep interning gated behind active dependency tracking. --- src/cache.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index b0c6a204..a5ae7204 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -161,6 +161,13 @@ pub struct CachedPathImpl { canonicalized: OnceLock>, node_modules: OnceLock>, package_json: OnceLock>>, + /// Memoized interned form of `self.path`. Without it every `is_file` / + /// `is_dir` dependency push would re-enter the interner (bin lock + probe). + dep_path: std::sync::OnceLock, + /// Memoized `/node_modules`. `cached_node_modules` replays a + /// missing-dependency push on every cache hit where `node_modules` is + /// absent, which is most directory levels during an upward walk. + node_modules_dep_path: std::sync::OnceLock, /// Memoized `/package.json` for the `missing_dependencies` push /// that fires on every `package_json` cache-hit `None` (~97% of /// `package_json` calls in dep-tracking workloads). @@ -170,7 +177,7 @@ pub struct CachedPathImpl { impl ToUstrPath for CachedPathImpl { #[inline] fn to_ustr_path(&self) -> UstrPath { - self.path.to_ustr_path() + self.dep_path() } } @@ -184,10 +191,22 @@ impl CachedPathImpl { canonicalized: OnceLock::new(), node_modules: OnceLock::new(), package_json: OnceLock::new(), + dep_path: std::sync::OnceLock::new(), + node_modules_dep_path: std::sync::OnceLock::new(), package_json_dep_path: std::sync::OnceLock::new(), } } + fn dep_path(&self) -> UstrPath { + *self.dep_path.get_or_init(|| self.path.to_ustr_path()) + } + + fn node_modules_dep_path(&self) -> UstrPath { + *self + .node_modules_dep_path + .get_or_init(|| self.path.join("node_modules").to_ustr_path()) + } + /// Without this cache, each `None` cache-hit on `package_json` would /// re-`join` + re-allocate the `Arc` + re-hash on every push. fn package_json_dep_path(&self) -> UstrPath { @@ -310,8 +329,8 @@ impl CachedPathImpl { if let Some(nm) = self.node_modules.get() { // Replay ctx tracking from the cold path: module_directory -> is_dir calls // ctx.add_missing_dependency when node_modules doesn't exist on disk. - if nm.is_none() { - ctx.add_missing_dependency(&self.path.join("node_modules")); + if nm.is_none() && ctx.missing_dependencies.is_some() { + ctx.add_missing_dependency(&self.node_modules_dep_path()); } return nm.clone(); } @@ -443,7 +462,7 @@ impl CachedPathImpl { } Err(_) => { if ctx.file_dependencies.is_some() { - ctx.add_file_dependency(&self.path.join("package.json")); + ctx.add_file_dependency(&self.package_json_dep_path()); } } } @@ -521,6 +540,35 @@ fn hash_utf8_path(path: &Utf8Path) -> u64 { #[cfg(test)] mod tests { use super::*; + use crate::FileSystemOs; + + #[tokio::test] + async fn repeated_dep_conversions_reuse_one_memoized_handle() { + let cache = Cache::new(FileSystemOs::default()); + let cached = cache.value(Utf8Path::new("/a/b/c.js")); + + let first = cached.to_ustr_path(); + let second = cached.to_ustr_path(); + assert_eq!(first.as_str().as_ptr(), second.as_str().as_ptr()); + // Memoized: the second call must not have gone through the interner at all. + assert!(cached.dep_path.get().is_some()); + } + + #[tokio::test] + async fn node_modules_and_package_json_dep_paths_are_memoized() { + let cache = Cache::new(FileSystemOs::default()); + let cached = cache.value(Utf8Path::new("/a/b")); + + assert_eq!(cached.node_modules_dep_path().as_str(), "/a/b/node_modules"); + assert_eq!(cached.package_json_dep_path().as_str(), "/a/b/package.json"); + + assert!(cached.node_modules_dep_path.get().is_some()); + assert!(cached.package_json_dep_path.get().is_some()); + assert_eq!( + cached.node_modules_dep_path().as_str().as_ptr(), + cached.node_modules_dep_path().as_str().as_ptr() + ); + } #[test] fn hash_utf8_path_matches_the_platform_contract() { From 41fde7290b86bd7ed1ca153360d96d3eb5240fbc Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 02:31:29 +0800 Subject: [PATCH 09/26] refactor(package-json): store path and realpath as UstrPath --- src/cache.rs | 12 +++++----- src/lib.rs | 13 +++++------ src/package_json/simd.rs | 17 +++++++++------ src/tests/package_json.rs | 46 +++++++++++++++++++++++++++++---------- src/tests/pnp.rs | 7 +----- src/ustr_path.rs | 14 ++++++++++++ 6 files changed, 71 insertions(+), 38 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index a5ae7204..27572046 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -405,13 +405,13 @@ impl CachedPathImpl { return Ok(None); }; let real_path = if options.symlinks { - self.realpath(fs).await?.join("package.json") + self.realpath(fs).await?.join("package.json").to_ustr_path() } else { - package_json_path.clone() + package_json_path.to_ustr_path() }; match PackageJson::parse( - package_json_path.clone().into(), - real_path.into(), + package_json_path.to_ustr_path(), + real_path, package_json_string, ) { Ok(v) => Ok(Some(Arc::new(v))), @@ -428,7 +428,7 @@ impl CachedPathImpl { if let Some(err) = serde_err { Err(ResolveError::from_serde_json_error( - package_json_path.into(), + package_json_path.as_std_path().to_path_buf(), &err, Some(package_json_string), )) @@ -436,7 +436,7 @@ impl CachedPathImpl { let (line, column) = off_to_location(&package_json_string, parse_err.index()); Err(ResolveError::JSON(JSONError { - path: package_json_path.into(), + path: package_json_path.as_std_path().to_path_buf(), message: parse_err.error().to_string(), line, column, diff --git a/src/lib.rs b/src/lib.rs index 19b2e727..096a2f25 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1184,8 +1184,7 @@ impl ResolverGeneric { // 5. let MATCH = PACKAGE_EXPORTS_RESOLVE(pathToFileURL(SCOPE), // "." + X.slice("name".length), `package.json` "exports", ["node", "require"]) // defined in the ESM resolver. - let package_url = - Utf8Path::from_path(package_json.directory()).expect("path should be UTF-8"); + let package_url = package_json.directory(); // Note: The subpath is not prepended with a dot on purpose // because `package_exports_resolve` matches subpath without the leading dot. for exports in package_json.exports_fields(&self.options.exports_fields) { @@ -1286,9 +1285,7 @@ impl ResolverGeneric { } ctx.with_resolving_alias(new_specifier.to_string()); ctx.with_fully_specified(false); - let cached_path = self - .cache - .value(Utf8Path::from_path(package_json.directory()).expect("path should be UTF-8")); + let cached_path = self.cache.value(package_json.directory()); self .require(&cached_path, new_specifier, ctx) .await @@ -1938,7 +1935,7 @@ impl ResolverGeneric { if specifier == "#" { return Err(ResolveError::InvalidModuleSpecifier( specifier.to_string(), - package_json.path.clone(), + package_json.path.as_std_path().to_path_buf(), )); } } @@ -1946,7 +1943,7 @@ impl ResolverGeneric { .package_imports_exports_resolve( specifier, imports, - Utf8Path::from_path(package_json.directory()).expect("path should be UTF-8"), + package_json.directory(), /* is_imports */ true, &self.options.condition_names, ctx, @@ -1962,7 +1959,7 @@ impl ResolverGeneric { if has_imports { Err(ResolveError::PackageImportNotDefined( specifier.to_string(), - package_json.path.clone(), + package_json.path.as_std_path().to_path_buf(), )) } else { Ok(None) diff --git a/src/package_json/simd.rs b/src/package_json/simd.rs index d9759856..cc528ebe 100644 --- a/src/package_json/simd.rs +++ b/src/package_json/simd.rs @@ -5,7 +5,6 @@ use std::{ fmt::{Debug, Formatter}, marker::PhantomData, - path::{Path, PathBuf}, }; use camino::Utf8Path; @@ -17,7 +16,7 @@ use simd_json::{ to_borrowed_value, BorrowedValue, Error as SimdParseError, ObjectHasher, }; -use crate::{path::PathUtil, ResolveError}; +use crate::{path::PathUtil, ResolveError, UstrPath}; pub type JSONMap<'a> = simd_json::borrowed::Object<'a>; @@ -135,10 +134,10 @@ impl Default for JSONCell { #[derive(Debug, Default)] pub struct PackageJson { /// Path to `package.json`. Contains the `package.json` filename. - pub path: PathBuf, + pub path: UstrPath, /// Realpath to `package.json`. Contains the `package.json` filename. - pub realpath: PathBuf, + pub realpath: UstrPath, /// The "name" field defines your package's name. /// The "name" field can be used in addition to the "exports" field to self-reference a package using its name. @@ -190,7 +189,11 @@ impl From for ParseError { impl PackageJson { /// # Panics /// # Errors - pub(crate) fn parse(path: PathBuf, realpath: PathBuf, json: Vec) -> Result { + pub(crate) fn parse( + path: UstrPath, + realpath: UstrPath, + json: Vec, + ) -> Result { if json.starts_with(&BOM) { return Err(ParseError { message: "BOM character found".to_string(), @@ -276,7 +279,7 @@ impl PackageJson { /// # Panics /// /// * When the package.json path is misconfigured. - pub fn directory(&self) -> &Path { + pub fn directory(&self) -> &Utf8Path { debug_assert!(self .realpath .file_name() @@ -373,7 +376,7 @@ impl PackageJson { return Self::alias_value(path, value); } } else { - let dir = Utf8Path::from_path(self.path.parent().unwrap()).expect("path should be UTF-8"); + let dir = self.path.parent().unwrap(); for (key, value) in object { let joined = dir.normalize_with(key.to_string()); if joined == path { diff --git a/src/tests/package_json.rs b/src/tests/package_json.rs index 59f83b5e..a885dcb3 100644 --- a/src/tests/package_json.rs +++ b/src/tests/package_json.rs @@ -1,15 +1,13 @@ #[cfg(test)] mod tests { - use std::path::PathBuf; - - use crate::{package_json::ParseError, PackageJson}; + use crate::{package_json::ParseError, PackageJson, UstrPath}; #[tokio::test] async fn test_json_with_bom() { - let mock_path = PathBuf::from("package.json"); + let mock_path = UstrPath::new("package.json"); let json_with_bom = b"\xEF\xBB\xBF{\"name\": \"example-package\"}".to_vec(); - let result = PackageJson::parse(mock_path.clone(), mock_path.clone(), json_with_bom).err(); + let result = PackageJson::parse(mock_path, mock_path, json_with_bom).err(); assert_eq!( result, @@ -22,20 +20,20 @@ mod tests { #[tokio::test] async fn test_normal_json() { - let mock_path = PathBuf::from("package.json"); + let mock_path = UstrPath::new("package.json"); let json_with_bom = r##"{"name": "example-package"}"##.as_bytes().to_vec(); - let parsed = PackageJson::parse(mock_path.clone(), mock_path.clone(), json_with_bom).unwrap(); + let parsed = PackageJson::parse(mock_path, mock_path, json_with_bom).unwrap(); assert_eq!(parsed.name.unwrap(), "example-package"); } #[tokio::test] async fn test_broken_json() { - let mock_path = PathBuf::from("package.json"); + let mock_path = UstrPath::new("package.json"); let json_with_bom = r##"{"broken":"string"##.as_bytes().to_vec(); - let parsed_err = PackageJson::parse(mock_path.clone(), mock_path.clone(), json_with_bom).err(); + let parsed_err = PackageJson::parse(mock_path, mock_path, json_with_bom).err(); assert_eq!( parsed_err, @@ -49,10 +47,10 @@ mod tests { #[tokio::test] async fn test_empty_string() { - let mock_path = PathBuf::from("package.json"); + let mock_path = UstrPath::new("package.json"); let json_with_bom = " ".as_bytes().to_vec(); - let parse_error = PackageJson::parse(mock_path.clone(), mock_path.clone(), json_with_bom) + let parse_error = PackageJson::parse(mock_path, mock_path, json_with_bom) .err() .unwrap(); @@ -64,4 +62,30 @@ mod tests { } ); } + + #[tokio::test] + async fn package_json_path_is_one_interned_pointer_across_resolves() { + use crate::{ResolveOptions, Resolver}; + + let f = crate::tests::fixture().join("extensions"); + let resolver = Resolver::new(ResolveOptions { + extensions: vec![".ts".into(), String::new(), ".js".into()], + ..ResolveOptions::default() + }); + + let a = resolver.resolve(&f, "./foo").await.expect("should resolve"); + let b = resolver + .resolve(&f, "./foo.ts") + .await + .expect("should resolve"); + + let (Some(pa), Some(pb)) = (a.package_json(), b.package_json()) else { + panic!("both resolutions should carry a package.json"); + }; + assert_eq!( + pa.path.as_str().as_ptr(), + pb.path.as_str().as_ptr(), + "the same package.json path must be one interned pointer, not two copies" + ); + } } diff --git a/src/tests/pnp.rs b/src/tests/pnp.rs index ed659145..1f69b73d 100644 --- a/src/tests/pnp.rs +++ b/src/tests/pnp.rs @@ -150,12 +150,7 @@ async fn pnp_resolve_description_file() { let r = resolver.resolve(&fixture, &full_path).await.unwrap(); assert_eq!( - r.package_json - .unwrap() - .path - .to_str() - .expect("path should be UTF-8") - .to_string(), + r.package_json.unwrap().path.as_str().to_string(), Utf8Path::from_path(&fixture) .expect("path should be UTF-8") .join(".yarn/cache/preact-npm-10.25.4-2dd2c0aa44-33a009d614.zip/node_modules/preact") diff --git a/src/ustr_path.rs b/src/ustr_path.rs index aac14202..5a294f7f 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -157,6 +157,15 @@ impl UstrPath { } } +impl Default for UstrPath { + /// The empty path. `Ustr::default()` interns `""`, so this is a handle to a + /// real interned entry rather than a dangling one. + #[inline] + fn default() -> Self { + Self(Ustr::default()) + } +} + impl Hash for UstrPath { #[inline] fn hash(&self, state: &mut H) { @@ -308,6 +317,11 @@ mod tests { use super::*; + #[test] + fn default_is_the_empty_path() { + assert_eq!(UstrPath::default().as_str(), ""); + } + #[test] fn same_string_is_same_pointer() { let a = UstrPath::new("/a/b/c.js"); From a57076b751c98e14ecbfff7ae582a2593618902e Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 02:51:14 +0800 Subject: [PATCH 10/26] refactor(resolution): store resolved path as UstrPath Resolution.path, CachedPathImpl.canonicalized, and realpath()'s return type move from Utf8PathBuf to the interned, Copy UstrPath. Both arms of realpath's cache-hit fast path and both branches of load_realpath now avoid allocating. --- src/cache.rs | 23 +++++++++++++---------- src/lib.rs | 6 +++--- src/resolution.rs | 36 ++++++++++++++++++++++++++++++------ 3 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 27572046..ec8ec8ff 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -158,7 +158,7 @@ pub struct CachedPathImpl { path: Box, parent: Option, meta: OnceLock>, - canonicalized: OnceLock>, + canonicalized: OnceLock>, node_modules: OnceLock>, package_json: OnceLock>>, /// Memoized interned form of `self.path`. Without it every `is_file` / @@ -259,11 +259,12 @@ impl CachedPathImpl { ) } - pub async fn realpath(&self, fs: &Fs) -> io::Result { + pub async fn realpath(&self, fs: &Fs) -> io::Result { // Cache hit: avoid the heap-allocated `Box::pin` for the cache-miss state machine - // by returning before delegating to the boxed recursive helper. + // by returning before delegating to the boxed recursive helper. Both arms are now + // a `Copy` — neither allocates. if let Some(cached) = self.canonicalized.get() { - return Ok(cached.clone().unwrap_or_else(|| self.path.to_path_buf())); + return Ok(cached.unwrap_or_else(|| self.dep_path())); } self.realpath_uncached(fs).await } @@ -271,7 +272,7 @@ impl CachedPathImpl { fn realpath_uncached<'a, Fs: FileSystem + Send + Sync>( &'a self, fs: &'a Fs, - ) -> BoxFuture<'a, io::Result> { + ) -> BoxFuture<'a, io::Result> { Box::pin(async move { self .canonicalized @@ -284,10 +285,12 @@ impl CachedPathImpl { return fs .canonicalize(self.path.as_std_path()) .await - .map(|path| Some(Utf8PathBuf::from_path_buf(path).expect("path should be UTF-8"))); + .map(|path| Some(path.to_ustr_path())); } if let Some(parent) = self.parent() { - let mut real_path = parent.realpath(fs).await?; + // Reuse the parent's realpath as a mutable buffer for the final + // component instead of rebuilding the whole path. + let mut real_path = parent.realpath(fs).await?.to_path_buf(); // Unnormalized paths (e.g. from alias values or absolute specifiers) can // end in `..`, where `file_name()` returns `None` and would silently drop // the component; POSIX semantics pop it after the parent is resolved. @@ -298,13 +301,13 @@ impl CachedPathImpl { } _ => {} } - return Ok(Some(real_path)); + return Ok(Some(real_path.to_ustr_path())); } Ok(None) }) .await - .cloned() - .map(|r| r.unwrap_or_else(|| self.path.to_path_buf())) + .copied() + .map(|r| r.unwrap_or_else(|| self.dep_path())) }) } diff --git a/src/lib.rs b/src/lib.rs index 096a2f25..08d6c22f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -750,18 +750,18 @@ impl ResolverGeneric { &self, cached_path: &CachedPath, ctx: &mut Ctx, - ) -> Result { + ) -> Result { if self.options.symlinks { cached_path .realpath(&self.cache.fs) .await .map(|path| { - ctx.add_file_dependency(path.as_path()); + ctx.add_file_dependency(&path); path }) .map_err(ResolveError::from) } else { - Ok(cached_path.to_path_buf()) + Ok(cached_path.to_ustr_path()) } } diff --git a/src/resolution.rs b/src/resolution.rs index 0acab18f..9541a994 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -4,14 +4,12 @@ use std::{ sync::Arc, }; -use camino::Utf8PathBuf; - -use crate::package_json::PackageJson; +use crate::{package_json::PackageJson, ustr_path::UstrPath}; /// The final path resolution with optional `?query` and `#fragment` #[derive(Clone)] pub struct Resolution { - pub(crate) path: Utf8PathBuf, + pub(crate) path: UstrPath, /// path query `?query`, contains `?`. pub(crate) query: Option, @@ -46,9 +44,17 @@ impl Resolution { self.path.as_std_path() } + /// Returns the interned path without query and fragment. + /// + /// Zero-copy: hand this to a downstream store instead of `path()` to avoid + /// re-allocating and re-hashing the string. + pub fn ustr_path(&self) -> UstrPath { + self.path + } + /// Returns the path without query and fragment pub fn into_path_buf(self) -> PathBuf { - self.path.into_std_path_buf() + self.path.as_std_path().to_path_buf() } /// Returns the path query `?query`, contains the leading `?` @@ -68,7 +74,7 @@ impl Resolution { /// Returns the full path with query and fragment pub fn full_path(&self) -> PathBuf { - let mut path = self.path.clone().into_string(); + let mut path = self.path.as_str().to_owned(); if let Some(query) = &self.query { path.push_str(query); } @@ -93,3 +99,21 @@ async fn test() { assert_eq!(resolution.full_path(), PathBuf::from("foo?query#fragment")); assert_eq!(resolution.into_path_buf(), PathBuf::from("foo")); } + +#[tokio::test] +async fn ustr_path_accessor_is_the_same_pointer_as_the_stored_path() { + let resolution = Resolution { + path: "foo".into(), + query: None, + fragment: None, + package_json: None, + }; + assert_eq!(resolution.ustr_path().as_str(), "foo"); + assert_eq!( + resolution.ustr_path().as_str().as_ptr(), + UstrPath::new("foo").as_str().as_ptr() + ); + // The legacy accessors keep their signatures. + assert_eq!(resolution.path(), Path::new("foo")); + assert_eq!(resolution.into_path_buf(), PathBuf::from("foo")); +} From 26553b36993ef3d399163f8f6a8b92a44f0a67be Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 03:06:16 +0800 Subject: [PATCH 11/26] perf(tsconfig): keep file dependencies as interned UstrPath --- src/cache.rs | 10 +++--- src/lib.rs | 4 +-- src/tests/tsconfig_project_references.rs | 39 ++++++++++++++++++++++++ src/tsconfig.rs | 13 +++++--- 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index ec8ec8ff..3bb6bcdf 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -5,7 +5,6 @@ use std::{ hash::{BuildHasherDefault, Hash, Hasher}, io, ops::Deref, - path::PathBuf, sync::Arc, }; @@ -26,7 +25,7 @@ use crate::{ pub struct Cache { pub(crate) fs: Fs, paths: DashSet>, - tsconfigs: DashMap, BuildHasherDefault>, + tsconfigs: DashMap, BuildHasherDefault>, } impl Cache { @@ -68,7 +67,8 @@ impl Cache { F: FnOnce(TsConfig) -> Fut + Send, Fut: Send + Future>, { - if let Some(tsconfig_ref) = self.tsconfigs.get(path.as_std_path()) { + let key = path.to_ustr_path(); + if let Some(tsconfig_ref) = self.tsconfigs.get(&key) { return Ok(Arc::clone(tsconfig_ref.value())); } let meta = self.fs.metadata(path.as_std_path()).await.ok(); @@ -96,9 +96,7 @@ impl Cache { })?; tsconfig = callback(tsconfig).await?; let tsconfig = Arc::new(tsconfig.build()); - self - .tsconfigs - .insert(path.as_std_path().to_path_buf(), Arc::clone(&tsconfig)); + self.tsconfigs.insert(key, Arc::clone(&tsconfig)); Ok(tsconfig) } } diff --git a/src/lib.rs b/src/lib.rs index 08d6c22f..f60b3f00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1497,7 +1497,7 @@ impl ResolverGeneric { ) .await?; for dependency in &tsconfig.file_dependencies { - ctx.add_file_dependency(dependency.as_path()); + ctx.add_file_dependency(dependency); } let paths = tsconfig.resolve(cached_path.path(), specifier); for path in paths { @@ -1617,7 +1617,7 @@ impl ResolverGeneric { .await?; tsconfig .file_dependencies - .extend(reference_tsconfig.file_dependencies.iter().cloned()); + .extend(reference_tsconfig.file_dependencies.iter().copied()); for nested in &reference_tsconfig.flattened_references { if flattened_reference_paths.insert(nested.path.clone()) { tsconfig.flattened_references.push(Arc::clone(nested)); diff --git a/src/tests/tsconfig_project_references.rs b/src/tests/tsconfig_project_references.rs index b977c85a..7a1dcff2 100644 --- a/src/tests/tsconfig_project_references.rs +++ b/src/tests/tsconfig_project_references.rs @@ -82,6 +82,45 @@ async fn tsconfig_file_as_file_dependencies() { } } +#[tokio::test] +async fn tsconfig_dependency_is_one_interned_pointer_across_resolves() { + let f = super::fixture_root().join("tsconfig/cases/project_references"); + + let resolver = Resolver::new(ResolveOptions { + tsconfig: Some(TsconfigOptions { + config_file: f.join("app"), + references: TsconfigReferences::Auto, + }), + ..ResolveOptions::default() + }); + + let shared_tsconfig = f.join("app/tsconfig.json"); + let expected = UstrPath::from(&shared_tsconfig); + + let mut pointers = vec![]; + for (dir, request) in [ + (f.join("project_b/src"), "@/index.ts"), + (f.join("project_a"), "@/index.ts"), + ] { + let mut ctx = ResolveContext::default(); + resolver + .resolve_with_context(&dir, request, &mut ctx) + .await + .expect("should resolve"); + let dep = ctx + .file_dependencies + .get(&expected) + .expect("app/tsconfig.json must be a file dependency"); + pointers.push(dep.as_str().as_ptr()); + } + + assert!( + pointers.windows(2).all(|w| w[0] == w[1]), + "the same tsconfig.json must be one interned pointer across resolves, \ + not one heap copy per resolve" + ); +} + #[tokio::test] async fn disabled() { let f = super::fixture_root().join("tsconfig/cases/project_references"); diff --git a/src/tsconfig.rs b/src/tsconfig.rs index 04c4fc45..5c247e1c 100644 --- a/src/tsconfig.rs +++ b/src/tsconfig.rs @@ -5,10 +5,13 @@ use indexmap::{IndexMap, IndexSet}; use rustc_hash::FxHasher; use serde::Deserialize; -use crate::path::PathUtil; +use crate::{ + path::PathUtil, + ustr_path::{ToUstrPath, UstrPath}, +}; pub type CompilerOptionsPathsMap = IndexMap, BuildHasherDefault>; -pub type FileDependencies = IndexSet>; +pub type FileDependencies = IndexSet>; #[derive(Debug, Clone, Eq, PartialEq, Deserialize)] #[serde(untagged)] @@ -87,13 +90,13 @@ impl TsConfig { let mut tsconfig: Self = serde_json::from_str("{}")?; tsconfig.root = root; tsconfig.path = path.to_path_buf(); - tsconfig.file_dependencies.insert(path.to_path_buf()); + tsconfig.file_dependencies.insert(path.to_ustr_path()); return Ok(tsconfig); } let mut tsconfig: Self = serde_json::from_str(json)?; tsconfig.root = root; tsconfig.path = path.to_path_buf(); - tsconfig.file_dependencies.insert(path.to_path_buf()); + tsconfig.file_dependencies.insert(path.to_ustr_path()); let directory = tsconfig.directory().to_path_buf(); if let Some(base_url) = &tsconfig.compiler_options.base_url { // keep the `${configDir}` template variable in the baseUrl @@ -164,7 +167,7 @@ impl TsConfig { } self .file_dependencies - .extend(other_config.file_dependencies.iter().cloned()); + .extend(other_config.file_dependencies.iter().copied()); } pub fn resolve(&self, path: &Utf8Path, specifier: &str) -> Vec { From 1bfb9430cba847e777472ce6b63534b344198ab0 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 03:19:35 +0800 Subject: [PATCH 12/26] refactor(pnp): store manifest path as UstrPath --- src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f60b3f00..96864a9d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -123,7 +123,7 @@ pub struct ResolverGeneric { /// Pre-built byte-trie over `options.fallback` keys. fallback_trie: AliasTrie, #[cfg(feature = "yarn_pnp")] - pnp_manifest: Arc>, + pnp_manifest: Arc>, /// Paths that have been searched and confirmed to have no `.pnp.cjs` reachable by filesystem walk. #[cfg(feature = "yarn_pnp")] pnp_no_manifest_cache: Arc>, @@ -1003,7 +1003,7 @@ impl ResolverGeneric { #[cfg(feature = "yarn_pnp")] #[cfg_attr(feature = "enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = cached_path.path().as_str())))] - fn find_pnp_manifest(&self, cached_path: &CachedPath) -> Option> { + fn find_pnp_manifest(&self, cached_path: &CachedPath) -> Option> { // 1. Already have a manifest → return it (covers global cache paths too) if let Some(manifest) = self.pnp_manifest.load_full() { return Some(manifest); @@ -1032,7 +1032,7 @@ impl ResolverGeneric { tracing::debug!("use manifest path: {:?}", manifest_path); let manifest = pnp::load_pnp_manifest(&manifest_path).ok()?; - let manifest = Arc::new((manifest_path, manifest)); + let manifest = Arc::new((manifest_path.to_ustr_path(), manifest)); let previous = self .pnp_manifest From 23e0b37a4d79009b39aab12f506aea1a4c455a3a Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 03:37:36 +0800 Subject: [PATCH 13/26] test: assert dependency paths are interned once end to end Proves the UstrPath work through the real resolve path: repeated resolves push the same interned pointer (not a fresh allocation) per dependency, and re-interning an already-seen path hits the existing entry. Adds an ignored test that prints ustr::num_entries()/total_allocated() as one-off memory evidence, reproducible via `cargo test -- --ignored --nocapture`. --- src/tests/interning.rs | 103 +++++++++++++++++++++++++++++++++++++++++ src/tests/mod.rs | 1 + 2 files changed, 104 insertions(+) create mode 100644 src/tests/interning.rs diff --git a/src/tests/interning.rs b/src/tests/interning.rs new file mode 100644 index 00000000..a7b5c88d --- /dev/null +++ b/src/tests/interning.rs @@ -0,0 +1,103 @@ +//! End-to-end guarantees for `UstrPath` interning. +//! +//! These assert on **pointer identity**, never on `ustr::num_entries()` or +//! `total_allocated()` — the interner is process-global and shared with every +//! other test running in parallel, so any global-count assertion is flaky by +//! construction. + +use std::path::PathBuf; + +use crate::{ResolveContext, ResolveOptions, Resolver}; + +/// `fixtures/enhanced_resolve/test/fixtures/extensions` — the same fixture and +/// options `src/tests/extensions.rs` resolves `./foo` against. +fn fixture_and_resolver() -> (PathBuf, Resolver) { + let f = super::fixture().join("extensions"); + let resolver = Resolver::new(ResolveOptions { + extensions: vec![".ts".into(), String::new(), ".js".into()], + ..ResolveOptions::default() + }); + (f, resolver) +} + +#[tokio::test] +async fn repeated_resolves_share_one_pointer_per_dependency() { + let (f, resolver) = fixture_and_resolver(); + + let mut first: Option> = None; + + for _ in 0..3 { + let mut ctx = ResolveContext::default(); + resolver + .resolve_with_context(&f, "./foo", &mut ctx) + .await + .expect("should resolve"); + + let mut snapshot: Vec<(String, *const u8)> = ctx + .file_dependencies + .iter() + .map(|p| (p.as_str().to_owned(), p.as_str().as_ptr())) + .collect(); + snapshot.sort_by(|a, b| a.0.cmp(&b.0)); + + match &first { + None => first = Some(snapshot), + Some(baseline) => assert_eq!( + baseline, &snapshot, + "every dependency path must be the same interned pointer on every \ + resolve — a differing pointer means a fresh copy was allocated" + ), + } + } +} + +#[tokio::test] +async fn equal_paths_from_different_sources_are_one_pointer() { + let (f, resolver) = fixture_and_resolver(); + + let mut ctx = ResolveContext::default(); + resolver + .resolve_with_context(&f, "./foo", &mut ctx) + .await + .expect("should resolve"); + + assert!( + !ctx.file_dependencies.is_empty(), + "the fixture must produce at least one file dependency" + ); + + for dep in &ctx.file_dependencies { + let reinterned = crate::UstrPath::new(dep.as_str()); + assert_eq!( + dep.as_str().as_ptr(), + reinterned.as_str().as_ptr(), + "re-interning {dep} must hit the existing entry" + ); + } +} + +/// One-off memory evidence for the plan's headline claim: repeated pushes of +/// the same dependency path cost one interner entry, not one allocation per +/// push. Not run on normal `cargo test` — global counts are shared with every +/// other test in the binary, so they are only meaningful in isolation. +/// +/// Run with: `cargo test --all-features -- --ignored --nocapture interning` +#[tokio::test] +#[ignore = "prints process-global interner stats; run in isolation with --ignored"] +async fn report_interner_memory_usage() { + let (f, resolver) = fixture_and_resolver(); + + for _ in 0..100 { + let mut ctx = ResolveContext::default(); + resolver + .resolve_with_context(&f, "./foo", &mut ctx) + .await + .expect("should resolve"); + } + + eprintln!( + "interner: {} entries, {} bytes allocated", + ustr::num_entries(), + ustr::total_allocated() + ); +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 2ccfd495..210c0c5b 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -9,6 +9,7 @@ mod fallback; mod full_specified; mod imports_field; mod incorrect_description_file; +mod interning; mod main_field; mod memory_fs; mod missing; From 88b6694dc9c5456ce18702d194f87166872bba98 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 03:49:18 +0800 Subject: [PATCH 14/26] test: guard interning test against a vacuously empty fixture repeated_resolves_share_one_pointer_per_dependency compared snapshots across resolves but never checked they were non-empty, so an empty file_dependencies set would have passed without proving anything. --- src/tests/interning.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tests/interning.rs b/src/tests/interning.rs index a7b5c88d..9c54e10b 100644 --- a/src/tests/interning.rs +++ b/src/tests/interning.rs @@ -40,6 +40,12 @@ async fn repeated_resolves_share_one_pointer_per_dependency() { .collect(); snapshot.sort_by(|a, b| a.0.cmp(&b.0)); + assert!( + !snapshot.is_empty(), + "the fixture must produce at least one file dependency, \ + otherwise this test passes without proving anything" + ); + match &first { None => first = Some(snapshot), Some(baseline) => assert_eq!( From 738904d192814c0d15a26a6d2e5bf2656873eb60 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 04:19:30 +0800 Subject: [PATCH 15/26] perf(tsconfig): intern config_file once instead of on every cache lookup Cache::tsconfig interned `path` on every call, including cache hits. Since options.tsconfig.config_file is constant per resolver, every thread hitting load_tsconfig_paths (re-entered per resolve for top-level lookups, alias candidates/redirects, and package self/imports subpaths) serialized on the same ustr interner bin mutex for a hash that never changes. Cache::tsconfig now takes an already-interned UstrPath instead of interning internally. ResolverGeneric caches options.tsconfig.config_file as UstrPath once at construction and passes it straight through on the hot path; the cold extends/ references call sites still intern inline. --- src/cache.rs | 14 +++++++++----- src/lib.rs | 31 +++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 3bb6bcdf..02a6438d 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -57,23 +57,27 @@ impl Cache { data } + /// `path` must already be interned by the caller. `config_file` is constant + /// per resolver but this is re-entered several times per resolve, so + /// interning here on every hit would serialize every thread on the same + /// `ustr` bin mutex for a hash that never changes; see `ResolverGeneric`'s + /// `tsconfig_config_file` field, computed once at construction. pub async fn tsconfig( &self, root: bool, - path: &Utf8Path, + path: UstrPath, callback: F, // callback for modifying tsconfig with `extends` ) -> Result, ResolveError> where F: FnOnce(TsConfig) -> Fut + Send, Fut: Send + Future>, { - let key = path.to_ustr_path(); - if let Some(tsconfig_ref) = self.tsconfigs.get(&key) { + if let Some(tsconfig_ref) = self.tsconfigs.get(&path) { return Ok(Arc::clone(tsconfig_ref.value())); } let meta = self.fs.metadata(path.as_std_path()).await.ok(); let tsconfig_path = if meta.is_some_and(|m| m.is_file) { - Cow::Borrowed(path) + Cow::Borrowed(path.as_utf8_path()) } else if meta.is_some_and(|m| m.is_dir) { Cow::Owned(path.join("tsconfig.json")) } else { @@ -96,7 +100,7 @@ impl Cache { })?; tsconfig = callback(tsconfig).await?; let tsconfig = Arc::new(tsconfig.build()); - self.tsconfigs.insert(key, Arc::clone(&tsconfig)); + self.tsconfigs.insert(path, Arc::clone(&tsconfig)); Ok(tsconfig) } } diff --git a/src/lib.rs b/src/lib.rs index 96864a9d..8aa01709 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,6 +122,11 @@ pub struct ResolverGeneric { alias_trie: AliasTrie, /// Pre-built byte-trie over `options.fallback` keys. fallback_trie: AliasTrie, + /// `options.tsconfig.config_file` interned once at construction, since it is + /// constant per resolver but looked up on every `require_without_parse` entry + /// into `load_tsconfig_paths`. Interning it per-call would serialize every + /// thread on the same `ustr` bin mutex for a hash that never changes. + tsconfig_config_file: Option, #[cfg(feature = "yarn_pnp")] pnp_manifest: Arc>, /// Paths that have been searched and confirmed to have no `.pnp.cjs` reachable by filesystem walk. @@ -148,11 +153,16 @@ impl ResolverGeneric { let options = options.sanitize(); let alias_trie = AliasTrie::build(&options.alias); let fallback_trie = AliasTrie::build(&options.fallback); + let tsconfig_config_file = options + .tsconfig + .as_ref() + .map(|t| t.config_file.to_ustr_path()); Self { options, cache: Arc::new(Cache::new(Fs::default())), alias_trie, fallback_trie, + tsconfig_config_file, #[cfg(feature = "yarn_pnp")] pnp_manifest: Arc::new(arc_swap::ArcSwapOption::empty()), #[cfg(feature = "yarn_pnp")] @@ -167,11 +177,16 @@ impl ResolverGeneric { let options = options.sanitize(); let alias_trie = AliasTrie::build(&options.alias); let fallback_trie = AliasTrie::build(&options.fallback); + let tsconfig_config_file = options + .tsconfig + .as_ref() + .map(|t| t.config_file.to_ustr_path()); Self { options, cache: Arc::new(Cache::new(file_system)), alias_trie, fallback_trie, + tsconfig_config_file, #[cfg(feature = "yarn_pnp")] pnp_manifest: Arc::new(arc_swap::ArcSwapOption::empty()), #[cfg(feature = "yarn_pnp")] @@ -186,11 +201,16 @@ impl ResolverGeneric { let options = options.sanitize(); let alias_trie = AliasTrie::build(&options.alias); let fallback_trie = AliasTrie::build(&options.fallback); + let tsconfig_config_file = options + .tsconfig + .as_ref() + .map(|t| t.config_file.to_ustr_path()); Self { options, cache: Arc::clone(&self.cache), alias_trie, fallback_trie, + tsconfig_config_file, #[cfg(feature = "yarn_pnp")] pnp_manifest: Arc::clone(&self.pnp_manifest), #[cfg(feature = "yarn_pnp")] @@ -1489,10 +1509,13 @@ impl ResolverGeneric { let Some(tsconfig_options) = &self.options.tsconfig else { return Ok(None); }; + let config_file = self + .tsconfig_config_file + .expect("computed alongside options.tsconfig at construction"); let tsconfig = self .load_tsconfig( /* root */ true, - Utf8Path::from_path(&tsconfig_options.config_file).expect("path should be UTF-8"), + config_file, &tsconfig_options.references, ) .await?; @@ -1513,7 +1536,7 @@ impl ResolverGeneric { fn load_tsconfig<'a>( &'a self, root: bool, - path: &'a Utf8Path, + path: UstrPath, references: &'a TsconfigReferences, ) -> BoxFuture<'a, Result, ResolveError>> { let fut = async move { @@ -1586,7 +1609,7 @@ impl ResolverGeneric { .cache .tsconfig( /* root */ true, - &reference_tsconfig_path, + reference_tsconfig_path.to_ustr_path(), |mut reference_tsconfig| { let current_path = current_path.clone(); async move { @@ -1663,7 +1686,7 @@ impl ResolverGeneric { let extended_tsconfig = self .load_tsconfig( /* root */ false, - &extended_tsconfig_path, + extended_tsconfig_path.to_ustr_path(), &TsconfigReferences::Disabled, ) .await?; From 4d435829e18e2473acb6daeace690985436a3236 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 04:20:30 +0800 Subject: [PATCH 16/26] docs(cache,ustr-path): correct stale comments, document doc(hidden) re-export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three comments in cache.rs still referred to hash_path, deleted when it moved into this file and was renamed hash_utf8_path, and claimed the unix byte-wise hash matches CachedPath's PartialEq. It does not: PartialEq is component-wise via std Path, so paths differing only in trailing/repeated separators compare equal but hash differently on unix. This predates this branch and never produces a wrong result (mismatched hashes just land in different DashSet buckets), only a duplicate cache entry — the comments now say so honestly. Also documents why ustr_path.rs re-exports ustr::IdentityHasher verbatim instead of a local newtype: rspack's ArcPathSet needs the exact same type for mem::take to work. That upstream item is #[doc(hidden)] and therefore outside semver; the ustr version pin in Cargo.toml is what actually protects the re-export. --- src/cache.rs | 30 +++++++++++++++++++++++------- src/ustr_path.rs | 12 ++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 02a6438d..3c312824 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -119,11 +119,17 @@ impl PartialEq for CachedPath { // Compare through std `Path`, not camino's `Utf8Path`. std `Path`/`Components` // equality has a raw-byte `memcmp` fast path (its own comment: "for hashmap // lookups"), whereas camino's `Utf8Path` always walks components with no fast - // path — that regressed this hottest cache-lookup equality. `as_std_path()` - // also preserves the per-platform semantics that match `hash_path` - // (byte-wise on Unix; component-wise on Windows, so `pack1/foo` and - // `pack1\foo` stay the same entry). A plain `as_str()` byte compare would be - // inconsistent with the component-wise hash on Windows and break dedup there. + // path — that regressed this hottest cache-lookup equality. + // + // Note this is *not* fully consistent with `hash_utf8_path` on Unix: this + // equality is component-wise (via std `Path`), so e.g. `/a/b` and `/a/b/` + // compare equal, while `hash_utf8_path` hashes the raw bytes and gives them + // different hashes on Unix. That gap predates this branch (the old + // `hash_path` had the same mismatch). It cannot produce a wrong lookup + // result — differing hashes just land in different `DashSet` buckets, so + // this `PartialEq` is never consulted for them — the only cost is a + // duplicate `CachedPath` entry (and duplicate metadata probe) for paths + // that differ only in trailing or repeated separators. self.0.path.as_std_path() == other.0.path.as_std_path() } } @@ -488,8 +494,10 @@ impl Hash for dyn CacheKey + '_ { impl PartialEq for dyn CacheKey + '_ { fn eq(&self, other: &Self) -> bool { - // std `Path` equality (memcmp fast path + per-platform semantics matching - // `hash_path`); see `CachedPath`'s `PartialEq` for the full rationale. + // std `Path` equality (memcmp fast path); not fully consistent with + // `hash_utf8_path` on Unix — see `CachedPath`'s `PartialEq` for the full + // rationale and its benign consequence (duplicate cache entries, never a + // wrong lookup). self.tuple().1.as_std_path() == other.tuple().1.as_std_path() } } @@ -532,6 +540,14 @@ impl Hasher for IdentityHasher { /// expensive on this, the hottest lookup in the crate. On other platforms it /// goes through `Path` so `pack1/foo` and `pack1\foo` stay one entry, matching /// `CachedPath`'s `PartialEq`. +/// +/// On Unix this bulk byte hash is *not* equivalent to `CachedPath`'s +/// `PartialEq`, which compares component-wise through std `Path`: paths that +/// differ only by a trailing or repeated separator (e.g. `/a/b` vs `/a/b/`) +/// are `PartialEq`-equal but hash differently here. That only costs a +/// duplicate `CachedPath` entry for such paths — it can never produce a wrong +/// lookup result, since mismatched hashes simply land in different buckets +/// and `PartialEq` is never consulted for them. #[inline] fn hash_utf8_path(path: &Utf8Path) -> u64 { let mut hasher = FxHasher::default(); diff --git a/src/ustr_path.rs b/src/ustr_path.rs index 5a294f7f..22108701 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -225,6 +225,18 @@ pub type UstrPathSet = HashSet Date: Fri, 31 Jul 2026 10:14:09 +0800 Subject: [PATCH 17/26] test: make path assertions platform-aware Two assertions hardcoded unix separators and failed on windows-latest. The implementation is correct: UstrPath::new canonicalizes /a/b to \a\b on Windows by design, so the CI failure was the assertions testing normalization by accident instead of what they meant to test. Spelled the expectations out per platform rather than rebuilding them with join().to_ustr_path(), which would only prove the code agrees with itself. --- src/cache.rs | 20 ++++++++++++++++++-- src/ustr_path.rs | 10 ++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 3c312824..037565cc 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -580,8 +580,24 @@ mod tests { let cache = Cache::new(FileSystemOs::default()); let cached = cache.value(Utf8Path::new("/a/b")); - assert_eq!(cached.node_modules_dep_path().as_str(), "/a/b/node_modules"); - assert_eq!(cached.package_json_dep_path().as_str(), "/a/b/package.json"); + // Spelled out per platform rather than rebuilt with `join().to_ustr_path()` + // — deriving the expectation the same way the code does would only prove + // the memo agrees with itself. On Windows `UstrPath::new` canonicalizes + // separators to `\`, so a bare `"/a/b/node_modules"` literal fails there + // while the code is behaving exactly as designed. + let (expected_node_modules, expected_package_json) = if cfg!(windows) { + (r"\a\b\node_modules", r"\a\b\package.json") + } else { + ("/a/b/node_modules", "/a/b/package.json") + }; + assert_eq!( + cached.node_modules_dep_path().as_str(), + expected_node_modules + ); + assert_eq!( + cached.package_json_dep_path().as_str(), + expected_package_json + ); assert!(cached.node_modules_dep_path.get().is_some()); assert!(cached.package_json_dep_path.get().is_some()); diff --git a/src/ustr_path.rs b/src/ustr_path.rs index 22108701..0884181e 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -378,9 +378,15 @@ mod tests { #[test] fn debug_prints_the_path_not_the_ustr_wrapper() { + // The point of this test is that `Debug`/`Display` render the path rather + // than `Ustr`'s own `u!("...")` wrapper. The separator is incidental — on + // Windows `UstrPath::new` canonicalizes `/a/b` to `\a\b`, so the expected + // text has to follow the platform or the assertion tests normalization by + // accident instead of formatting. + let canonical = if cfg!(windows) { r"\a\b" } else { "/a/b" }; let p = UstrPath::new("/a/b"); - assert_eq!(format!("{p:?}"), "\"/a/b\""); - assert_eq!(format!("{p}"), "/a/b"); + assert_eq!(format!("{p:?}"), format!("{canonical:?}")); + assert_eq!(format!("{p}"), canonical); } #[test] From b2d20c9b6be631caef0662f4ecf3bd4ac1897b9b Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 17:36:54 +0800 Subject: [PATCH 18/26] fix(path): intern paths verbatim instead of normalizing on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `UstrPath::new` rewrote Windows paths into canonical form (`\` separators, collapsed and trailing separators removed, uppercase drive letter) so every spelling of a path interned to one pointer, matching the folding `Path`'s component-wise `Hash`/`Eq` provides. That is right for a cache key but wrong for a general string handle. rspack stores caller-supplied specifiers in path sets — a loader calling `addBuildDependency("./build.txt")` puts that literal into `BuildInfo`'s `build_dependencies` — and the rewrite was observable through the public JS API, where `./build.txt` came back as `.\build.txt`. It also stripped the trailing separator from directory entries, so `node_modules/` stopped matching the `**/node_modules/**` ignore glob in rspack's watcher. Store the string verbatim. Distinct Windows spellings of one path now intern to distinct handles and compare unequal; if that dedup is needed later it belongs in a separate canonicalizing constructor rather than in every intern. `normalize_windows_separators` and its tests are kept for that. --- src/ustr_path.rs | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/ustr_path.rs b/src/ustr_path.rs index 0884181e..ceba6ba1 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -30,6 +30,10 @@ use ustr::Ustr; #[repr(transparent)] pub struct UstrPath(Ustr); +#[cfg_attr( + not(test), + expect(dead_code, reason = "only used by the currently unwired normalizer") +)] #[inline] const fn is_sep(c: u8) -> bool { c == b'/' || c == b'\\' @@ -54,14 +58,17 @@ const fn is_sep(c: u8) -> bool { /// Platform-independent on purpose. camino's `Utf8Path::components()` picks its /// separator set at compile time, so it cannot express Windows semantics on a /// unix host — this would otherwise be untestable off Windows. -// Referenced by `UstrPath::new` only on Windows, but compiled and tested -// everywhere so the normalization rules stay verifiable on a unix host. Also -// excluded under `cfg(test)`: the test module below calls it directly, so on -// a non-Windows test build it is not actually dead and the expectation would -// go unfulfilled. +// NOT CURRENTLY WIRED UP. `UstrPath::new` used to call this on Windows, but +// `UstrPath` also carries strings that are not canonical filesystem paths — +// rspack stores caller-supplied dependency specifiers such as +// `addBuildDependency("./build.txt")` in path sets — and rewriting those +// changed values observable through the public JS API (`./build.txt` came back +// as `.\build.txt`). Correctness of the stored string wins over dedup for now; +// the function and its tests are kept so re-enabling it behind a separate +// canonicalizing constructor stays cheap. #[cfg_attr( - not(any(windows, test)), - expect(dead_code, reason = "windows-only caller; tested on all platforms") + not(test), + expect(dead_code, reason = "kept for reference; see comment above") )] fn normalize_windows_separators(s: &str) -> Option { let bytes = s.as_bytes(); @@ -118,18 +125,14 @@ fn normalize_windows_separators(s: &str) -> Option { } impl UstrPath { - /// Intern `path` and return a handle to it. + /// Intern `path` verbatim and return a handle to it. /// - /// On Windows the path is first rewritten into canonical form so that every - /// spelling of the same path (`C:/a/b`, `C:\a\b`, `C:\a\\b`, `C:\a\b\`) - /// interns to one pointer — preserving the dedup semantics `Path`'s - /// component-wise `Hash`/`Eq` used to provide. + /// The string is stored exactly as given. On Windows this means distinct + /// spellings of one path (`C:/a/b` vs `C:\a\b`) intern to distinct handles, + /// so they compare unequal where `Path`'s component-wise `Hash`/`Eq` would + /// have folded them — see the note on `normalize_windows_separators`. #[inline] pub fn new(path: &str) -> Self { - #[cfg(windows)] - if let Some(normalized) = normalize_windows_separators(path) { - return Self(Ustr::from(&normalized)); - } Self(Ustr::from(path)) } From 298220b16c51cf697278b7176a5956ecedbd29b5 Mon Sep 17 00:00:00 2001 From: pshu Date: Fri, 31 Jul 2026 17:55:17 +0800 Subject: [PATCH 19/26] fix(path): align UstrPath hash and equality with std Path on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interning stores paths verbatim, so on Windows the spellings `Path` treats as one path — `C:/a/b`, `C:\a\b`, `C:\a\\b`, `C:\a\b\`, `c:\a\b` — become distinct handles and, under a derived `PartialEq`, compared unequal. Using them as set or map keys therefore stopped deduplicating, which is the behaviour `hash_path`'s component-wise `Path::hash` used to provide. Fold the spellings in the comparison rather than in what gets interned: `PartialEq` tries the interned pointer first and falls back to `Path`'s component semantics on Windows, and `Hash` matches it. The stored string is still exactly what the caller passed, so rspack's caller-supplied specifiers survive the round trip unchanged. Both `Hash` branches end in a single `write_u64`. That is required, not stylistic: `UstrPathSet` is keyed by `ustr::IdentityHasher`, whose `write` only reads a value when handed exactly 8 bytes and silently yields 0 otherwise, so forwarding `Path::hash` directly would collapse every key into one bucket with no error. The new tests fold through that hasher rather than a generic one so a regression there fails loudly. Also drops a stale `expect(dead_code)` on `is_sep`, which no longer holds now that the normalizer is unwired — it was breaking `cargo check --all-features` on every platform and Check Wasm in CI. --- src/ustr_path.rs | 144 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 132 insertions(+), 12 deletions(-) diff --git a/src/ustr_path.rs b/src/ustr_path.rs index ceba6ba1..4127a60d 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -7,6 +7,8 @@ use std::{ }; use camino::{Utf8Path, Utf8PathBuf}; +#[cfg(windows)] +use rustc_hash::FxHasher; use ustr::Ustr; /// A globally interned UTF-8 path. @@ -15,10 +17,15 @@ use ustr::Ustr; /// once process-wide, so the same path handed to many consumers costs one /// pointer each instead of one heap allocation each. /// -/// Equality degenerates to a pointer comparison (interning guarantees one -/// pointer per string) and hashing to a single `u64` load from the interner +/// On unix, equality degenerates to a pointer comparison (interning guarantees +/// one pointer per string) and hashing to a single `u64` load from the interner /// entry header, so `UstrPathSet` lookups cost one `write_u64`. /// +/// On Windows both fall back to `Path`'s component semantics when the pointers +/// differ, so the spellings `Path` treats as one path stay one key. Paths are +/// stored verbatim, so that folding lives in the comparison rather than in what +/// gets interned. +/// /// The interner is shared with rspack — both crates depend on the same /// `ustr-fxhash` version, hence the same static — so a path interned here is /// already interned for rspack. @@ -26,14 +33,10 @@ use ustr::Ustr; /// # Lifetime /// /// Interned strings are **never freed**. See `CLAUDE_USTR_PATH_DESIGN.md` §4.3. -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy)] #[repr(transparent)] pub struct UstrPath(Ustr); -#[cfg_attr( - not(test), - expect(dead_code, reason = "only used by the currently unwired normalizer") -)] #[inline] const fn is_sep(c: u8) -> bool { c == b'/' || c == b'\\' @@ -127,10 +130,15 @@ fn normalize_windows_separators(s: &str) -> Option { impl UstrPath { /// Intern `path` verbatim and return a handle to it. /// - /// The string is stored exactly as given. On Windows this means distinct - /// spellings of one path (`C:/a/b` vs `C:\a\b`) intern to distinct handles, - /// so they compare unequal where `Path`'s component-wise `Hash`/`Eq` would - /// have folded them — see the note on `normalize_windows_separators`. + /// The string is stored exactly as given — rspack puts caller-supplied + /// specifiers such as `addBuildDependency("./build.txt")` into path sets and + /// reads them back through the JS API, so rewriting them here would change + /// observable values. See the note on `normalize_windows_separators`. + /// + /// On Windows this means distinct spellings of one path (`C:/a/b` vs + /// `C:\a\b`) intern to distinct handles. They still compare and hash equal — + /// `PartialEq`/`Hash` fold them via `Path`'s component semantics rather than + /// the stored bytes — so using them as set or map keys still dedups. #[inline] pub fn new(path: &str) -> Self { Self(Ustr::from(path)) @@ -169,10 +177,56 @@ impl Default for UstrPath { } } +impl PartialEq for UstrPath { + /// Interning makes the pointer check exhaustive for byte-identical strings, + /// which on unix is the whole story — `hash_utf8_path` has always compared + /// resolver paths byte-wise there. + /// + /// Windows additionally folds spellings: `Path`'s `Eq` walks components, so + /// `C:/a/b`, `C:\a\b`, `C:\a\\b`, `C:\a\b\` and `c:\a\b` are all one path. + /// Since [`UstrPath::new`] stores the string verbatim, those spellings are + /// distinct handles, and only the component walk can tell they are equal. + #[inline] + fn eq(&self, other: &Self) -> bool { + if self.0 == other.0 { + return true; + } + #[cfg(windows)] + { + self.as_std_path() == other.as_std_path() + } + #[cfg(not(windows))] + { + false + } + } +} + +impl Eq for UstrPath {} + impl Hash for UstrPath { + /// Kept in lockstep with [`PartialEq`] so `a == b` implies equal hashes. + /// + /// Both branches end in a single `write_u64`. That is load-bearing, not + /// stylistic: `UstrPathSet` is keyed by `ustr::IdentityHasher`, whose `write` + /// only reads a value when handed exactly 8 bytes and **silently yields 0** + /// otherwise. Forwarding `Path::hash` directly would feed it component-sized + /// writes, collapsing every key into bucket 0 without any error. #[inline] fn hash(&self, state: &mut H) { - state.write_u64(self.0.precomputed_hash()); + #[cfg(windows)] + { + // Fold the component walk down to one u64 first. Unlike unix, this + // cannot reuse the interner's precomputed byte hash: two spellings that + // compare equal must hash equally, and their bytes differ. + let mut hasher = FxHasher::default(); + self.as_std_path().hash(&mut hasher); + state.write_u64(hasher.finish()); + } + #[cfg(not(windows))] + { + state.write_u64(self.0.precomputed_hash()); + } } } @@ -350,6 +404,72 @@ mod tests { assert_ne!(UstrPath::new("/a/b"), UstrPath::new("/a/c")); } + /// `UstrPathSet` is keyed by `ustr::IdentityHasher`, so this is the hash that + /// actually decides bucketing. Folding it through that hasher — rather than a + /// generic one — is what proves `Hash` still delivers exactly 8 bytes: the + /// identity hasher silently yields 0 for any other width. + fn set_hash(p: UstrPath) -> u64 { + let mut hasher = ustr::IdentityHasher::default(); + p.hash(&mut hasher); + hasher.finish() + } + + #[cfg(windows)] + #[test] + fn windows_spellings_of_one_path_are_equal_and_hash_alike() { + // Stored verbatim, so these are genuinely distinct interned handles — + // the folding has to come from `PartialEq`/`Hash`, not from interning. + let canonical = UstrPath::new(r"C:\a\b"); + for spelling in ["C:/a/b", r"C:/a\b", r"C:\a\\b", r"C:\a\b\", r"c:\a\b"] { + let p = UstrPath::new(spelling); + assert_ne!( + p.as_str(), + canonical.as_str(), + "{spelling:?} must be stored verbatim, not rewritten" + ); + assert_eq!(p, canonical, "{spelling:?} should compare equal"); + assert_eq!( + set_hash(p), + set_hash(canonical), + "{spelling:?} hashes differently, which would break set bucketing" + ); + } + } + + #[cfg(windows)] + #[test] + fn windows_distinct_paths_stay_distinct() { + assert_ne!(UstrPath::new(r"C:\a\b"), UstrPath::new(r"C:\a\c")); + assert_ne!(UstrPath::new(r"C:\a\b"), UstrPath::new(r"D:\a\b")); + } + + #[cfg(windows)] + #[test] + fn windows_equal_paths_are_one_key_in_a_set() { + let mut set: UstrPathSet = HashSet::default(); + set.insert(UstrPath::new(r"C:\a\b")); + assert!(set.contains(&UstrPath::new("C:/a/b"))); + assert!(set.contains(&UstrPath::new(r"c:\a\b"))); + set.insert(UstrPath::new("C:/a/b")); + assert_eq!(set.len(), 1, "equal spellings must collapse to one entry"); + } + + #[test] + fn hash_delivers_eight_bytes_to_the_identity_hasher() { + // Guards both branches of `Hash`: a regression that forwarded + // `Path::hash` straight through would write component-sized chunks, and + // `IdentityHasher` would silently return 0 rather than fail. + assert_ne!(set_hash(UstrPath::new("/some/long/path/segment.js")), 0); + } + + #[test] + fn equal_paths_always_hash_equal() { + let a = UstrPath::new("/a/b"); + let b = UstrPath::new("/a/b"); + assert_eq!(a, b); + assert_eq!(set_hash(a), set_hash(b)); + } + #[test] fn hash_is_the_precomputed_hash() { let p = UstrPath::new("/x/y"); From ef9defbdb6b5d989e13ef01bcd8d69299dcefb32 Mon Sep 17 00:00:00 2001 From: pshu Date: Mon, 3 Aug 2026 14:07:16 +0800 Subject: [PATCH 20/26] refactor(path): swap ustr for a refcounted interner so paths can be freed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ustr` never frees an interned string. rspack calls `resolver_factory.clear_cache()` on every rebuild (via `plugin_driver.clear_cache`, from both `rebuild.rs` and the scopeguard in `compiler/mod.rs`), so a dev server rebuilds this cache continuously — and with a leak-forever interner the strings it drops stay resident, making RSS climb monotonically and never come back down. `internment::ArcIntern` refcounts instead: the entry goes away with the last handle. A new test in `cache.rs` pins that down — it interns a path no other test uses, checks the cache is holding it, then asserts the refcount falls back to 1 after `Cache::clear()`. Notable consequences: - `UstrPath` is no longer `Copy`. Every handoff is a refcount bump now, and the eight sites that relied on `Copy` (`*self`, `.copied()`, `*get_or_init`) clone explicitly. - `as_str()` no longer returns `&'static str`. It cannot: the entry is freed once the last handle drops. - The struct carries its own hash rather than using `ArcIntern`'s. Two reasons. `ArcIntern` hashes the pointer, but paths are stored verbatim, so on Windows `C:/a/b` and `C:\a\b` are distinct allocations that must still compare and hash alike — a pointer hash would break `a == b implies hash(a) == hash(b)`. Pointers are also aligned, so the low bits `IdentityHasher` buckets on are always zero. Computing it once at construction keeps lookups a single `write_u64` and keeps both platforms' semantics correct. - `IdentityHasher` is now defined here instead of re-exported from `ustr`. The local one panics in debug on any write that is not `write_u64`, and folds the bytes in release: the ustr version silently yielded 0 for other widths, which would collapse a misused map into one bucket with no error. - `internment`'s arc feature pulls ahash -> getrandom 0.3, which will not build for wasm32-unknown-unknown without a named backend. Handled with a target-scoped dependency plus `.cargo/config.toml`, so it applies only to that target — it is a CI sanity check here, and the shipped wasm artifact (wasm32-wasip1-threads) needs none of it. Verified wasm-bindgen stays out of the native and wasi dependency trees. --- .cargo/config.toml | 5 + Cargo.lock | 188 ++++++++++++++++++++++++---- Cargo.toml | 19 ++- src/cache.rs | 57 +++++++-- src/lib.rs | 3 +- src/resolution.rs | 2 +- src/tests/interning.rs | 5 +- src/tests/package_json.rs | 8 +- src/tsconfig.rs | 5 +- src/ustr_path.rs | 253 ++++++++++++++++++++++---------------- 10 files changed, 391 insertions(+), 154 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 00000000..bdfb681e --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,5 @@ +# Names the getrandom backend for wasm32-unknown-unknown. Required alongside +# the `wasm_js` feature (the feature alone is not enough). Scoped to that one +# target, which this repo only builds as a CI check. +[target.wasm32-unknown-unknown] +rustflags = ['--cfg', 'getrandom_backend="wasm_js"'] diff --git a/Cargo.lock b/Cargo.lock index 6aef94cb..518657b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,19 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "getrandom 0.3.4", + "once_cell", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.3" @@ -97,6 +110,12 @@ version = "2.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" +[[package]] +name = "bumpalo" +version = "3.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" + [[package]] name = "byteorder" version = "1.5.0" @@ -201,7 +220,7 @@ dependencies = [ "anyhow", "cc", "colored", - "getrandom", + "getrandom 0.2.17", "glob", "libc", "nix", @@ -336,6 +355,19 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d765eb1c0bda10d31e0ea185f5ee15da532d60b0912d2bd1441783439e749c5" +[[package]] +name = "dashmap" +version = "5.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" +dependencies = [ + "cfg-if", + "hashbrown 0.14.5", + "lock_api", + "once_cell", + "parking_lot_core", +] + [[package]] name = "dashmap" version = "6.1.0" @@ -432,6 +464,12 @@ dependencies = [ "num-traits", ] +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "foldhash" version = "0.2.0" @@ -537,6 +575,20 @@ dependencies = [ "wasi", ] +[[package]] +name = "getrandom" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" +dependencies = [ + "cfg-if", + "js-sys", + "libc", + "r-efi", + "wasip2", + "wasm-bindgen", +] + [[package]] name = "glob" version = "0.3.3" @@ -570,6 +622,17 @@ version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash 0.1.5", +] + [[package]] name = "hashbrown" version = "0.16.1" @@ -578,7 +641,7 @@ checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ "allocator-api2", "equivalent", - "foldhash", + "foldhash 0.2.0", ] [[package]] @@ -605,6 +668,18 @@ dependencies = [ "serde_core", ] +[[package]] +name = "internment" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "636d4b0f6a39fd684effe2a73f5310df16a3fa7954c26d36833e98f44d1977a2" +dependencies = [ + "ahash", + "dashmap 5.5.3", + "hashbrown 0.15.5", + "once_cell", +] + [[package]] name = "is-terminal" version = "0.4.17" @@ -631,6 +706,16 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" +[[package]] +name = "js-sys" +version = "0.3.103" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" +dependencies = [ + "cfg-if", + "wasm-bindgen", +] + [[package]] name = "json-strip-comments" version = "3.1.1" @@ -861,16 +946,6 @@ version = "11.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" -[[package]] -name = "parking_lot" -version = "0.12.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" -dependencies = [ - "lock_api", - "parking_lot_core", -] - [[package]] name = "parking_lot_core" version = "0.9.12" @@ -934,6 +1009,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" + [[package]] name = "radix_trie" version = "0.3.0" @@ -1043,11 +1124,13 @@ dependencies = [ "camino", "cfg-if", "codspeed-criterion-compat", - "dashmap", + "dashmap 6.1.0", "document-features", "dunce", "futures", + "getrandom 0.3.4", "indexmap", + "internment", "json-strip-comments", "mimalloc", "normalize-path", @@ -1061,7 +1144,6 @@ dependencies = [ "thiserror", "tokio", "tracing", - "ustr-fxhash", "vfs", ] @@ -1337,18 +1419,6 @@ version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f6ccf251212114b54433ec949fd6a7841275f9ada20dddd2f29e9ceea4501493" -[[package]] -name = "ustr-fxhash" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4d2224cabd887261556e0d3a483a7e9f87dcbaab3b820df7d748305a8d5cbc8" -dependencies = [ - "byteorder", - "lazy_static", - "parking_lot", - "rustc-hash", -] - [[package]] name = "value-trait" version = "0.12.1" @@ -1361,6 +1431,12 @@ dependencies = [ "ryu", ] +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "vfs" version = "0.13.0" @@ -1386,6 +1462,60 @@ version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" +[[package]] +name = "wasip2" +version = "1.0.4+wasi-0.2.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b67efb37e106e55ce722a510d6b5f9c17f083e5fc79afc2badeb12cc313d9487" +dependencies = [ + "wit-bindgen", +] + +[[package]] +name = "wasm-bindgen" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" +dependencies = [ + "cfg-if", + "once_cell", + "rustversion", + "wasm-bindgen-macro", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" +dependencies = [ + "bumpalo", + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" +dependencies = [ + "unicode-ident", +] + [[package]] name = "winapi-util" version = "0.1.11" @@ -1548,6 +1678,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650" +[[package]] +name = "wit-bindgen" +version = "0.57.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" + [[package]] name = "zerocopy" version = "0.8.27" diff --git a/Cargo.toml b/Cargo.toml index a6fb06c3..11da9546 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,10 +91,13 @@ indexmap = { version = "2.14.0", features = ["serde"] } json-strip-comments = "3.1.1" rustc-hash = { version = "2.1.2", default-features = false, features = ["std"] } thiserror = "2.0.18" -# Global string interner. MUST stay byte-identical to rspack's workspace -# declaration — a version split creates a second interner static and silently -# destroys the "one copy per path" guarantee that `UstrPath` exists for. -ustr = { package = "ustr-fxhash", version = "1.0.1", default-features = false } +# Refcounted global string interner. Entries are dropped when the last handle +# goes away, which is what lets a `Cache::clear()` actually return memory — +# rspack clears the resolver cache on every rebuild, so a leak-forever interner +# would make RSS climb monotonically in dev. MUST stay in lockstep with +# rspack's declaration: a version split creates a second interner and silently +# destroys the "one copy per path" guarantee this exists for. +internment = { version = "0.8.6", default-features = false, features = ["arc"] } pnp = { version = "0.12.9", optional = true } @@ -103,6 +106,14 @@ async-trait = "0.1.89" document-features = { version = "0.2.12", optional = true } futures = "0.3.32" +# `internment`'s arc feature pulls ahash -> getrandom 0.3, which refuses to +# build for wasm32-unknown-unknown unless a backend is named. That target is a +# CI sanity check only — the shipped wasm artifact is wasm32-wasip1-threads, +# which needs none of this — and the dependency exists solely for that target, +# so native and wasi builds never see wasm-bindgen. +[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies] +getrandom = { version = "0.3", features = ["wasm_js"] } + [target.'cfg(not(target_arch = "wasm32"))'.dependencies] tokio = { version = "1.52.3", default-features = false, features = ["sync", "rt-multi-thread", "macros"] } [target.'cfg(target_arch = "wasm32")'.dependencies] diff --git a/src/cache.rs b/src/cache.rs index 037565cc..03fb829f 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -206,21 +206,26 @@ impl CachedPathImpl { } fn dep_path(&self) -> UstrPath { - *self.dep_path.get_or_init(|| self.path.to_ustr_path()) + self + .dep_path + .get_or_init(|| self.path.to_ustr_path()) + .clone() } fn node_modules_dep_path(&self) -> UstrPath { - *self + self .node_modules_dep_path .get_or_init(|| self.path.join("node_modules").to_ustr_path()) + .clone() } /// Without this cache, each `None` cache-hit on `package_json` would - /// re-`join` + re-allocate the `Arc` + re-hash on every push. + /// re-`join` + re-intern on every push. fn package_json_dep_path(&self) -> UstrPath { - *self + self .package_json_dep_path .get_or_init(|| self.path.join("package.json").to_ustr_path()) + .clone() } pub fn path(&self) -> &Utf8Path { @@ -269,10 +274,10 @@ impl CachedPathImpl { pub async fn realpath(&self, fs: &Fs) -> io::Result { // Cache hit: avoid the heap-allocated `Box::pin` for the cache-miss state machine - // by returning before delegating to the boxed recursive helper. Both arms are now - // a `Copy` — neither allocates. + // by returning before delegating to the boxed recursive helper. Both arms are a + // refcount bump — neither allocates or re-enters the interner. if let Some(cached) = self.canonicalized.get() { - return Ok(cached.unwrap_or_else(|| self.dep_path())); + return Ok(cached.clone().unwrap_or_else(|| self.dep_path())); } self.realpath_uncached(fs).await } @@ -314,7 +319,7 @@ impl CachedPathImpl { Ok(None) }) .await - .copied() + .cloned() .map(|r| r.unwrap_or_else(|| self.dep_path())) }) } @@ -607,6 +612,42 @@ mod tests { ); } + /// The reason this crate uses a refcounted interner rather than a + /// leak-forever one. + /// + /// rspack calls `resolver_factory.clear_cache()` on every rebuild (via + /// `plugin_driver.clear_cache`), so a dev server rebuilds this cache + /// continuously. If interned paths outlived it, RSS would climb + /// monotonically and never come back down. + /// + /// Uses a path no other test resolves, and asserts on that one handle's + /// refcount. A global tally would be flaky: the interner is process-wide, so + /// a shared fixture path is held by every parallel test's cache at once. + #[tokio::test] + async fn clearing_the_cache_releases_interned_paths() { + let cache = Cache::new(FileSystemOs::default()); + let unique = Utf8Path::new("/only/this/test/interns/this/exact/path.js"); + + let cached = cache.value(unique); + let handle = cached.to_ustr_path(); + drop(cached); + + assert!( + handle.refcount() >= 2, + "the cache should still hold this path, got refcount {}", + handle.refcount() + ); + + cache.clear(); + + assert_eq!( + handle.refcount(), + 1, + "clear() must drop every handle the cache held; only this test's own \ + handle should remain" + ); + } + #[test] fn hash_utf8_path_matches_the_platform_contract() { let path = Utf8Path::new("/a/b/c.js"); diff --git a/src/lib.rs b/src/lib.rs index 8aa01709..943665be 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1511,6 +1511,7 @@ impl ResolverGeneric { }; let config_file = self .tsconfig_config_file + .clone() .expect("computed alongside options.tsconfig at construction"); let tsconfig = self .load_tsconfig( @@ -1640,7 +1641,7 @@ impl ResolverGeneric { .await?; tsconfig .file_dependencies - .extend(reference_tsconfig.file_dependencies.iter().copied()); + .extend(reference_tsconfig.file_dependencies.iter().cloned()); for nested in &reference_tsconfig.flattened_references { if flattened_reference_paths.insert(nested.path.clone()) { tsconfig.flattened_references.push(Arc::clone(nested)); diff --git a/src/resolution.rs b/src/resolution.rs index 9541a994..b9f255ba 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -49,7 +49,7 @@ impl Resolution { /// Zero-copy: hand this to a downstream store instead of `path()` to avoid /// re-allocating and re-hashing the string. pub fn ustr_path(&self) -> UstrPath { - self.path + self.path.clone() } /// Returns the path without query and fragment diff --git a/src/tests/interning.rs b/src/tests/interning.rs index 9c54e10b..cea710a8 100644 --- a/src/tests/interning.rs +++ b/src/tests/interning.rs @@ -102,8 +102,7 @@ async fn report_interner_memory_usage() { } eprintln!( - "interner: {} entries, {} bytes allocated", - ustr::num_entries(), - ustr::total_allocated() + "interner: {} live entries", + internment::ArcIntern::::num_objects_interned() ); } diff --git a/src/tests/package_json.rs b/src/tests/package_json.rs index a885dcb3..39c6d82a 100644 --- a/src/tests/package_json.rs +++ b/src/tests/package_json.rs @@ -7,7 +7,7 @@ mod tests { let mock_path = UstrPath::new("package.json"); let json_with_bom = b"\xEF\xBB\xBF{\"name\": \"example-package\"}".to_vec(); - let result = PackageJson::parse(mock_path, mock_path, json_with_bom).err(); + let result = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom).err(); assert_eq!( result, @@ -23,7 +23,7 @@ mod tests { let mock_path = UstrPath::new("package.json"); let json_with_bom = r##"{"name": "example-package"}"##.as_bytes().to_vec(); - let parsed = PackageJson::parse(mock_path, mock_path, json_with_bom).unwrap(); + let parsed = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom).unwrap(); assert_eq!(parsed.name.unwrap(), "example-package"); } @@ -33,7 +33,7 @@ mod tests { let mock_path = UstrPath::new("package.json"); let json_with_bom = r##"{"broken":"string"##.as_bytes().to_vec(); - let parsed_err = PackageJson::parse(mock_path, mock_path, json_with_bom).err(); + let parsed_err = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom).err(); assert_eq!( parsed_err, @@ -50,7 +50,7 @@ mod tests { let mock_path = UstrPath::new("package.json"); let json_with_bom = " ".as_bytes().to_vec(); - let parse_error = PackageJson::parse(mock_path, mock_path, json_with_bom) + let parse_error = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom) .err() .unwrap(); diff --git a/src/tsconfig.rs b/src/tsconfig.rs index 5c247e1c..d81901c6 100644 --- a/src/tsconfig.rs +++ b/src/tsconfig.rs @@ -11,7 +11,8 @@ use crate::{ }; pub type CompilerOptionsPathsMap = IndexMap, BuildHasherDefault>; -pub type FileDependencies = IndexSet>; +pub type FileDependencies = + IndexSet>; #[derive(Debug, Clone, Eq, PartialEq, Deserialize)] #[serde(untagged)] @@ -167,7 +168,7 @@ impl TsConfig { } self .file_dependencies - .extend(other_config.file_dependencies.iter().copied()); + .extend(other_config.file_dependencies.iter().cloned()); } pub fn resolve(&self, path: &Utf8Path, specifier: &str) -> Vec { diff --git a/src/ustr_path.rs b/src/ustr_path.rs index 4127a60d..fed3cb4f 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -7,35 +7,60 @@ use std::{ }; use camino::{Utf8Path, Utf8PathBuf}; -#[cfg(windows)] +use internment::ArcIntern; use rustc_hash::FxHasher; -use ustr::Ustr; /// A globally interned UTF-8 path. /// -/// 8 bytes, `Copy`, and `'static`: every distinct path string exists exactly -/// once process-wide, so the same path handed to many consumers costs one -/// pointer each instead of one heap allocation each. +/// 16 bytes: a refcounted handle into a process-wide interner, plus the +/// precomputed hash. Every distinct path string is stored once no matter how +/// many consumers hold it. /// -/// On unix, equality degenerates to a pointer comparison (interning guarantees -/// one pointer per string) and hashing to a single `u64` load from the interner -/// entry header, so `UstrPathSet` lookups cost one `write_u64`. +/// Equality is a pointer comparison whenever the strings are byte-identical, +/// and hashing is a single `u64` load, so `UstrPathSet` lookups cost one +/// `write_u64`. On Windows, spellings that differ only in separators or drive +/// case are distinct allocations, so both fall back to `Path`'s component +/// semantics to keep them one key. Paths are stored verbatim — the folding +/// lives in the comparison, not in what gets interned. /// -/// On Windows both fall back to `Path`'s component semantics when the pointers -/// differ, so the spellings `Path` treats as one path stay one key. Paths are -/// stored verbatim, so that folding lives in the comparison rather than in what -/// gets interned. -/// -/// The interner is shared with rspack — both crates depend on the same -/// `ustr-fxhash` version, hence the same static — so a path interned here is -/// already interned for rspack. +/// The interner is shared with rspack: both crates depend on the same +/// `internment` version, hence the same static, so a path interned here is +/// already interned there. /// /// # Lifetime /// -/// Interned strings are **never freed**. See `CLAUDE_USTR_PATH_DESIGN.md` §4.3. -#[derive(Clone, Copy)] -#[repr(transparent)] -pub struct UstrPath(Ustr); +/// Entries are refcounted and **dropped when the last handle goes away**. This +/// is what lets `Cache::clear()` actually return memory: rspack clears the +/// resolver cache on every rebuild, so a leak-forever interner would make RSS +/// climb monotonically in a dev server. The flip side is that `as_str()` +/// borrows from `self` rather than being `'static`. +#[derive(Clone)] +pub struct UstrPath { + inner: ArcIntern, + /// Precomputed so set lookups stay a single `write_u64`. + /// + /// Cannot be derived from `inner`'s address: on Windows two spellings of one + /// path are distinct allocations that must hash alike, and an address would + /// also be useless to `IdentityHasher` (pointers are aligned, so the low bits + /// that pick the bucket are always zero). + hash: u64, +} + +/// Hash a path the way `PartialEq` compares it, so `a == b` implies equal +/// hashes on every platform. +/// +/// Unix hashes the raw bytes — `hash_utf8_path` has always compared resolver +/// paths byte-wise there. Windows walks components, matching `Path`'s own +/// `Hash`, so `C:/a/b`, `C:\a\b`, `C:\a\b\` and `c:\a\b` land together. +#[inline] +fn hash_path_str(s: &str) -> u64 { + let mut hasher = FxHasher::default(); + #[cfg(windows)] + Path::new(s).hash(&mut hasher); + #[cfg(not(windows))] + hasher.write(s.as_bytes()); + hasher.finish() +} #[inline] const fn is_sep(c: u8) -> bool { @@ -141,59 +166,73 @@ impl UstrPath { /// the stored bytes — so using them as set or map keys still dedups. #[inline] pub fn new(path: &str) -> Self { - Self(Ustr::from(path)) + Self { + inner: ArcIntern::from(path), + hash: hash_path_str(path), + } } + /// Borrows from `self`, not `'static`: the entry is freed once the last + /// handle drops. #[inline] - pub fn as_str(&self) -> &'static str { - self.0.as_str() + pub fn as_str(&self) -> &str { + &self.inner } #[inline] - pub fn as_utf8_path(&self) -> &'static Utf8Path { - Utf8Path::new(self.0.as_str()) + pub fn as_utf8_path(&self) -> &Utf8Path { + Utf8Path::new(self.as_str()) } #[inline] - pub fn as_std_path(&self) -> &'static Path { - Path::new(self.0.as_str()) + pub fn as_std_path(&self) -> &Path { + Path::new(self.as_str()) } - /// The `FxHash` of the path bytes, precomputed by the interner. + /// The hash used for set and map lookups, computed once at construction. /// - /// Reading it is a single load from the entry header (`char_ptr - 16`). + /// Matches [`PartialEq`] per platform: raw bytes on unix, `Path` components + /// on Windows. #[inline] pub fn precomputed_hash(&self) -> u64 { - self.0.precomputed_hash() + self.hash + } + + /// How many handles currently share this interned string. Test/diagnostic + /// use — the count is a snapshot and can change concurrently. + #[cfg(test)] + pub(crate) fn refcount(&self) -> usize { + self.inner.refcount() } } impl Default for UstrPath { - /// The empty path. `Ustr::default()` interns `""`, so this is a handle to a - /// real interned entry rather than a dangling one. + /// The empty path — a handle to a real interned `""`, not a dangling one. #[inline] fn default() -> Self { - Self(Ustr::default()) + Self::new("") } } impl PartialEq for UstrPath { - /// Interning makes the pointer check exhaustive for byte-identical strings, - /// which on unix is the whole story — `hash_utf8_path` has always compared - /// resolver paths byte-wise there. + /// Byte-identical strings share one interner entry, so the pointer check + /// settles them — on unix that is the whole story, matching the byte-wise + /// comparison `hash_utf8_path` has always used for resolver paths. /// /// Windows additionally folds spellings: `Path`'s `Eq` walks components, so /// `C:/a/b`, `C:\a\b`, `C:\a\\b`, `C:\a\b\` and `c:\a\b` are all one path. - /// Since [`UstrPath::new`] stores the string verbatim, those spellings are - /// distinct handles, and only the component walk can tell they are equal. + /// Since [`UstrPath::new`] stores the string verbatim those are distinct + /// entries, and only the component walk can tell they are equal. The hash + /// check in front of it is a cheap reject, valid because `hash_path_str` + /// hashes components on Windows too. #[inline] fn eq(&self, other: &Self) -> bool { - if self.0 == other.0 { + if self.inner == other.inner { return true; } #[cfg(windows)] { - self.as_std_path() == other.as_std_path() + self.hash == other.hash && self.as_std_path() == other.as_std_path() } #[cfg(not(windows))] { @@ -205,28 +244,16 @@ impl PartialEq for UstrPath { impl Eq for UstrPath {} impl Hash for UstrPath { - /// Kept in lockstep with [`PartialEq`] so `a == b` implies equal hashes. + /// One `write_u64` of the hash computed at construction. /// - /// Both branches end in a single `write_u64`. That is load-bearing, not - /// stylistic: `UstrPathSet` is keyed by `ustr::IdentityHasher`, whose `write` - /// only reads a value when handed exactly 8 bytes and **silently yields 0** - /// otherwise. Forwarding `Path::hash` directly would feed it component-sized - /// writes, collapsing every key into bucket 0 without any error. + /// The single write is load-bearing, not stylistic: [`UstrPathSet`] is keyed + /// by [`IdentityHasher`], whose `write` only reads a value when handed + /// exactly 8 bytes and **silently yields 0** otherwise. Hashing the path + /// inline here would feed it component-sized writes and collapse every key + /// into one bucket with no error. #[inline] fn hash(&self, state: &mut H) { - #[cfg(windows)] - { - // Fold the component walk down to one u64 first. Unlike unix, this - // cannot reuse the interner's precomputed byte hash: two spellings that - // compare equal must hash equally, and their bytes differ. - let mut hasher = FxHasher::default(); - self.as_std_path().hash(&mut hasher); - state.write_u64(hasher.finish()); - } - #[cfg(not(windows))] - { - state.write_u64(self.0.precomputed_hash()); - } + state.write_u64(self.hash); } } @@ -277,24 +304,46 @@ impl fmt::Display for UstrPath { /// Uses `ustr::IdentityHasher` rather than the private `IdentityHasher` in /// `crate::cache` so rspack's `ArcPathSet` is the same concrete type and can /// take these sets by `mem::take` instead of re-bucketing them. -pub type UstrPathSet = HashSet>; +pub type UstrPathSet = HashSet>; -/// Re-exported so downstream crates can spell the same concrete set type -/// without taking a direct `ustr` dependency (and without risking a version -/// split — see the note on the `ustr` dependency in `Cargo.toml`). +/// Passes an already-computed hash straight through. /// -/// This is deliberately the upstream `ustr` type, not a local newtype: rspack -/// spells its `ArcPathSet` with this exact type, and only that type identity -/// lets rspack `mem::take` our dependency sets instead of re-bucketing every -/// element into its own hasher. Do not replace it with a local hasher. +/// [`UstrPath::hash`] writes the hash it computed at construction, so re-mixing +/// it here would be wasted work. Downstream spells its own path sets with this +/// exact type so it can `mem::take` ours instead of re-bucketing every element. /// -/// Upstream marks it `#[doc(hidden)]` (it is an implementation detail of -/// `ustr-fxhash`), so it is not semver-protected there — a patch release could -/// rename or remove it without notice. The pinned `ustr` version in -/// `Cargo.toml` is what actually protects this re-export; bumping that -/// dependency must re-verify `IdentityHasher` still exists and still behaves -/// like an identity hash. -pub use ustr::IdentityHasher; +/// Only `write_u64` is meaningful. Anything else is a misuse — the key type is +/// not `UstrPath` — and would silently produce 0 for every key, so it panics in +/// debug builds rather than quietly degrading the map into a linked list. +#[derive(Default, Clone, Copy)] +pub struct IdentityHasher(u64); + +impl Hasher for IdentityHasher { + #[inline] + fn write(&mut self, bytes: &[u8]) { + debug_assert!( + false, + "IdentityHasher only accepts write_u64; got a {}-byte write. The key \ + type is probably not UstrPath.", + bytes.len() + ); + // Release builds: fold the bytes rather than yielding 0, so a misuse + // degrades performance instead of collapsing every key into one bucket. + let mut h = FxHasher::default(); + h.write(bytes); + self.0 = h.finish(); + } + + #[inline] + fn write_u64(&mut self, n: u64) { + self.0 = n; + } + + #[inline] + fn finish(&self) -> u64 { + self.0 + } +} /// Convert any path-shaped value into an interned [`UstrPath`]. /// @@ -350,7 +399,7 @@ impl ToUstrPath for PathBuf { impl ToUstrPath for UstrPath { #[inline] fn to_ustr_path(&self) -> UstrPath { - *self + self.clone() } } @@ -408,8 +457,8 @@ mod tests { /// actually decides bucketing. Folding it through that hasher — rather than a /// generic one — is what proves `Hash` still delivers exactly 8 bytes: the /// identity hasher silently yields 0 for any other width. - fn set_hash(p: UstrPath) -> u64 { - let mut hasher = ustr::IdentityHasher::default(); + fn set_hash(p: &UstrPath) -> u64 { + let mut hasher = IdentityHasher::default(); p.hash(&mut hasher); hasher.finish() } @@ -429,8 +478,8 @@ mod tests { ); assert_eq!(p, canonical, "{spelling:?} should compare equal"); assert_eq!( - set_hash(p), - set_hash(canonical), + set_hash(&p), + set_hash(&canonical), "{spelling:?} hashes differently, which would break set bucketing" ); } @@ -459,7 +508,7 @@ mod tests { // Guards both branches of `Hash`: a regression that forwarded // `Path::hash` straight through would write component-sized chunks, and // `IdentityHasher` would silently return 0 rather than fail. - assert_ne!(set_hash(UstrPath::new("/some/long/path/segment.js")), 0); + assert_ne!(set_hash(&UstrPath::new("/some/long/path/segment.js")), 0); } #[test] @@ -467,10 +516,16 @@ mod tests { let a = UstrPath::new("/a/b"); let b = UstrPath::new("/a/b"); assert_eq!(a, b); - assert_eq!(set_hash(a), set_hash(b)); + assert_eq!(set_hash(&a), set_hash(&b)); } #[test] + // Unix only: there `Hash` forwards the interner's precomputed byte hash + // directly. Windows folds spellings through `Path`'s component semantics + // instead, so the two deliberately differ — that branch is covered by + // `windows_spellings_of_one_path_are_equal_and_hash_alike` and by + // `hash_delivers_eight_bytes_to_the_identity_hasher`. + #[cfg(not(windows))] fn hash_is_the_precomputed_hash() { let p = UstrPath::new("/x/y"); let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -501,15 +556,12 @@ mod tests { #[test] fn debug_prints_the_path_not_the_ustr_wrapper() { - // The point of this test is that `Debug`/`Display` render the path rather - // than `Ustr`'s own `u!("...")` wrapper. The separator is incidental — on - // Windows `UstrPath::new` canonicalizes `/a/b` to `\a\b`, so the expected - // text has to follow the platform or the assertion tests normalization by - // accident instead of formatting. - let canonical = if cfg!(windows) { r"\a\b" } else { "/a/b" }; + // `Debug`/`Display` render the path, not `Ustr`'s own `u!("...")` wrapper. + // Platform-independent because interning is verbatim — the string comes + // back exactly as passed in on every platform. let p = UstrPath::new("/a/b"); - assert_eq!(format!("{p:?}"), format!("{canonical:?}")); - assert_eq!(format!("{p}"), canonical); + assert_eq!(format!("{p:?}"), "\"/a/b\""); + assert_eq!(format!("{p}"), "/a/b"); } #[test] @@ -528,7 +580,7 @@ mod tests { // exactly 8 bytes. `UstrPath::hash` goes through the default `write_u64`, // which forwards `u64::to_ne_bytes()` — exactly 8. Guard that invariant so // a future change to `Hash` cannot silently collapse every key to bucket 0. - let mut hasher = ustr::IdentityHasher::default(); + let mut hasher = IdentityHasher::default(); UstrPath::new("/some/long/path/that/is/not/eight/bytes").hash(&mut hasher); assert_ne!(hasher.finish(), 0); } @@ -621,21 +673,12 @@ mod tests { assert_eq!(normalize_windows_separators(r"relative\Path"), None); } - #[cfg(windows)] - #[test] - fn equivalent_windows_spellings_intern_to_one_pointer() { - let canonical = UstrPath::new(r"C:\a\b"); - for spelling in [ - r"C:\a\b", "C:/a/b", r"C:/a\b", r"C:\a\\b", r"C:\a\b\", "c:/a/b", r"c:\a\b", - ] { - let p = UstrPath::new(spelling); - assert_eq!( - p.as_str().as_ptr(), - canonical.as_str().as_ptr(), - "spelling {spelling:?} should intern to the canonical pointer" - ); - } - } + // `equivalent_windows_spellings_intern_to_one_pointer` used to live here. It + // asserted every spelling interned to one pointer, which only held while + // `UstrPath::new` normalized. Interning is verbatim now, so the spellings are + // distinct handles that compare and hash equal instead — asserted by + // `windows_spellings_of_one_path_are_equal_and_hash_alike` above, which also + // checks the strings really are stored distinctly. #[test] fn to_ustr_path_accepts_every_path_flavor() { From 01eb426d873fef737d35cf2ef80243e72eccd2ed Mon Sep 17 00:00:00 2001 From: pshu Date: Mon, 3 Aug 2026 14:14:57 +0800 Subject: [PATCH 21/26] fix(path): repair two CI breaks from the interner swap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `node_modules_and_package_json_dep_paths_are_memoized` still asserted the literal `\a\b\node_modules`, left over from when `UstrPath::new` normalized separators. Interning is verbatim now, so `/a/b` keeps its slashes and only the joined component gets a `\` — the real value is `/a/b\node_modules`. Assert `file_name()` and `parent()` instead: pinning the joined string tests camino's platform behaviour, not the memo, and that is what broke twice. `getrandom` carries no code reference — it exists only to enable the wasm_js backend feature for wasm32-unknown-unknown — so cargo-shear reported it as unused. Listed under package.metadata.cargo-shear. --- Cargo.toml | 6 ++++++ src/cache.rs | 30 ++++++++++++------------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 11da9546..ccf659c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -149,6 +149,12 @@ yarn_pnp = ["pnp", "arc-swap"] # For remove tracing calls in release build enable_instrument = [] +# `getrandom` carries no code reference — it exists only to turn on the +# `wasm_js` backend feature for wasm32-unknown-unknown, so cargo-shear cannot +# see it being used. +[package.metadata.cargo-shear] +ignored = ["getrandom"] + [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/src/cache.rs b/src/cache.rs index 03fb829f..ea10f0dd 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -585,24 +585,18 @@ mod tests { let cache = Cache::new(FileSystemOs::default()); let cached = cache.value(Utf8Path::new("/a/b")); - // Spelled out per platform rather than rebuilt with `join().to_ustr_path()` - // — deriving the expectation the same way the code does would only prove - // the memo agrees with itself. On Windows `UstrPath::new` canonicalizes - // separators to `\`, so a bare `"/a/b/node_modules"` literal fails there - // while the code is behaving exactly as designed. - let (expected_node_modules, expected_package_json) = if cfg!(windows) { - (r"\a\b\node_modules", r"\a\b\package.json") - } else { - ("/a/b/node_modules", "/a/b/package.json") - }; - assert_eq!( - cached.node_modules_dep_path().as_str(), - expected_node_modules - ); - assert_eq!( - cached.package_json_dep_path().as_str(), - expected_package_json - ); + // Assert the structure, not the literal. Paths are interned verbatim, so + // the separator here is whatever `Utf8Path::join` picks: on Windows + // `/a/b` + `node_modules` is `/a/b\node_modules`, keeping the base's `/` + // and appending a `\`. Pinning that string would be testing camino, and + // it is exactly what broke this test before. What matters is that the memo + // yields `/`. + let node_modules = cached.node_modules_dep_path(); + let package_json = cached.package_json_dep_path(); + assert_eq!(node_modules.file_name(), Some("node_modules")); + assert_eq!(package_json.file_name(), Some("package.json")); + assert_eq!(node_modules.parent(), Some(Utf8Path::new("/a/b"))); + assert_eq!(package_json.parent(), Some(Utf8Path::new("/a/b"))); assert!(cached.node_modules_dep_path.get().is_some()); assert!(cached.package_json_dep_path.get().is_some()); From db5dea9745919f306090fa05cc651a2cb545c38c Mon Sep 17 00:00:00 2001 From: pshu Date: Mon, 3 Aug 2026 16:56:26 +0800 Subject: [PATCH 22/26] perf(interner): replace internment with a purpose-built refcounted interner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `internment::ArcIntern` costs 1004 Ir per intern against `ustr`'s 161, which callgrind attributes to two things it cannot avoid through its API: its `DashMap` container re-hashes the string with `ahash` (2.5 full-string hashes per intern) even though `UstrPath` has already computed an `FxHash` for it, and the handle has to carry that hash separately because the pointer is not usable as one, widening every stored path to 16 bytes. The replacement takes the hash as a parameter and keeps it in the entry header, so interning hashes once and the handle is back to one pointer. Entry storage is a single inline allocation rather than a header plus a boxed str. Concurrency rests on one invariant: every mutation that can drive a refcount to zero, and every mutation that could raise one back from zero, happens under that entry's shard lock. `clone` is the one unlocked mutation and can do neither, since its caller holds a handle. That removes the need for a resurrection retry protocol at the cost of one uncontended lock per drop, measured at 2.5 Ir. Measured on the resolver benchmarks (callgrind, est_cycles vs main): intern drops to 246 Ir, and the aggregate regression goes from +2.64% to +0.90% — single-thread +8.45% to +2.78%, [mt]resolve +7.34% to +1.98%. The remaining gap to `ustr` is the allocation a freed entry needs on its next use, which is the price of releasing memory at all. Dropping `internment` also drops ahash and getrandom, so the wasm32-unknown-unknown backend workaround goes away with it. --- .cargo/config.toml | 5 - Cargo.lock | 166 +--------------- Cargo.toml | 27 +-- src/interner.rs | 429 +++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/tests/interning.rs | 5 +- src/ustr_path.rs | 78 ++++---- 7 files changed, 481 insertions(+), 230 deletions(-) delete mode 100644 .cargo/config.toml create mode 100644 src/interner.rs diff --git a/.cargo/config.toml b/.cargo/config.toml deleted file mode 100644 index bdfb681e..00000000 --- a/.cargo/config.toml +++ /dev/null @@ -1,5 +0,0 @@ -# Names the getrandom backend for wasm32-unknown-unknown. Required alongside -# the `wasm_js` feature (the feature alone is not enough). Scoped to that one -# target, which this repo only builds as a CI check. -[target.wasm32-unknown-unknown] -rustflags = ['--cfg', 'getrandom_backend="wasm_js"'] diff --git a/Cargo.lock b/Cargo.lock index 518657b9..c7b804bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,19 +8,6 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" -[[package]] -name = "ahash" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" -dependencies = [ - "cfg-if", - "getrandom 0.3.4", - "once_cell", - "version_check", - "zerocopy", -] - [[package]] name = "aho-corasick" version = "1.1.3" @@ -110,12 +97,6 @@ version = "2.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" -[[package]] -name = "bumpalo" -version = "3.20.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" - [[package]] name = "byteorder" version = "1.5.0" @@ -220,7 +201,7 @@ dependencies = [ "anyhow", "cc", "colored", - "getrandom 0.2.17", + "getrandom", "glob", "libc", "nix", @@ -355,19 +336,6 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d765eb1c0bda10d31e0ea185f5ee15da532d60b0912d2bd1441783439e749c5" -[[package]] -name = "dashmap" -version = "5.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" -dependencies = [ - "cfg-if", - "hashbrown 0.14.5", - "lock_api", - "once_cell", - "parking_lot_core", -] - [[package]] name = "dashmap" version = "6.1.0" @@ -464,12 +432,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "foldhash" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" - [[package]] name = "foldhash" version = "0.2.0" @@ -575,20 +537,6 @@ dependencies = [ "wasi", ] -[[package]] -name = "getrandom" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" -dependencies = [ - "cfg-if", - "js-sys", - "libc", - "r-efi", - "wasip2", - "wasm-bindgen", -] - [[package]] name = "glob" version = "0.3.3" @@ -622,17 +570,6 @@ version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" -[[package]] -name = "hashbrown" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" -dependencies = [ - "allocator-api2", - "equivalent", - "foldhash 0.1.5", -] - [[package]] name = "hashbrown" version = "0.16.1" @@ -641,7 +578,7 @@ checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ "allocator-api2", "equivalent", - "foldhash 0.2.0", + "foldhash", ] [[package]] @@ -668,18 +605,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "internment" -version = "0.8.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "636d4b0f6a39fd684effe2a73f5310df16a3fa7954c26d36833e98f44d1977a2" -dependencies = [ - "ahash", - "dashmap 5.5.3", - "hashbrown 0.15.5", - "once_cell", -] - [[package]] name = "is-terminal" version = "0.4.17" @@ -706,16 +631,6 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" -[[package]] -name = "js-sys" -version = "0.3.103" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" -dependencies = [ - "cfg-if", - "wasm-bindgen", -] - [[package]] name = "json-strip-comments" version = "3.1.1" @@ -1009,12 +924,6 @@ dependencies = [ "proc-macro2", ] -[[package]] -name = "r-efi" -version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" - [[package]] name = "radix_trie" version = "0.3.0" @@ -1124,13 +1033,12 @@ dependencies = [ "camino", "cfg-if", "codspeed-criterion-compat", - "dashmap 6.1.0", + "dashmap", "document-features", "dunce", "futures", - "getrandom 0.3.4", + "hashbrown 0.17.0", "indexmap", - "internment", "json-strip-comments", "mimalloc", "normalize-path", @@ -1431,12 +1339,6 @@ dependencies = [ "ryu", ] -[[package]] -name = "version_check" -version = "0.9.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" - [[package]] name = "vfs" version = "0.13.0" @@ -1462,60 +1364,6 @@ version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" -[[package]] -name = "wasip2" -version = "1.0.4+wasi-0.2.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b67efb37e106e55ce722a510d6b5f9c17f083e5fc79afc2badeb12cc313d9487" -dependencies = [ - "wit-bindgen", -] - -[[package]] -name = "wasm-bindgen" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" -dependencies = [ - "cfg-if", - "once_cell", - "rustversion", - "wasm-bindgen-macro", - "wasm-bindgen-shared", -] - -[[package]] -name = "wasm-bindgen-macro" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" -dependencies = [ - "quote", - "wasm-bindgen-macro-support", -] - -[[package]] -name = "wasm-bindgen-macro-support" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" -dependencies = [ - "bumpalo", - "proc-macro2", - "quote", - "syn", - "wasm-bindgen-shared", -] - -[[package]] -name = "wasm-bindgen-shared" -version = "0.2.126" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" -dependencies = [ - "unicode-ident", -] - [[package]] name = "winapi-util" version = "0.1.11" @@ -1678,12 +1526,6 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650" -[[package]] -name = "wit-bindgen" -version = "0.57.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" - [[package]] name = "zerocopy" version = "0.8.27" diff --git a/Cargo.toml b/Cargo.toml index ccf659c4..320c6fd8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,13 +91,12 @@ indexmap = { version = "2.14.0", features = ["serde"] } json-strip-comments = "3.1.1" rustc-hash = { version = "2.1.2", default-features = false, features = ["std"] } thiserror = "2.0.18" -# Refcounted global string interner. Entries are dropped when the last handle -# goes away, which is what lets a `Cache::clear()` actually return memory — -# rspack clears the resolver cache on every rebuild, so a leak-forever interner -# would make RSS climb monotonically in dev. MUST stay in lockstep with -# rspack's declaration: a version split creates a second interner and silently -# destroys the "one copy per path" guarantee this exists for. -internment = { version = "0.8.6", default-features = false, features = ["arc"] } +# Backing table for `src/interner.rs`. `HashTable` is the piece that off-the- +# shelf interners cannot offer: it looks up by a caller-supplied hash, so the +# interner reuses the `FxHash` `UstrPath` already computed instead of hashing +# every path a second time. Version tracks what `indexmap` resolves to, so no +# extra copy of hashbrown enters the graph. +hashbrown = { version = "0.17.0", default-features = false, features = ["inline-more"] } pnp = { version = "0.12.9", optional = true } @@ -106,14 +105,6 @@ async-trait = "0.1.89" document-features = { version = "0.2.12", optional = true } futures = "0.3.32" -# `internment`'s arc feature pulls ahash -> getrandom 0.3, which refuses to -# build for wasm32-unknown-unknown unless a backend is named. That target is a -# CI sanity check only — the shipped wasm artifact is wasm32-wasip1-threads, -# which needs none of this — and the dependency exists solely for that target, -# so native and wasi builds never see wasm-bindgen. -[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies] -getrandom = { version = "0.3", features = ["wasm_js"] } - [target.'cfg(not(target_arch = "wasm32"))'.dependencies] tokio = { version = "1.52.3", default-features = false, features = ["sync", "rt-multi-thread", "macros"] } [target.'cfg(target_arch = "wasm32")'.dependencies] @@ -149,12 +140,6 @@ yarn_pnp = ["pnp", "arc-swap"] # For remove tracing calls in release build enable_instrument = [] -# `getrandom` carries no code reference — it exists only to turn on the -# `wasm_js` backend feature for wasm32-unknown-unknown, so cargo-shear cannot -# see it being used. -[package.metadata.cargo-shear] -ignored = ["getrandom"] - [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/src/interner.rs b/src/interner.rs new file mode 100644 index 00000000..e34cb438 --- /dev/null +++ b/src/interner.rs @@ -0,0 +1,429 @@ +//! A refcounted, sharded global string interner. +//! +//! Exists because the two obvious off-the-shelf choices each fail one half of +//! what [`crate::UstrPath`] needs: +//! +//! - `ustr` never frees, so a dev server's RSS climbs monotonically across +//! rebuilds. +//! - `internment::ArcIntern` frees, but its container hashes the string with +//! `ahash` behind a `DashMap`, so it re-hashes a string whose hash the caller +//! already computed — measured at 2.5 full-string hashes per intern, and +//! 1004 Ir per intern against `ustr`'s 161. +//! +//! This one takes the hash as a parameter and keeps it in the entry header, so +//! interning hashes the string exactly once and [`Interned::hash`] is a load. +//! +//! # Concurrency +//! +//! Every mutation that can drive a refcount to zero, and every mutation that +//! could raise one back from zero, happens while holding that entry's shard +//! lock. [`Interned::clone`] is the one unlocked mutation, and it can do +//! neither: its caller holds a handle, so the count it increments is at least +//! one. Two consequences fall out, and they are what make the design safe +//! without a resurrection protocol: +//! +//! - A count reaching zero means the dropping thread held the last handle, so +//! no concurrent `clone` of that entry is possible. +//! - A concurrent [`intern`] of the same string needs the shard lock the +//! dropping thread is holding, so it cannot observe a dying entry. + +use std::{ + alloc::{self, Layout}, + ptr::NonNull, + slice, str, + sync::{ + atomic::{AtomicUsize, Ordering}, + Mutex, MutexGuard, PoisonError, + }, +}; + +use hashbrown::HashTable; + +/// Interner entry: header followed by the string bytes in the same allocation. +/// +/// One allocation rather than a header plus a `Box` because interning is +/// allocation-bound — a refcounted interner re-allocates every entry that was +/// freed since the last use, and doubling the allocation count per entry is +/// directly visible in the resolver benchmarks. +#[repr(C)] +struct Entry { + count: AtomicUsize, + /// The caller's hash of the string. Stored so the handle stays one pointer + /// wide and [`Interned::hash`] costs a load instead of a re-hash. + hash: u64, + len: usize, + // `len` bytes of UTF-8 follow. +} + +/// Offset of the string bytes within an entry allocation. +/// +/// `[u8]` has alignment 1, so `Layout::extend` places it immediately after the +/// header with no padding — `Entry::layout` debug-asserts that this constant +/// and the computed offset agree. Spelling it as a constant keeps `Entry::str`, +/// which every `as_str()` goes through, down to one add. +const PAYLOAD_OFFSET: usize = std::mem::size_of::(); + +impl Entry { + /// Layout of a `len`-byte entry, plus the offset of the string bytes. + /// + /// `alloc` and `dealloc` must agree exactly, so both go through here. + fn layout(len: usize) -> (Layout, usize) { + let (layout, offset) = Layout::new::() + .extend(Layout::array::(len).expect("interned string length fits in a Layout")) + .expect("interner entry layout"); + debug_assert_eq!(offset, PAYLOAD_OFFSET); + (layout.pad_to_align(), offset) + } + + /// Allocate an entry holding `s`, with the refcount already at one for the + /// handle the caller is about to build. + fn alloc(s: &str, hash: u64) -> NonNull { + let (layout, offset) = Self::layout(s.len()); + // SAFETY: `layout` has non-zero size — the header alone is non-empty. + let Some(ptr) = NonNull::new(unsafe { alloc::alloc(layout) }) else { + alloc::handle_alloc_error(layout) + }; + // `alloc` returns memory aligned to `layout`, which starts from + // `Layout::new::()`, so this is `Self`'s alignment by construction. + let ptr = ptr.cast::(); + // SAFETY: `ptr` is freshly allocated for `layout`, so it is valid for + // writes of the header, and of `s.len()` bytes at `offset`. The two ranges + // cannot overlap `s`, which lives in the caller's memory. + unsafe { + ptr.as_ptr().write(Self { + count: AtomicUsize::new(1), + hash, + len: s.len(), + }); + ptr + .as_ptr() + .cast::() + .add(offset) + .copy_from_nonoverlapping(s.as_ptr(), s.len()); + } + ptr + } + + /// # Safety + /// + /// `ptr` must come from [`Entry::alloc`], must not have been deallocated, + /// and must be unreachable — removed from the table with a zero refcount. + unsafe fn dealloc(ptr: NonNull) { + // SAFETY: the caller guarantees `ptr` is a live entry, so reading `len` + // reproduces the layout it was allocated with. + let (layout, _) = Self::layout(unsafe { ptr.as_ref() }.len); + // SAFETY: same allocation and same layout as `alloc` used. + unsafe { alloc::dealloc(ptr.as_ptr().cast::(), layout) } + } + + /// # Safety + /// + /// `ptr` must point at a live entry, and the returned reference must not + /// outlive the handle that keeps it alive. + #[inline] + unsafe fn str<'a>(ptr: NonNull) -> &'a str { + // SAFETY: the caller guarantees `ptr` is live. + let len = unsafe { ptr.as_ref() }.len; + // SAFETY: `alloc` wrote exactly `len` bytes of a `&str` at + // `PAYLOAD_OFFSET`, so the range is initialized, in bounds, and UTF-8. + unsafe { + let bytes = slice::from_raw_parts(ptr.as_ptr().cast::().add(PAYLOAD_OFFSET), len); + str::from_utf8_unchecked(bytes) + } + } +} + +/// Enough shards that four resolver threads rarely collide, matching what +/// `ustr` uses. Must stay a power of two — [`shard`] slices the index out of +/// the hash's top bits. +const SHARDS: usize = 64; +const SHARD_BITS: u32 = SHARDS.trailing_zeros(); +const _: () = assert!(SHARDS.is_power_of_two()); + +/// `NonNull` is neither `Send` nor `Sync`, so it cannot go in a `static` +/// directly. Entries are: their bytes are immutable and `count` is atomic. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +struct EntryPtr(NonNull); + +// SAFETY: see the type's doc comment — an entry is immutable apart from its +// atomic refcount, and freeing one requires the shard lock. +unsafe impl Send for EntryPtr {} +// SAFETY: as above. +unsafe impl Sync for EntryPtr {} + +type Shard = Mutex>; + +/// The process-wide interner. +/// +/// Sharded by the **top** bits of the hash: `hashbrown` indexes buckets with +/// the low bits, so sharding on those would give every entry in a shard the +/// same bucket index. +static SHARD_TABLE: [Shard; SHARDS] = [const { Mutex::new(HashTable::new()) }; SHARDS]; + +fn shard(hash: u64) -> MutexGuard<'static, HashTable> { + let index = (hash >> (u64::BITS - SHARD_BITS)) as usize; + // A panic while holding a shard lock cannot leave the table inconsistent: + // the only calls made under it are hash-table operations and the allocator, + // neither of which unwinds partway through a mutation. Recovering beats + // poisoning every future intern of that shard. + SHARD_TABLE[index] + .lock() + .unwrap_or_else(PoisonError::into_inner) +} + +/// A refcounted handle to a globally interned string. +/// +/// One pointer wide. Not `Copy`: the refcount has to be maintained. +pub struct Interned { + ptr: NonNull, +} + +// SAFETY: `Entry` is immutable apart from `count`, which is atomic, and every +// operation that can free it happens under the shard lock (see module docs). +unsafe impl Send for Interned {} +// SAFETY: as above — sharing `&Interned` only exposes immutable string bytes. +unsafe impl Sync for Interned {} + +impl Interned { + #[inline] + pub fn as_str(&self) -> &str { + // SAFETY: `self` holds a reference, so the entry is live, and the returned + // borrow is tied to `self`. + unsafe { Entry::str(self.ptr) } + } + + /// The hash handed to [`intern`], read back from the entry header. + #[inline] + pub fn hash(&self) -> u64 { + // SAFETY: `self` holds a reference, so the entry is live. + unsafe { self.ptr.as_ref() }.hash + } + + /// Whether both handles name the same interner entry. + /// + /// Interning guarantees one entry per distinct string, so this settles + /// equality for byte-identical strings. + #[inline] + pub fn ptr_eq(&self, other: &Self) -> bool { + self.ptr == other.ptr + } + + /// How many handles currently share this entry. Test and diagnostic use — + /// the count is a snapshot and can change concurrently. + #[cfg(test)] + pub(crate) fn refcount(&self) -> usize { + // SAFETY: `self` holds a reference, so the entry is live. + unsafe { self.ptr.as_ref() }.count.load(Ordering::Acquire) + } +} + +/// How many entries the interner currently holds, across all shards. +/// +/// Diagnostic only, and inherently a snapshot: the interner is process-wide, so +/// this counts every string every thread is holding, not just the caller's. +#[cfg(test)] +pub fn live_entries() -> usize { + SHARD_TABLE + .iter() + .map(|shard| { + let table = shard.lock().unwrap_or_else(PoisonError::into_inner); + let len = table.len(); + drop(table); + len + }) + .sum() +} + +/// Intern `s`, whose hash the caller has already computed. +/// +/// `hash` must be a deterministic function of `s` alone; two calls with the +/// same string and different hashes would create two entries for it and break +/// the one-entry-per-string guarantee that [`Interned::ptr_eq`] relies on. It +/// is not otherwise constrained — [`crate::UstrPath`] deliberately passes a +/// hash that folds equivalent Windows spellings together, which only makes +/// those spellings share a shard and a bucket. +pub fn intern(s: &str, hash: u64) -> Interned { + let mut table = shard(hash); + if let Some(&EntryPtr(ptr)) = table.find(hash, |&EntryPtr(entry)| { + // SAFETY: entries stay in the table only while live. + unsafe { Entry::str(entry) == s } + }) { + // Under the shard lock, so this cannot race a drop that is freeing it. + // SAFETY: the entry is live and in the table. + unsafe { ptr.as_ref() } + .count + .fetch_add(1, Ordering::Relaxed); + return Interned { ptr }; + } + let ptr = Entry::alloc(s, hash); + table.insert_unique(hash, EntryPtr(ptr), |&EntryPtr(entry)| { + // SAFETY: entries stay in the table only while live. + unsafe { entry.as_ref() }.hash + }); + // The handle already owns the entry's first reference, so releasing the + // shard before building it is safe and keeps the critical section minimal. + drop(table); + Interned { ptr } +} + +impl Clone for Interned { + #[inline] + fn clone(&self) -> Self { + // No lock: `self` is a live handle, so the count is at least one both + // before and after. See the module's concurrency notes. + // SAFETY: `self` holds a reference, so the entry is live. + unsafe { self.ptr.as_ref() } + .count + .fetch_add(1, Ordering::Relaxed); + Self { ptr: self.ptr } + } +} + +impl Drop for Interned { + fn drop(&mut self) { + let hash = self.hash(); + // Taking the shard lock before the decrement is what removes the need for + // a resurrection protocol: an entry can only reach zero, and only be found + // again, under this lock. + let mut table = shard(hash); + // SAFETY: `self` still holds a reference, so the entry is live. + if unsafe { self.ptr.as_ref() } + .count + .fetch_sub(1, Ordering::AcqRel) + != 1 + { + return; + } + table + .find_entry(hash, |&EntryPtr(entry)| entry == self.ptr) + .expect("a live interned entry is always in its shard") + .remove(); + drop(table); + // SAFETY: the count reached zero and the entry is out of the table, so no + // other handle or lookup can reach it. + unsafe { Entry::dealloc(self.ptr) } + } +} + +#[cfg(test)] +mod tests { + use std::{sync::Arc, thread}; + + use super::{intern, Interned}; + + /// The interner is process-wide and shared with every other test running in + /// parallel, so assertions here use strings no other test interns and only + /// ever check one entry's own state. + fn h(s: &str) -> u64 { + use std::hash::Hasher as _; + let mut hasher = rustc_hash::FxHasher::default(); + hasher.write(s.as_bytes()); + hasher.finish() + } + + fn get(s: &str) -> Interned { + intern(s, h(s)) + } + + #[test] + fn equal_strings_share_one_entry() { + let a = get("interner::equal_strings/a/b/c.js"); + let b = get("interner::equal_strings/a/b/c.js"); + assert!(a.ptr_eq(&b)); + assert_eq!(a.as_str(), "interner::equal_strings/a/b/c.js"); + } + + #[test] + fn different_strings_get_different_entries() { + let a = get("interner::different/a.js"); + let b = get("interner::different/b.js"); + assert!(!a.ptr_eq(&b)); + } + + #[test] + fn the_empty_string_round_trips() { + // Zero-length payload is the one case where the entry is header-only. + let a = get(""); + assert_eq!(a.as_str(), ""); + assert!(a.ptr_eq(&get(""))); + } + + #[test] + fn hash_is_returned_verbatim() { + // The interner must not re-derive the hash: `UstrPath` relies on getting + // back exactly what it computed, including a Windows-folded value that + // does not match the stored bytes. + let interned = intern("interner::verbatim/x.js", 0xDEAD_BEEF_1234_5678); + assert_eq!(interned.hash(), 0xDEAD_BEEF_1234_5678); + } + + #[test] + fn colliding_hashes_stay_distinct_entries() { + // Windows folding hands equal hashes to different strings on purpose, so + // the table must disambiguate by bytes rather than trusting the hash. + let a = intern("interner::collide/one", 0x5555_5555_5555_5555); + let b = intern("interner::collide/two", 0x5555_5555_5555_5555); + assert!(!a.ptr_eq(&b)); + assert_eq!(a.as_str(), "interner::collide/one"); + assert_eq!(b.as_str(), "interner::collide/two"); + } + + #[test] + fn refcount_tracks_handles() { + let a = get("interner::refcount/a.js"); + assert_eq!(a.refcount(), 1); + let b = a.clone(); + assert_eq!(a.refcount(), 2); + drop(b); + assert_eq!(a.refcount(), 1); + } + + #[test] + fn dropping_the_last_handle_frees_the_entry() { + let first = get("interner::freed/a.js"); + let address = first.as_str().as_ptr(); + drop(first); + + // Re-interning after the entry was freed must produce a working handle. + // (It may or may not reuse `address` — the allocator decides — so the + // assertion is on the contents, not the pointer.) + let second = get("interner::freed/a.js"); + assert_eq!(second.as_str(), "interner::freed/a.js"); + assert_eq!( + second.refcount(), + 1, + "the freed entry must not have lingered" + ); + let _ = address; + } + + #[test] + fn concurrent_intern_and_drop_of_one_string_is_sound() { + // Drives the race the shard lock exists for: threads repeatedly take the + // last handle to zero while others intern the same string. Under Miri or + // ASan a resurrection bug surfaces here; without them it still catches + // double-free and lost-entry bugs. + let barrier = Arc::new(std::sync::Barrier::new(8)); + let threads: Vec<_> = (0..8) + .map(|t| { + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + for i in 0..2000 { + let key = format!("interner::race/{}", i % 4); + let a = get(&key); + assert_eq!(a.as_str(), key); + let b = a.clone(); + assert!(a.ptr_eq(&b)); + drop(a); + assert_eq!(b.as_str(), key); + } + t + }) + }) + .collect(); + for thread in threads { + thread.join().expect("interner race thread panicked"); + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 943665be..6f576f0b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -53,6 +53,7 @@ mod cache; mod context; mod error; mod file_system; +mod interner; mod options; mod package_json; mod path; diff --git a/src/tests/interning.rs b/src/tests/interning.rs index cea710a8..166e12b7 100644 --- a/src/tests/interning.rs +++ b/src/tests/interning.rs @@ -101,8 +101,5 @@ async fn report_interner_memory_usage() { .expect("should resolve"); } - eprintln!( - "interner: {} live entries", - internment::ArcIntern::::num_objects_interned() - ); + eprintln!("interner: {} live entries", crate::interner::live_entries()); } diff --git a/src/ustr_path.rs b/src/ustr_path.rs index fed3cb4f..da3d0275 100644 --- a/src/ustr_path.rs +++ b/src/ustr_path.rs @@ -7,25 +7,25 @@ use std::{ }; use camino::{Utf8Path, Utf8PathBuf}; -use internment::ArcIntern; use rustc_hash::FxHasher; +use crate::interner::{self, Interned}; + /// A globally interned UTF-8 path. /// -/// 16 bytes: a refcounted handle into a process-wide interner, plus the -/// precomputed hash. Every distinct path string is stored once no matter how -/// many consumers hold it. +/// One pointer wide. Every distinct path string is stored once no matter how +/// many consumers hold it, so the same path handed to N downstream stores +/// costs N pointers rather than N copies. /// /// Equality is a pointer comparison whenever the strings are byte-identical, -/// and hashing is a single `u64` load, so `UstrPathSet` lookups cost one -/// `write_u64`. On Windows, spellings that differ only in separators or drive -/// case are distinct allocations, so both fall back to `Path`'s component -/// semantics to keep them one key. Paths are stored verbatim — the folding -/// lives in the comparison, not in what gets interned. +/// and hashing is a single `u64` load from the entry header, so `UstrPathSet` +/// lookups cost one `write_u64`. On Windows, spellings that differ only in +/// separators or drive case are distinct entries, so both fall back to +/// `Path`'s component semantics to keep them one key. Paths are stored +/// verbatim — the folding lives in the comparison, not in what gets interned. /// -/// The interner is shared with rspack: both crates depend on the same -/// `internment` version, hence the same static, so a path interned here is -/// already interned there. +/// The interner is a static inside this crate, so rspack shares it by using +/// this type rather than by agreeing on a third-party crate version. /// /// # Lifetime /// @@ -35,16 +35,7 @@ use rustc_hash::FxHasher; /// climb monotonically in a dev server. The flip side is that `as_str()` /// borrows from `self` rather than being `'static`. #[derive(Clone)] -pub struct UstrPath { - inner: ArcIntern, - /// Precomputed so set lookups stay a single `write_u64`. - /// - /// Cannot be derived from `inner`'s address: on Windows two spellings of one - /// path are distinct allocations that must hash alike, and an address would - /// also be useless to `IdentityHasher` (pointers are aligned, so the low bits - /// that pick the bucket are always zero). - hash: u64, -} +pub struct UstrPath(Interned); /// Hash a path the way `PartialEq` compares it, so `a == b` implies equal /// hashes on every platform. @@ -166,17 +157,14 @@ impl UstrPath { /// the stored bytes — so using them as set or map keys still dedups. #[inline] pub fn new(path: &str) -> Self { - Self { - inner: ArcIntern::from(path), - hash: hash_path_str(path), - } + Self(interner::intern(path, hash_path_str(path))) } /// Borrows from `self`, not `'static`: the entry is freed once the last /// handle drops. #[inline] pub fn as_str(&self) -> &str { - &self.inner + self.0.as_str() } #[inline] @@ -189,20 +177,21 @@ impl UstrPath { Path::new(self.as_str()) } - /// The hash used for set and map lookups, computed once at construction. + /// The hash used for set and map lookups, computed once at construction and + /// read back from the interner entry header. /// /// Matches [`PartialEq`] per platform: raw bytes on unix, `Path` components /// on Windows. #[inline] pub fn precomputed_hash(&self) -> u64 { - self.hash + self.0.hash() } /// How many handles currently share this interned string. Test/diagnostic /// use — the count is a snapshot and can change concurrently. #[cfg(test)] pub(crate) fn refcount(&self) -> usize { - self.inner.refcount() + self.0.refcount() } } @@ -227,12 +216,13 @@ impl PartialEq for UstrPath { /// hashes components on Windows too. #[inline] fn eq(&self, other: &Self) -> bool { - if self.inner == other.inner { + if self.0.ptr_eq(&other.0) { return true; } #[cfg(windows)] { - self.hash == other.hash && self.as_std_path() == other.as_std_path() + self.precomputed_hash() == other.precomputed_hash() + && self.as_std_path() == other.as_std_path() } #[cfg(not(windows))] { @@ -253,7 +243,7 @@ impl Hash for UstrPath { /// into one bucket with no error. #[inline] fn hash(&self, state: &mut H) { - state.write_u64(self.hash); + state.write_u64(self.precomputed_hash()); } } @@ -301,9 +291,9 @@ impl fmt::Display for UstrPath { /// A `HashSet` keyed by the interner's precomputed hash. /// -/// Uses `ustr::IdentityHasher` rather than the private `IdentityHasher` in -/// `crate::cache` so rspack's `ArcPathSet` is the same concrete type and can -/// take these sets by `mem::take` instead of re-bucketing them. +/// Spelled with [`IdentityHasher`] rather than a set-local hasher so rspack's +/// `ArcPathSet` is the same concrete type and can take these sets by +/// `mem::take` instead of re-bucketing every element. pub type UstrPathSet = HashSet>; /// Passes an already-computed hash straight through. @@ -440,6 +430,18 @@ mod tests { assert_eq!(UstrPath::default().as_str(), ""); } + /// The handle must stay one pointer wide. It is stored by the million + /// downstream — every dependency set, every `Vec` — so widening it + /// (by caching the hash beside the pointer instead of in the interner entry, + /// say) shows up directly as `memcpy` in the resolver benchmarks. + #[test] + fn the_handle_is_one_pointer_wide() { + assert_eq!( + std::mem::size_of::(), + std::mem::size_of::() + ); + } + #[test] fn same_string_is_same_pointer() { let a = UstrPath::new("/a/b/c.js"); @@ -453,7 +455,7 @@ mod tests { assert_ne!(UstrPath::new("/a/b"), UstrPath::new("/a/c")); } - /// `UstrPathSet` is keyed by `ustr::IdentityHasher`, so this is the hash that + /// `UstrPathSet` is keyed by [`IdentityHasher`], so this is the hash that /// actually decides bucketing. Folding it through that hasher — rather than a /// generic one — is what proves `Hash` still delivers exactly 8 bytes: the /// identity hasher silently yields 0 for any other width. @@ -576,7 +578,7 @@ mod tests { #[test] fn identity_hasher_receives_eight_bytes() { - // `ustr::IdentityHasher::write` silently produces a 0 hash unless it gets + // `IdentityHasher::write` silently produces a 0 hash unless it gets // exactly 8 bytes. `UstrPath::hash` goes through the default `write_u64`, // which forwards `u64::to_ne_bytes()` — exactly 8. Guard that invariant so // a future change to `Hash` cannot silently collapse every key to bucket 0. From cbe846882aca9b895bcf0de9f124ca0fdf9332a5 Mon Sep 17 00:00:00 2001 From: pshu Date: Mon, 3 Aug 2026 22:17:36 +0800 Subject: [PATCH 23/26] perf(interner): drop refcounts without taking the shard lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most drops release a temporary handle while the cache still holds the path, so they never reach zero and never need the table — yet every one of them was taking a shard lock, serializing resolver threads on a mutex they had no work to do behind. The decrement moves out of the lock. Reaching zero is now a *claim* on the entry rather than ownership of it, so the drop path re-establishes both facts it used to get for free, in this order: 1. Look the entry up by address, without dereferencing it. A concurrent dropper may already have freed it, and removal happens under this lock, so absence means someone else won the claim. 2. Re-check the count. A concurrent `intern` may have resurrected the entry between the decrement and the lock; the count then names exactly the handles that exist, so the dropper walks away. `intern` is unchanged: it increments under the lock, and doing so to a zero-count entry is a valid resurrection rather than a race to detect. `concurrent_zero_crossings_are_sound` targets both windows — eight threads over a two-key space, every handle dropped immediately. Deleting either check above makes it fail reliably; it passed three for three before, which is why the test is written to keep entries at zero rather than merely busy. Drop falls from 53.5 Ir to 34.7 Ir per call. The contention this removes is invisible to callgrind, which serializes threads — CodSpeed's multi-threaded benchmarks are the measurement that can see it. --- src/interner.rs | 119 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 24 deletions(-) diff --git a/src/interner.rs b/src/interner.rs index e34cb438..4aefec45 100644 --- a/src/interner.rs +++ b/src/interner.rs @@ -15,17 +15,26 @@ //! //! # Concurrency //! -//! Every mutation that can drive a refcount to zero, and every mutation that -//! could raise one back from zero, happens while holding that entry's shard -//! lock. [`Interned::clone`] is the one unlocked mutation, and it can do -//! neither: its caller holds a handle, so the count it increments is at least -//! one. Two consequences fall out, and they are what make the design safe -//! without a resurrection protocol: +//! Refcount changes are lock-free; only the table is locked. [`intern`] holds +//! the shard lock while it looks up and increments, and a freed entry leaves +//! the table under that same lock, so the two never overlap. Both +//! [`Interned::clone`] and [`Interned::drop`] touch the count without any lock. //! -//! - A count reaching zero means the dropping thread held the last handle, so -//! no concurrent `clone` of that entry is possible. -//! - A concurrent [`intern`] of the same string needs the shard lock the -//! dropping thread is holding, so it cannot observe a dying entry. +//! Dropping to zero is therefore a *claim* on the entry, not ownership of it. +//! Between that decrement and the shard lock, two things can happen, and the +//! drop path checks for both before freeing: +//! +//! - **Resurrection.** An `intern` of the same string finds the entry and +//! increments it back. The count then names exactly the handles that exist, +//! so the dropper simply walks away — hence the `count != 0` re-check under +//! the lock. +//! - **A second claim.** The resurrected handle is itself dropped to zero, and +//! two threads now believe they must free. Removal happens under the lock, so +//! the loser finds the entry already absent — hence the lookup by address, +//! which must not dereference, before anything else. +//! +//! `clone` needs neither check: its caller holds a handle, so the count it +//! increments is at least one and cannot be a resurrection. use std::{ alloc::{self, Layout}, @@ -245,11 +254,15 @@ pub fn live_entries() -> usize { pub fn intern(s: &str, hash: u64) -> Interned { let mut table = shard(hash); if let Some(&EntryPtr(ptr)) = table.find(hash, |&EntryPtr(entry)| { - // SAFETY: entries stay in the table only while live. + // SAFETY: an entry is removed from the table before it is freed, and both + // happen under this lock, so everything reachable here is alive. unsafe { Entry::str(entry) == s } }) { - // Under the shard lock, so this cannot race a drop that is freeing it. - // SAFETY: the entry is live and in the table. + // The old value may be 0 — a dropper claimed this entry and is waiting for + // the lock. Resurrecting it is exactly right: it will see a non-zero count + // and leave the entry alone. Relaxed is enough because the lock we hold + // orders this against that check. + // SAFETY: the entry is in the table, so it is alive. unsafe { ptr.as_ref() } .count .fetch_add(1, Ordering::Relaxed); @@ -257,7 +270,7 @@ pub fn intern(s: &str, hash: u64) -> Interned { } let ptr = Entry::alloc(s, hash); table.insert_unique(hash, EntryPtr(ptr), |&EntryPtr(entry)| { - // SAFETY: entries stay in the table only while live. + // SAFETY: reachable table entries are alive (see above). unsafe { entry.as_ref() }.hash }); // The handle already owns the entry's first reference, so releasing the @@ -281,26 +294,43 @@ impl Clone for Interned { impl Drop for Interned { fn drop(&mut self) { + // Read the hash while we still own a reference. Past the decrement below, + // `self.ptr` is only an address — dereferencing it is unsound until the + // shard lock re-establishes that the entry is still alive. let hash = self.hash(); - // Taking the shard lock before the decrement is what removes the need for - // a resurrection protocol: an entry can only reach zero, and only be found - // again, under this lock. - let mut table = shard(hash); // SAFETY: `self` still holds a reference, so the entry is live. if unsafe { self.ptr.as_ref() } .count .fetch_sub(1, Ordering::AcqRel) != 1 { + // The overwhelming majority of drops land here — a temporary handle + // going away while the cache still holds the path. Keeping this path + // lock-free is what stops the shards from serializing resolver threads. + return; + } + let mut table = shard(hash); + // Address comparison only: a concurrent dropper may already have freed + // this entry, so it must not be dereferenced before it is found. + let Ok(entry) = table.find_entry(hash, |&EntryPtr(candidate)| candidate == self.ptr) else { + // Gone from the table, which only happens after another thread claimed + // the free. Nothing left to do — and nothing left to read. + return; + }; + // Found under the lock, and entries are freed only after being removed + // under this same lock, so the entry is alive again for as long as we hold + // it. + // SAFETY: as above. + if unsafe { self.ptr.as_ref() }.count.load(Ordering::Acquire) != 0 { + // Resurrected: an `intern` found this entry between our decrement and + // our lock, and now owns it. Leaving it in place is correct — the count + // reflects exactly the handles that exist. return; } - table - .find_entry(hash, |&EntryPtr(entry)| entry == self.ptr) - .expect("a live interned entry is always in its shard") - .remove(); + entry.remove(); drop(table); - // SAFETY: the count reached zero and the entry is out of the table, so no - // other handle or lookup can reach it. + // SAFETY: zero refcount and out of the table, so no handle can exist and + // no lookup can reach it. unsafe { Entry::dealloc(self.ptr) } } } @@ -397,6 +427,47 @@ mod tests { let _ = address; } + #[test] + fn concurrent_zero_crossings_are_sound() { + // Aimed squarely at the two windows the lock-free drop path opens: + // resurrection, and two threads both claiming the free. Every handle here + // is created and dropped immediately over a key space of two, so threads + // are almost always racing on an entry that is at or near zero — the state + // a version that freed unconditionally after `fetch_sub` would corrupt. + // + // Reads `as_str()` after the clone specifically so a use-after-free lands + // on the string bytes, where ASan or Miri will catch it. + let barrier = Arc::new(std::sync::Barrier::new(8)); + let threads: Vec<_> = (0..8) + .map(|_| { + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + for i in 0..5000 { + let key = format!("interner::zero-cross/{}", i % 2); + let handle = get(&key); + assert_eq!(handle.as_str(), key); + drop(handle); + } + }) + }) + .collect(); + for thread in threads { + thread.join().expect("zero-crossing thread panicked"); + } + + // Every handle above is gone, so nothing may have leaked a live entry. + for i in 0..2 { + let key = format!("interner::zero-cross/{i}"); + let fresh = get(&key); + assert_eq!( + fresh.refcount(), + 1, + "{key} outlived every handle that referenced it" + ); + } + } + #[test] fn concurrent_intern_and_drop_of_one_string_is_sound() { // Drives the race the shard lock exists for: threads repeatedly take the From 850c9082eb7fc76a171c4251a944cb79e2effc1c Mon Sep 17 00:00:00 2001 From: pshu Date: Wed, 5 Aug 2026 02:11:27 +0800 Subject: [PATCH 24/26] perf(interner): reclaim entries by sweeping instead of freeing on drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Freeing an entry the moment its last handle went away made every drop a potential table mutation, which is why the drop path needed a lock, and then — once that lock moved off the fast path — a two-window race analysis and a sanitizer run to trust it. It also meant a `Cache::clear()` threw away entries the very next resolve would re-intern: 84% of interns were misses, at 246 Ir each. The table now owns a reference of its own. A handle's drop can no longer reach zero, so it is a bare decrement — no lock, no lookup, nothing to race. Entries leave in `Shard::sweep`, which drops those back down to the table's single reference, triggered by insertions at a threshold that scales with the shard so the amortized cost per insertion stays constant. Soundness collapses back to one line: every 1 -> 2 transition happens under the shard lock, because the only way to get a handle to a table entry is `intern`, which holds at least the read lock, and a sweep holds the write lock. `clone` needs no lock since its caller's handle already puts the count at 2 or more. Lookups take a read lock rather than an exclusive one, so threads interning known paths stop queueing behind each other. Measured against main (callgrind, est_cycles): the aggregate goes from +0.90% to -0.69%, ahead of even the leak-forever `ustr` baseline at -0.38%. Interning halves to 122.7 Ir because entries survive `clear_cache()` and the next iteration hits instead of missing, and allocations land at 80,042 — exactly `ustr`'s count, 3,002 below main — while the table stays bounded. `Interner` grows a `Drop` so a dropped instance frees what it owns; an entry a handle still refers to is leaked rather than freed, since `Interned` does not borrow from the interner. ASan on Linux reports no errors and no leaks. --- src/cache.rs | 16 +- src/interner.rs | 597 +++++++++++++++++++++++++++++------------------- 2 files changed, 372 insertions(+), 241 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index ea10f0dd..da39f21a 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -617,6 +617,9 @@ mod tests { /// Uses a path no other test resolves, and asserts on that one handle's /// refcount. A global tally would be flaky: the interner is process-wide, so /// a shared fixture path is held by every parallel test's cache at once. + /// + /// The interner's table owns a reference of its own, so the floor here is 2, + /// not 1 — the entry itself is reclaimed later, by a sweep. #[tokio::test] async fn clearing_the_cache_releases_interned_paths() { let cache = Cache::new(FileSystemOs::default()); @@ -626,19 +629,20 @@ mod tests { let handle = cached.to_ustr_path(); drop(cached); + let held = handle.refcount(); assert!( - handle.refcount() >= 2, - "the cache should still hold this path, got refcount {}", - handle.refcount() + held >= 3, + "the cache and this test should both hold this path on top of the \ + interner's own reference, got refcount {held}" ); cache.clear(); assert_eq!( handle.refcount(), - 1, - "clear() must drop every handle the cache held; only this test's own \ - handle should remain" + 2, + "clear() must drop every handle the cache held, leaving this test's \ + handle and the interner's own reference" ); } diff --git a/src/interner.rs b/src/interner.rs index 4aefec45..11f0296d 100644 --- a/src/interner.rs +++ b/src/interner.rs @@ -1,4 +1,4 @@ -//! A refcounted, sharded global string interner. +//! A refcounted, sharded global string interner that reclaims its own entries. //! //! Exists because the two obvious off-the-shelf choices each fail one half of //! what [`crate::UstrPath`] needs: @@ -11,30 +11,43 @@ //! 1004 Ir per intern against `ustr`'s 161. //! //! This one takes the hash as a parameter and keeps it in the entry header, so -//! interning hashes the string exactly once and [`Interned::hash`] is a load. +//! interning hashes the string exactly once and [`Interned::hash`] is a load +//! rather than a rehash. That header is also why entries are hand-allocated +//! instead of being `Arc`: an `Arc` is a fat pointer with nowhere to +//! put the hash, which would push every stored path from 8 bytes to 24. //! -//! # Concurrency +//! # Reclamation //! -//! Refcount changes are lock-free; only the table is locked. [`intern`] holds -//! the shard lock while it looks up and increments, and a freed entry leaves -//! the table under that same lock, so the two never overlap. Both -//! [`Interned::clone`] and [`Interned::drop`] touch the count without any lock. +//! **The table owns a reference.** An entry's count is one for the table plus +//! one per live handle, so dropping a handle can never reach zero and never has +//! to free anything — [`Interned::drop`] is a bare decrement, with no lock, no +//! table lookup, and no race to lose. //! -//! Dropping to zero is therefore a *claim* on the entry, not ownership of it. -//! Between that decrement and the shard lock, two things can happen, and the -//! drop path checks for both before freeing: +//! Freeing happens in [`Shard::sweep`], which removes every entry whose count +//! is back down to one. Sweeps are triggered by insertions, at a threshold that +//! scales with the shard, so each insertion amortizes to a constant number of +//! checks however large the table grows. Memory therefore comes back on a +//! bounded delay rather than instantly, which is what a dev server needs: the +//! table cannot grow without bound when paths are transient, and it holds +//! exactly the live set when they are retained. //! -//! - **Resurrection.** An `intern` of the same string finds the entry and -//! increments it back. The count then names exactly the handles that exist, -//! so the dropper simply walks away — hence the `count != 0` re-check under -//! the lock. -//! - **A second claim.** The resurrected handle is itself dropped to zero, and -//! two threads now believe they must free. Removal happens under the lock, so -//! the loser finds the entry already absent — hence the lookup by address, -//! which must not dereference, before anything else. +//! # Soundness //! -//! `clone` needs neither check: its caller holds a handle, so the count it -//! increments is at least one and cannot be a resurrection. +//! Sweeping on `count == 1` rests on one invariant: +//! +//! > Every operation that takes a count from 1 to 2 happens under that shard's +//! > lock. +//! +//! - The only way to obtain a handle to a table entry is [`Interner::intern`], +//! which holds at least the read lock while it clones. +//! - A sweep holds the **write** lock, so no `intern` can be mid-clone. +//! - [`Interned::clone`] takes no lock, but its caller already holds a handle, +//! so the count it raises is at least 2 — never a 1. +//! +//! A count of 1 observed under the write lock therefore means the table is the +//! only owner and no one else can reach the entry. `Arc` reasons about its own +//! count the same way; the relaxed load is ordered by the lock, since a clone +//! that released the read lock happens-before the sweeper's write lock. use std::{ alloc::{self, Layout}, @@ -42,7 +55,7 @@ use std::{ slice, str, sync::{ atomic::{AtomicUsize, Ordering}, - Mutex, MutexGuard, PoisonError, + PoisonError, RwLock, }, }; @@ -51,11 +64,11 @@ use hashbrown::HashTable; /// Interner entry: header followed by the string bytes in the same allocation. /// /// One allocation rather than a header plus a `Box` because interning is -/// allocation-bound — a refcounted interner re-allocates every entry that was -/// freed since the last use, and doubling the allocation count per entry is -/// directly visible in the resolver benchmarks. +/// allocation-bound, and doubling the allocation count per entry is directly +/// visible in the resolver benchmarks. #[repr(C)] struct Entry { + /// One for the table, plus one per live [`Interned`]. count: AtomicUsize, /// The caller's hash of the string. Stored so the handle stays one pointer /// wide and [`Interned::hash`] costs a load instead of a re-hash. @@ -84,8 +97,8 @@ impl Entry { (layout.pad_to_align(), offset) } - /// Allocate an entry holding `s`, with the refcount already at one for the - /// handle the caller is about to build. + /// Allocate an entry holding `s`, with the count already at two: one for the + /// table it is about to enter, one for the handle the caller gets back. fn alloc(s: &str, hash: u64) -> NonNull { let (layout, offset) = Self::layout(s.len()); // SAFETY: `layout` has non-zero size — the header alone is non-empty. @@ -100,7 +113,7 @@ impl Entry { // cannot overlap `s`, which lives in the caller's memory. unsafe { ptr.as_ptr().write(Self { - count: AtomicUsize::new(1), + count: AtomicUsize::new(2), hash, len: s.len(), }); @@ -116,7 +129,7 @@ impl Entry { /// # Safety /// /// `ptr` must come from [`Entry::alloc`], must not have been deallocated, - /// and must be unreachable — removed from the table with a zero refcount. + /// and must already be out of its shard with a count of one. unsafe fn dealloc(ptr: NonNull) { // SAFETY: the caller guarantees `ptr` is a live entry, so reading `len` // reproduces the layout it was allocated with. @@ -128,7 +141,7 @@ impl Entry { /// # Safety /// /// `ptr` must point at a live entry, and the returned reference must not - /// outlive the handle that keeps it alive. + /// outlive the handle or table slot that keeps it alive. #[inline] unsafe fn str<'a>(ptr: NonNull) -> &'a str { // SAFETY: the caller guarantees `ptr` is live. @@ -142,53 +155,253 @@ impl Entry { } } -/// Enough shards that four resolver threads rarely collide, matching what -/// `ustr` uses. Must stay a power of two — [`shard`] slices the index out of -/// the hash's top bits. -const SHARDS: usize = 64; -const SHARD_BITS: u32 = SHARDS.trailing_zeros(); -const _: () = assert!(SHARDS.is_power_of_two()); - /// `NonNull` is neither `Send` nor `Sync`, so it cannot go in a `static` /// directly. Entries are: their bytes are immutable and `count` is atomic. #[derive(Clone, Copy, PartialEq, Eq, Debug)] struct EntryPtr(NonNull); // SAFETY: see the type's doc comment — an entry is immutable apart from its -// atomic refcount, and freeing one requires the shard lock. +// atomic count, and freeing one requires the shard's write lock. unsafe impl Send for EntryPtr {} // SAFETY: as above. unsafe impl Sync for EntryPtr {} -type Shard = Mutex>; +/// Enough shards that a build's worth of resolver threads rarely collide, +/// matching what `ustr` uses. Must stay a power of two — [`shard_index`] slices +/// the index out of the hash's top bits. +const SHARDS: usize = 64; +const SHARD_BITS: u32 = SHARDS.trailing_zeros(); +const _: () = assert!(SHARDS.is_power_of_two()); + +/// Smallest table a shard bothers sweeping. Below this, walking the table costs +/// more than the handful of entries it could reclaim. +const MIN_SWEEP_THRESHOLD: usize = 64; -/// The process-wide interner. +struct Shard { + table: HashTable, + inserts_since_sweep: usize, +} + +impl Shard { + const fn new() -> Self { + Self { + table: HashTable::new(), + inserts_since_sweep: 0, + } + } + + /// Sweep once insertions since the last sweep reach half the table. Scaling + /// the threshold with the table is what keeps this amortized constant: a + /// sweep walks `len` entries and buys `len / 2` insertions, so the per-insert + /// cost stays flat however large the table grows. + fn threshold(&self) -> usize { + MIN_SWEEP_THRESHOLD.max(self.table.len() / 2) + } + + /// Remove every entry no handle refers to, returning them for the caller to + /// free once the lock is released. + fn sweep(&mut self) -> Vec { + self.inserts_since_sweep = 0; + self + .table + .extract_if(|&mut EntryPtr(entry)| { + // SAFETY: entries leave the table only here, under the write lock, so + // everything still reachable is live. + unsafe { entry.as_ref() }.count.load(Ordering::Acquire) == 1 + }) + .collect() + } +} + +/// A string interner. Distinct instances have distinct tables. /// +/// The crate uses one global instance; this is a type rather than a set of free +/// functions so that tests can work against an isolated interner instead of +/// racing each other through process-wide state. +pub struct Interner { + shards: [RwLock; SHARDS], +} + +impl Interner { + pub const fn new() -> Self { + Self { + shards: [const { RwLock::new(Shard::new()) }; SHARDS], + } + } + + /// Intern `s`, whose hash the caller has already computed. + /// + /// `hash` must be a deterministic function of `s` alone; two calls with the + /// same string and different hashes would create two entries for it and break + /// the one-entry-per-string guarantee that [`Interned::ptr_eq`] relies on. It + /// is not otherwise constrained — [`crate::UstrPath`] deliberately passes a + /// hash that folds equivalent Windows spellings together, which only makes + /// those spellings share a shard and a bucket. + pub fn intern(&self, s: &str, hash: u64) -> Interned { + let lock = &self.shards[shard_index(hash)]; + + // Hit path: a read lock, so threads interning paths that are already known + // — the common case once a build is warm — never wait on each other. + { + let shard = lock.read().unwrap_or_else(PoisonError::into_inner); + if let Some(ptr) = find(&shard.table, s, hash) { + return ptr; + } + } + + // Miss path. The read lock was released above, so re-check: another thread + // may have interned `s` in that window. + let mut shard = lock.write().unwrap_or_else(PoisonError::into_inner); + if let Some(ptr) = find(&shard.table, s, hash) { + return ptr; + } + + let ptr = Entry::alloc(s, hash); + shard + .table + .insert_unique(hash, EntryPtr(ptr), |&EntryPtr(entry)| { + // SAFETY: reachable table entries are live. + unsafe { entry.as_ref() }.hash + }); + shard.inserts_since_sweep += 1; + let doomed = if shard.inserts_since_sweep > shard.threshold() { + shard.sweep() + } else { + Vec::new() + }; + drop(shard); + + // Freeing outside the lock keeps the critical section to table work. + free_all(doomed); + Interned { ptr } + } + + /// Reclaim every entry no handle refers to, across all shards. + /// + /// Sweeps are otherwise driven by insertions, so a shard that has gone quiet + /// keeps its dead entries indefinitely. Call this to collect them anyway. + /// + /// Test-gated only because nothing in the crate calls it yet. The natural + /// caller is `Cache::clear()` — rspack runs it on every rebuild — which would + /// turn the bounded-delay reclamation into immediate reclamation. + #[cfg(test)] + pub fn sweep(&self) { + for lock in &self.shards { + let doomed = lock.write().unwrap_or_else(PoisonError::into_inner).sweep(); + free_all(doomed); + } + } + + /// How many entries the interner holds. Diagnostic and test use — the count + /// includes entries that are dead but not yet swept. + #[cfg(test)] + pub fn len(&self) -> usize { + self + .shards + .iter() + .map(|lock| { + let shard = lock.read().unwrap_or_else(PoisonError::into_inner); + let len = shard.table.len(); + drop(shard); + len + }) + .sum() + } +} + +impl Default for Interner { + fn default() -> Self { + Self::new() + } +} + +impl Drop for Interner { + /// Free what the table still owns. Without this, dropping an `Interner` + /// leaks every entry in it — the global one never drops, but tests build + /// their own, and a leak checker sees them. + /// + /// Entries a handle still refers to are deliberately leaked instead. An + /// [`Interned`] does not borrow from its `Interner`, so freeing those would + /// leave the holder with a dangling pointer; outliving the interner is a bug + /// at the call site, and leaking is the safe way to report it. + fn drop(&mut self) { + for lock in &mut self.shards { + let shard = lock.get_mut().unwrap_or_else(PoisonError::into_inner); + for EntryPtr(entry) in shard.table.drain() { + // SAFETY: `&mut self` means no other thread can reach these entries, + // and the count tells us whether any handle still can. + if unsafe { entry.as_ref() }.count.load(Ordering::Acquire) == 1 { + // SAFETY: table-only, and just drained out of the table. + unsafe { Entry::dealloc(entry) } + } else { + debug_assert!( + false, + "an Interned outlived its Interner; the handle is now dangling" + ); + } + } + } + } +} + +/// Look `s` up and take a reference to it. The caller must hold the shard lock. +#[inline] +fn find(table: &HashTable, s: &str, hash: u64) -> Option { + let &EntryPtr(ptr) = table.find(hash, |&EntryPtr(entry)| { + // SAFETY: entries leave the table only under the write lock, which the + // caller's lock excludes, so everything reachable here is live. + unsafe { Entry::str(entry) == s } + })?; + // SAFETY: as above. + unsafe { ptr.as_ref() } + .count + .fetch_add(1, Ordering::Relaxed); + Some(Interned { ptr }) +} + +/// # Panics +/// +/// Never; the entries come from a sweep, which only yields what it removed. +fn free_all(doomed: Vec) { + for EntryPtr(entry) in doomed { + // SAFETY: a sweep yields only entries it removed from its table under the + // write lock, each with a count of one — the table's own reference. + unsafe { Entry::dealloc(entry) } + } +} + /// Sharded by the **top** bits of the hash: `hashbrown` indexes buckets with /// the low bits, so sharding on those would give every entry in a shard the /// same bucket index. -static SHARD_TABLE: [Shard; SHARDS] = [const { Mutex::new(HashTable::new()) }; SHARDS]; - -fn shard(hash: u64) -> MutexGuard<'static, HashTable> { - let index = (hash >> (u64::BITS - SHARD_BITS)) as usize; - // A panic while holding a shard lock cannot leave the table inconsistent: - // the only calls made under it are hash-table operations and the allocator, - // neither of which unwinds partway through a mutation. Recovering beats - // poisoning every future intern of that shard. - SHARD_TABLE[index] - .lock() - .unwrap_or_else(PoisonError::into_inner) +#[inline] +fn shard_index(hash: u64) -> usize { + (hash >> (u64::BITS - SHARD_BITS)) as usize +} + +/// The process-wide interner. +static GLOBAL: Interner = Interner::new(); + +/// Intern `s` into the process-wide interner. See [`Interner::intern`]. +#[inline] +pub fn intern(s: &str, hash: u64) -> Interned { + GLOBAL.intern(s, hash) +} + +/// Entry count of the process-wide interner. See [`Interner::len`]. +#[cfg(test)] +pub fn live_entries() -> usize { + GLOBAL.len() } /// A refcounted handle to a globally interned string. /// -/// One pointer wide. Not `Copy`: the refcount has to be maintained. +/// One pointer wide. Not `Copy`: the count has to be maintained. pub struct Interned { ptr: NonNull, } -// SAFETY: `Entry` is immutable apart from `count`, which is atomic, and every -// operation that can free it happens under the shard lock (see module docs). +// SAFETY: `Entry` is immutable apart from `count`, which is atomic, and an +// entry is freed only under its shard's write lock (see module docs). unsafe impl Send for Interned {} // SAFETY: as above — sharing `&Interned` only exposes immutable string bytes. unsafe impl Sync for Interned {} @@ -217,8 +430,8 @@ impl Interned { self.ptr == other.ptr } - /// How many handles currently share this entry. Test and diagnostic use — - /// the count is a snapshot and can change concurrently. + /// One for the table plus one per live handle. Test and diagnostic use — the + /// count is a snapshot and can change concurrently. #[cfg(test)] pub(crate) fn refcount(&self) -> usize { // SAFETY: `self` holds a reference, so the entry is live. @@ -226,64 +439,11 @@ impl Interned { } } -/// How many entries the interner currently holds, across all shards. -/// -/// Diagnostic only, and inherently a snapshot: the interner is process-wide, so -/// this counts every string every thread is holding, not just the caller's. -#[cfg(test)] -pub fn live_entries() -> usize { - SHARD_TABLE - .iter() - .map(|shard| { - let table = shard.lock().unwrap_or_else(PoisonError::into_inner); - let len = table.len(); - drop(table); - len - }) - .sum() -} - -/// Intern `s`, whose hash the caller has already computed. -/// -/// `hash` must be a deterministic function of `s` alone; two calls with the -/// same string and different hashes would create two entries for it and break -/// the one-entry-per-string guarantee that [`Interned::ptr_eq`] relies on. It -/// is not otherwise constrained — [`crate::UstrPath`] deliberately passes a -/// hash that folds equivalent Windows spellings together, which only makes -/// those spellings share a shard and a bucket. -pub fn intern(s: &str, hash: u64) -> Interned { - let mut table = shard(hash); - if let Some(&EntryPtr(ptr)) = table.find(hash, |&EntryPtr(entry)| { - // SAFETY: an entry is removed from the table before it is freed, and both - // happen under this lock, so everything reachable here is alive. - unsafe { Entry::str(entry) == s } - }) { - // The old value may be 0 — a dropper claimed this entry and is waiting for - // the lock. Resurrecting it is exactly right: it will see a non-zero count - // and leave the entry alone. Relaxed is enough because the lock we hold - // orders this against that check. - // SAFETY: the entry is in the table, so it is alive. - unsafe { ptr.as_ref() } - .count - .fetch_add(1, Ordering::Relaxed); - return Interned { ptr }; - } - let ptr = Entry::alloc(s, hash); - table.insert_unique(hash, EntryPtr(ptr), |&EntryPtr(entry)| { - // SAFETY: reachable table entries are alive (see above). - unsafe { entry.as_ref() }.hash - }); - // The handle already owns the entry's first reference, so releasing the - // shard before building it is safe and keeps the critical section minimal. - drop(table); - Interned { ptr } -} - impl Clone for Interned { #[inline] fn clone(&self) -> Self { - // No lock: `self` is a live handle, so the count is at least one both - // before and after. See the module's concurrency notes. + // No lock: the caller holds a handle, so this raises a count of at least + // two, never the 1 -> 2 transition a sweep must not miss. // SAFETY: `self` holds a reference, so the entry is live. unsafe { self.ptr.as_ref() } .count @@ -293,45 +453,15 @@ impl Clone for Interned { } impl Drop for Interned { + #[inline] fn drop(&mut self) { - // Read the hash while we still own a reference. Past the decrement below, - // `self.ptr` is only an address — dereferencing it is unsound until the - // shard lock re-establishes that the entry is still alive. - let hash = self.hash(); - // SAFETY: `self` still holds a reference, so the entry is live. - if unsafe { self.ptr.as_ref() } + // The table's own reference keeps this from reaching zero, so there is + // nothing to free and nothing to lock. `Release` pairs with the acquire the + // sweeper gets from the shard's write lock. + // SAFETY: `self` holds a reference, so the entry is live. + unsafe { self.ptr.as_ref() } .count - .fetch_sub(1, Ordering::AcqRel) - != 1 - { - // The overwhelming majority of drops land here — a temporary handle - // going away while the cache still holds the path. Keeping this path - // lock-free is what stops the shards from serializing resolver threads. - return; - } - let mut table = shard(hash); - // Address comparison only: a concurrent dropper may already have freed - // this entry, so it must not be dereferenced before it is found. - let Ok(entry) = table.find_entry(hash, |&EntryPtr(candidate)| candidate == self.ptr) else { - // Gone from the table, which only happens after another thread claimed - // the free. Nothing left to do — and nothing left to read. - return; - }; - // Found under the lock, and entries are freed only after being removed - // under this same lock, so the entry is alive again for as long as we hold - // it. - // SAFETY: as above. - if unsafe { self.ptr.as_ref() }.count.load(Ordering::Acquire) != 0 { - // Resurrected: an `intern` found this entry between our decrement and - // our lock, and now owns it. Leaving it in place is correct — the count - // reflects exactly the handles that exist. - return; - } - entry.remove(); - drop(table); - // SAFETY: zero refcount and out of the table, so no handle can exist and - // no lookup can reach it. - unsafe { Entry::dealloc(self.ptr) } + .fetch_sub(1, Ordering::Release); } } @@ -339,11 +469,8 @@ impl Drop for Interned { mod tests { use std::{sync::Arc, thread}; - use super::{intern, Interned}; + use super::{Interned, Interner}; - /// The interner is process-wide and shared with every other test running in - /// parallel, so assertions here use strings no other test interns and only - /// ever check one entry's own state. fn h(s: &str) -> u64 { use std::hash::Hasher as _; let mut hasher = rustc_hash::FxHasher::default(); @@ -351,31 +478,36 @@ mod tests { hasher.finish() } - fn get(s: &str) -> Interned { - intern(s, h(s)) + /// Tests use their own interner rather than the global one, so they cannot + /// perturb each other's counts through process-wide state. + fn get(interner: &Interner, s: &str) -> Interned { + interner.intern(s, h(s)) } #[test] fn equal_strings_share_one_entry() { - let a = get("interner::equal_strings/a/b/c.js"); - let b = get("interner::equal_strings/a/b/c.js"); + let interner = Interner::new(); + let a = get(&interner, "/a/b/c.js"); + let b = get(&interner, "/a/b/c.js"); assert!(a.ptr_eq(&b)); - assert_eq!(a.as_str(), "interner::equal_strings/a/b/c.js"); + assert_eq!(a.as_str(), "/a/b/c.js"); + assert_eq!(interner.len(), 1); } #[test] fn different_strings_get_different_entries() { - let a = get("interner::different/a.js"); - let b = get("interner::different/b.js"); - assert!(!a.ptr_eq(&b)); + let interner = Interner::new(); + assert!(!get(&interner, "/a.js").ptr_eq(&get(&interner, "/b.js"))); + assert_eq!(interner.len(), 2); } #[test] fn the_empty_string_round_trips() { // Zero-length payload is the one case where the entry is header-only. - let a = get(""); + let interner = Interner::new(); + let a = get(&interner, ""); assert_eq!(a.as_str(), ""); - assert!(a.ptr_eq(&get(""))); + assert!(a.ptr_eq(&get(&interner, ""))); } #[test] @@ -383,118 +515,113 @@ mod tests { // The interner must not re-derive the hash: `UstrPath` relies on getting // back exactly what it computed, including a Windows-folded value that // does not match the stored bytes. - let interned = intern("interner::verbatim/x.js", 0xDEAD_BEEF_1234_5678); - assert_eq!(interned.hash(), 0xDEAD_BEEF_1234_5678); + let interner = Interner::new(); + assert_eq!( + interner.intern("/x.js", 0xDEAD_BEEF_1234_5678).hash(), + 0xDEAD_BEEF_1234_5678 + ); } #[test] fn colliding_hashes_stay_distinct_entries() { // Windows folding hands equal hashes to different strings on purpose, so // the table must disambiguate by bytes rather than trusting the hash. - let a = intern("interner::collide/one", 0x5555_5555_5555_5555); - let b = intern("interner::collide/two", 0x5555_5555_5555_5555); + let interner = Interner::new(); + let a = interner.intern("/one", 0x5555_5555_5555_5555); + let b = interner.intern("/two", 0x5555_5555_5555_5555); assert!(!a.ptr_eq(&b)); - assert_eq!(a.as_str(), "interner::collide/one"); - assert_eq!(b.as_str(), "interner::collide/two"); + assert_eq!(a.as_str(), "/one"); + assert_eq!(b.as_str(), "/two"); } #[test] - fn refcount_tracks_handles() { - let a = get("interner::refcount/a.js"); - assert_eq!(a.refcount(), 1); + fn refcount_counts_the_table_plus_every_handle() { + let interner = Interner::new(); + let a = get(&interner, "/a.js"); + assert_eq!(a.refcount(), 2, "the table holds one reference of its own"); let b = a.clone(); - assert_eq!(a.refcount(), 2); + assert_eq!(a.refcount(), 3); drop(b); - assert_eq!(a.refcount(), 1); + assert_eq!(a.refcount(), 2); } #[test] - fn dropping_the_last_handle_frees_the_entry() { - let first = get("interner::freed/a.js"); - let address = first.as_str().as_ptr(); - drop(first); - - // Re-interning after the entry was freed must produce a working handle. - // (It may or may not reuse `address` — the allocator decides — so the - // assertion is on the contents, not the pointer.) - let second = get("interner::freed/a.js"); - assert_eq!(second.as_str(), "interner::freed/a.js"); + fn sweeping_reclaims_entries_no_handle_refers_to() { + let interner = Interner::new(); + drop(get(&interner, "/dead.js")); + let alive = get(&interner, "/alive.js"); + assert_eq!(interner.len(), 2, "dropping a handle does not remove it"); + + interner.sweep(); + + assert_eq!(interner.len(), 1); assert_eq!( - second.refcount(), - 1, - "the freed entry must not have lingered" + alive.as_str(), + "/alive.js", + "a held entry must survive the sweep" ); - let _ = address; } #[test] - fn concurrent_zero_crossings_are_sound() { - // Aimed squarely at the two windows the lock-free drop path opens: - // resurrection, and two threads both claiming the free. Every handle here - // is created and dropped immediately over a key space of two, so threads - // are almost always racing on an entry that is at or near zero — the state - // a version that freed unconditionally after `fetch_sub` would corrupt. - // - // Reads `as_str()` after the clone specifically so a use-after-free lands - // on the string bytes, where ASan or Miri will catch it. - let barrier = Arc::new(std::sync::Barrier::new(8)); - let threads: Vec<_> = (0..8) - .map(|_| { - let barrier = Arc::clone(&barrier); - thread::spawn(move || { - barrier.wait(); - for i in 0..5000 { - let key = format!("interner::zero-cross/{}", i % 2); - let handle = get(&key); - assert_eq!(handle.as_str(), key); - drop(handle); - } - }) - }) - .collect(); - for thread in threads { - thread.join().expect("zero-crossing thread panicked"); + fn a_held_entry_survives_repeated_sweeps() { + let interner = Interner::new(); + let held = get(&interner, "/held.js"); + for _ in 0..10 { + interner.sweep(); + assert!(held.ptr_eq(&get(&interner, "/held.js"))); } + assert_eq!(held.as_str(), "/held.js"); + } - // Every handle above is gone, so nothing may have leaked a live entry. - for i in 0..2 { - let key = format!("interner::zero-cross/{i}"); - let fresh = get(&key); - assert_eq!( - fresh.refcount(), - 1, - "{key} outlived every handle that referenced it" - ); + #[test] + fn one_shot_strings_do_not_grow_the_table_without_bound() { + // The property the insert-driven sweep exists for: a build that interns a + // hundred thousand paths and immediately discards them must not accumulate + // them. The bound is roughly SHARDS * MIN_SWEEP_THRESHOLD. + let interner = Interner::new(); + for i in 0..100_000 { + drop(get(&interner, &format!("/transient/{i}.js"))); } + let len = interner.len(); + assert!(len < 8_192, "table grew to {len}, which is not bounded"); } #[test] - fn concurrent_intern_and_drop_of_one_string_is_sound() { - // Drives the race the shard lock exists for: threads repeatedly take the - // last handle to zero while others intern the same string. Under Miri or - // ASan a resurrection bug surfaces here; without them it still catches - // double-free and lost-entry bugs. + fn concurrent_intern_and_drop_is_sound() { + // Threads race interning and discarding a small key space while sweeps fire + // underneath them, driven by the one-shot strings. A sweep that freed an + // entry another thread had just cloned surfaces here under ASan or Miri. + let interner = Arc::new(Interner::new()); let barrier = Arc::new(std::sync::Barrier::new(8)); let threads: Vec<_> = (0..8) .map(|t| { + let interner = Arc::clone(&interner); let barrier = Arc::clone(&barrier); thread::spawn(move || { barrier.wait(); - for i in 0..2000 { - let key = format!("interner::race/{}", i % 4); - let a = get(&key); - assert_eq!(a.as_str(), key); - let b = a.clone(); - assert!(a.ptr_eq(&b)); - drop(a); - assert_eq!(b.as_str(), key); + for i in 0..5000 { + let shared = format!("/shared/{}", i % 4); + let handle = get(&interner, &shared); + assert_eq!(handle.as_str(), shared); + let cloned = handle.clone(); + drop(handle); + assert_eq!(cloned.as_str(), shared); + + let once = format!("/once/{t}/{i}"); + assert_eq!(get(&interner, &once).as_str(), once); } - t }) }) .collect(); for thread in threads { thread.join().expect("interner race thread panicked"); } + + interner.sweep(); + assert_eq!( + interner.len(), + 0, + "every handle is gone, so a full sweep must empty the table" + ); } } From 1ff65c93f40bb7ec724e74dca606e9b6c21e23d0 Mon Sep 17 00:00:00 2001 From: pshu Date: Wed, 5 Aug 2026 11:46:55 +0800 Subject: [PATCH 25/26] test(interner): pin the refcount an insert-triggered sweep depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sweep can fire from inside the same `intern` call that inserted a new entry, and at that moment the entry has no handle — the caller's `Interned` is built after the sweep returns. What keeps the sweep from collecting it is `Entry::alloc` starting the count at two: one for the table, one pre-paid for the handle on its way out. That is easy to undo. Starting the count at one and incrementing before the return reads as the more symmetric shape, and it makes `intern` free the very string it hands back. Nothing failed on that mistake except a heap corruption trap, far from the line that caused it. The test watches for the table shrinking to spot which insert swept, then asserts that insert's own entry survived and was not silently re-allocated. It also asserts a sweep happened at all, so it cannot pass by never reaching the case. --- src/interner.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/interner.rs b/src/interner.rs index 11f0296d..6e026386 100644 --- a/src/interner.rs +++ b/src/interner.rs @@ -586,6 +586,46 @@ mod tests { assert!(len < 8_192, "table grew to {len}, which is not bounded"); } + #[test] + fn the_insert_that_triggers_a_sweep_keeps_its_own_entry() { + // A sweep fires from inside the very call that inserted a new entry, and + // that entry has no handle yet — the caller's `Interned` is built after the + // sweep returns. What saves it is `Entry::alloc` starting the count at two: + // one for the table, one pre-paid for the handle about to be returned. Get + // that wrong and an insert would free the string it just returned. + let interner = Interner::new(); + let mut before = 0; + let mut sweeps = 0; + + for i in 0..20_000 { + let key = format!("/trigger/{i}.js"); + let handle = get(&interner, &key); + + // A shrinking table is the only externally visible sign that this + // particular insert swept. + let after = interner.len(); + if after < before { + sweeps += 1; + assert_eq!( + handle.as_str(), + key, + "the insert that swept lost the entry it had just returned" + ); + assert!( + get(&interner, &key).ptr_eq(&handle), + "the swept-through entry was evicted and re-allocated" + ); + } + before = after; + drop(handle); + } + + assert!( + sweeps > 0, + "no insert ever tripped a sweep, so this asserted nothing" + ); + } + #[test] fn concurrent_intern_and_drop_is_sound() { // Threads race interning and discarding a small key space while sweeps fire From 704b58c25a56867d83311f525d2ad261abbb2f0d Mon Sep 17 00:00:00 2001 From: pshu Date: Wed, 5 Aug 2026 12:08:48 +0800 Subject: [PATCH 26/26] refactor(path): rename UstrPath back to ResolverPath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `UstrPath` was named after its backing store, and that store has now changed three times — ustr, then internment, then this crate's own interner. The name went stale on the first change and is simply wrong after the last one, since no `ustr` is involved anywhere. `ResolverPath` names the role instead: the path type this crate hands out. Whatever the interner does next, that stays true. It is also the name main uses for the same slot, so downstream keeps the identifier it already has. Reusing the old name is safe here because the contract carried over. Equality was raw bytes on unix and `Path` components on Windows; interning stores one entry per distinct string, so pointer equality means the same thing on unix, and Windows still folds through `Path`. What genuinely changed — no longer `Copy`, no `Ord`, `as_str()` borrowing rather than `'static` — all fails to compile rather than passing quietly. Mechanical throughout: `ToUstrPath` -> `ToResolverPath`, `UstrPathSet` -> `ResolverPathSet`, `to_ustr_path` -> `to_resolver_path`, `src/ustr_path.rs` -> `src/resolver_path.rs`. Comments that named `ustr`'s bin mutex now name the shard lock that replaced it. `src/interner.rs` still mentions `ustr` where it means the crate — as a rejected alternative and as the precedent for the shard count. --- Cargo.toml | 2 +- src/cache.rs | 56 +++--- src/context.rs | 14 +- src/interner.rs | 6 +- src/lib.rs | 37 ++-- src/package_json/simd.rs | 10 +- src/resolution.rs | 14 +- src/{ustr_path.rs => resolver_path.rs} | 211 ++++++++++++----------- src/tests/dependencies.rs | 10 +- src/tests/extensions.rs | 18 +- src/tests/incorrect_description_file.rs | 10 +- src/tests/interning.rs | 6 +- src/tests/missing.rs | 6 +- src/tests/package_json.rs | 10 +- src/tests/pnp.rs | 4 +- src/tests/symlink.rs | 4 +- src/tests/tsconfig_project_references.rs | 10 +- src/tsconfig.rs | 8 +- 18 files changed, 228 insertions(+), 208 deletions(-) rename src/{ustr_path.rs => resolver_path.rs} (78%) diff --git a/Cargo.toml b/Cargo.toml index 320c6fd8..d93160c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,7 +93,7 @@ rustc-hash = { version = "2.1.2", default-features = false, features = thiserror = "2.0.18" # Backing table for `src/interner.rs`. `HashTable` is the piece that off-the- # shelf interners cannot offer: it looks up by a caller-supplied hash, so the -# interner reuses the `FxHash` `UstrPath` already computed instead of hashing +# interner reuses the `FxHash` `ResolverPath` already computed instead of hashing # every path a second time. Version tracks what `indexmap` resolves to, so no # extra copy of hashbrown enters the graph. hashbrown = { version = "0.17.0", default-features = false, features = ["inline-more"] } diff --git a/src/cache.rs b/src/cache.rs index da39f21a..80e8dcf1 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -17,7 +17,7 @@ use tokio::sync::OnceCell as OnceLock; use crate::{ context::ResolveContext as Ctx, package_json::{off_to_location, PackageJson}, - ustr_path::{ToUstrPath, UstrPath}, + resolver_path::{ResolverPath, ToResolverPath}, FileMetadata, FileSystem, JSONError, ResolveError, ResolveOptions, TsConfig, }; @@ -25,7 +25,7 @@ use crate::{ pub struct Cache { pub(crate) fs: Fs, paths: DashSet>, - tsconfigs: DashMap, BuildHasherDefault>, + tsconfigs: DashMap, BuildHasherDefault>, } impl Cache { @@ -60,12 +60,12 @@ impl Cache { /// `path` must already be interned by the caller. `config_file` is constant /// per resolver but this is re-entered several times per resolve, so /// interning here on every hit would serialize every thread on the same - /// `ustr` bin mutex for a hash that never changes; see `ResolverGeneric`'s + /// shard lock for a hash that never changes; see `ResolverGeneric`'s /// `tsconfig_config_file` field, computed once at construction. pub async fn tsconfig( &self, root: bool, - path: UstrPath, + path: ResolverPath, callback: F, // callback for modifying tsconfig with `extends` ) -> Result, ResolveError> where @@ -166,25 +166,25 @@ pub struct CachedPathImpl { path: Box, parent: Option, meta: OnceLock>, - canonicalized: OnceLock>, + canonicalized: OnceLock>, node_modules: OnceLock>, package_json: OnceLock>>, /// Memoized interned form of `self.path`. Without it every `is_file` / /// `is_dir` dependency push would re-enter the interner (bin lock + probe). - dep_path: std::sync::OnceLock, + dep_path: std::sync::OnceLock, /// Memoized `/node_modules`. `cached_node_modules` replays a /// missing-dependency push on every cache hit where `node_modules` is /// absent, which is most directory levels during an upward walk. - node_modules_dep_path: std::sync::OnceLock, + node_modules_dep_path: std::sync::OnceLock, /// Memoized `/package.json` for the `missing_dependencies` push /// that fires on every `package_json` cache-hit `None` (~97% of /// `package_json` calls in dep-tracking workloads). - package_json_dep_path: std::sync::OnceLock, + package_json_dep_path: std::sync::OnceLock, } -impl ToUstrPath for CachedPathImpl { +impl ToResolverPath for CachedPathImpl { #[inline] - fn to_ustr_path(&self) -> UstrPath { + fn to_resolver_path(&self) -> ResolverPath { self.dep_path() } } @@ -205,26 +205,26 @@ impl CachedPathImpl { } } - fn dep_path(&self) -> UstrPath { + fn dep_path(&self) -> ResolverPath { self .dep_path - .get_or_init(|| self.path.to_ustr_path()) + .get_or_init(|| self.path.to_resolver_path()) .clone() } - fn node_modules_dep_path(&self) -> UstrPath { + fn node_modules_dep_path(&self) -> ResolverPath { self .node_modules_dep_path - .get_or_init(|| self.path.join("node_modules").to_ustr_path()) + .get_or_init(|| self.path.join("node_modules").to_resolver_path()) .clone() } /// Without this cache, each `None` cache-hit on `package_json` would /// re-`join` + re-intern on every push. - fn package_json_dep_path(&self) -> UstrPath { + fn package_json_dep_path(&self) -> ResolverPath { self .package_json_dep_path - .get_or_init(|| self.path.join("package.json").to_ustr_path()) + .get_or_init(|| self.path.join("package.json").to_resolver_path()) .clone() } @@ -272,7 +272,7 @@ impl CachedPathImpl { ) } - pub async fn realpath(&self, fs: &Fs) -> io::Result { + pub async fn realpath(&self, fs: &Fs) -> io::Result { // Cache hit: avoid the heap-allocated `Box::pin` for the cache-miss state machine // by returning before delegating to the boxed recursive helper. Both arms are a // refcount bump — neither allocates or re-enters the interner. @@ -285,7 +285,7 @@ impl CachedPathImpl { fn realpath_uncached<'a, Fs: FileSystem + Send + Sync>( &'a self, fs: &'a Fs, - ) -> BoxFuture<'a, io::Result> { + ) -> BoxFuture<'a, io::Result> { Box::pin(async move { self .canonicalized @@ -298,7 +298,7 @@ impl CachedPathImpl { return fs .canonicalize(self.path.as_std_path()) .await - .map(|path| Some(path.to_ustr_path())); + .map(|path| Some(path.to_resolver_path())); } if let Some(parent) = self.parent() { // Reuse the parent's realpath as a mutable buffer for the final @@ -314,7 +314,7 @@ impl CachedPathImpl { } _ => {} } - return Ok(Some(real_path.to_ustr_path())); + return Ok(Some(real_path.to_resolver_path())); } Ok(None) }) @@ -421,12 +421,16 @@ impl CachedPathImpl { return Ok(None); }; let real_path = if options.symlinks { - self.realpath(fs).await?.join("package.json").to_ustr_path() + self + .realpath(fs) + .await? + .join("package.json") + .to_resolver_path() } else { - package_json_path.to_ustr_path() + package_json_path.to_resolver_path() }; match PackageJson::parse( - package_json_path.to_ustr_path(), + package_json_path.to_resolver_path(), real_path, package_json_string, ) { @@ -573,8 +577,8 @@ mod tests { let cache = Cache::new(FileSystemOs::default()); let cached = cache.value(Utf8Path::new("/a/b/c.js")); - let first = cached.to_ustr_path(); - let second = cached.to_ustr_path(); + let first = cached.to_resolver_path(); + let second = cached.to_resolver_path(); assert_eq!(first.as_str().as_ptr(), second.as_str().as_ptr()); // Memoized: the second call must not have gone through the interner at all. assert!(cached.dep_path.get().is_some()); @@ -626,7 +630,7 @@ mod tests { let unique = Utf8Path::new("/only/this/test/interns/this/exact/path.js"); let cached = cache.value(unique); - let handle = cached.to_ustr_path(); + let handle = cached.to_resolver_path(); drop(cached); let held = handle.refcount(); diff --git a/src/context.rs b/src/context.rs index eab9d215..84a0dcf5 100644 --- a/src/context.rs +++ b/src/context.rs @@ -2,7 +2,7 @@ use std::ops::{Deref, DerefMut}; use crate::{ error::ResolveError, - ustr_path::{ToUstrPath, UstrPath}, + resolver_path::{ResolverPath, ToResolverPath}, }; #[derive(Debug, Default, Clone)] @@ -17,10 +17,10 @@ pub struct ResolveContextImpl { pub fragment: Option, /// Files that were found on file system - pub file_dependencies: Option>, + pub file_dependencies: Option>, /// Files that were not found on file system - pub missing_dependencies: Option>, + pub missing_dependencies: Option>, /// The current resolving alias for bailing recursion alias. pub resolving_alias: Option, @@ -68,15 +68,15 @@ impl ResolveContext { // would be evaluated (and interned) before the `if let` ever runs, silently // paying that cost on every call even when `resolve()` was invoked without a // context and these fields are `None`. - pub fn add_file_dependency(&mut self, dep: &P) { + pub fn add_file_dependency(&mut self, dep: &P) { if let Some(deps) = &mut self.file_dependencies { - deps.push(dep.to_ustr_path()); + deps.push(dep.to_resolver_path()); } } - pub fn add_missing_dependency(&mut self, dep: &P) { + pub fn add_missing_dependency(&mut self, dep: &P) { if let Some(deps) = &mut self.missing_dependencies { - deps.push(dep.to_ustr_path()); + deps.push(dep.to_resolver_path()); } } diff --git a/src/interner.rs b/src/interner.rs index 6e026386..b032fbcc 100644 --- a/src/interner.rs +++ b/src/interner.rs @@ -1,7 +1,7 @@ //! A refcounted, sharded global string interner that reclaims its own entries. //! //! Exists because the two obvious off-the-shelf choices each fail one half of -//! what [`crate::UstrPath`] needs: +//! what [`crate::ResolverPath`] needs: //! //! - `ustr` never frees, so a dev server's RSS climbs monotonically across //! rebuilds. @@ -234,7 +234,7 @@ impl Interner { /// `hash` must be a deterministic function of `s` alone; two calls with the /// same string and different hashes would create two entries for it and break /// the one-entry-per-string guarantee that [`Interned::ptr_eq`] relies on. It - /// is not otherwise constrained — [`crate::UstrPath`] deliberately passes a + /// is not otherwise constrained — [`crate::ResolverPath`] deliberately passes a /// hash that folds equivalent Windows spellings together, which only makes /// those spellings share a shard and a bucket. pub fn intern(&self, s: &str, hash: u64) -> Interned { @@ -512,7 +512,7 @@ mod tests { #[test] fn hash_is_returned_verbatim() { - // The interner must not re-derive the hash: `UstrPath` relies on getting + // The interner must not re-derive the hash: `ResolverPath` relies on getting // back exactly what it computed, including a Windows-folded value that // does not match the stored bytes. let interner = Interner::new(); diff --git a/src/lib.rs b/src/lib.rs index 6f576f0b..d19c43a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,9 +58,9 @@ mod options; mod package_json; mod path; mod resolution; +mod resolver_path; mod specifier; mod tsconfig; -mod ustr_path; #[cfg(test)] mod tests; @@ -97,7 +97,7 @@ pub use crate::{ }, package_json::{JSONValue, ModuleType, PackageJson}, resolution::Resolution, - ustr_path::{IdentityHasher, ToUstrPath, UstrPath, UstrPathSet}, + resolver_path::{IdentityHasher, ResolverPath, ResolverPathSet, ToResolverPath}, }; type ResolveResult = Result, ResolveError>; @@ -106,10 +106,10 @@ type ResolveResult = Result, ResolveError>; #[derive(Debug, Default, Clone)] pub struct ResolveContext { /// Files that were found on file system - pub file_dependencies: UstrPathSet, + pub file_dependencies: ResolverPathSet, /// Dependencies that were not found on file system - pub missing_dependencies: UstrPathSet, + pub missing_dependencies: ResolverPathSet, } /// Resolver with the current operating system as the file system @@ -126,10 +126,10 @@ pub struct ResolverGeneric { /// `options.tsconfig.config_file` interned once at construction, since it is /// constant per resolver but looked up on every `require_without_parse` entry /// into `load_tsconfig_paths`. Interning it per-call would serialize every - /// thread on the same `ustr` bin mutex for a hash that never changes. - tsconfig_config_file: Option, + /// thread on the same interner shard for a hash that never changes. + tsconfig_config_file: Option, #[cfg(feature = "yarn_pnp")] - pnp_manifest: Arc>, + pnp_manifest: Arc>, /// Paths that have been searched and confirmed to have no `.pnp.cjs` reachable by filesystem walk. #[cfg(feature = "yarn_pnp")] pnp_no_manifest_cache: Arc>, @@ -157,7 +157,7 @@ impl ResolverGeneric { let tsconfig_config_file = options .tsconfig .as_ref() - .map(|t| t.config_file.to_ustr_path()); + .map(|t| t.config_file.to_resolver_path()); Self { options, cache: Arc::new(Cache::new(Fs::default())), @@ -181,7 +181,7 @@ impl ResolverGeneric { let tsconfig_config_file = options .tsconfig .as_ref() - .map(|t| t.config_file.to_ustr_path()); + .map(|t| t.config_file.to_resolver_path()); Self { options, cache: Arc::new(Cache::new(file_system)), @@ -205,7 +205,7 @@ impl ResolverGeneric { let tsconfig_config_file = options .tsconfig .as_ref() - .map(|t| t.config_file.to_ustr_path()); + .map(|t| t.config_file.to_resolver_path()); Self { options, cache: Arc::clone(&self.cache), @@ -771,7 +771,7 @@ impl ResolverGeneric { &self, cached_path: &CachedPath, ctx: &mut Ctx, - ) -> Result { + ) -> Result { if self.options.symlinks { cached_path .realpath(&self.cache.fs) @@ -782,7 +782,7 @@ impl ResolverGeneric { }) .map_err(ResolveError::from) } else { - Ok(cached_path.to_ustr_path()) + Ok(cached_path.to_resolver_path()) } } @@ -1024,7 +1024,10 @@ impl ResolverGeneric { #[cfg(feature = "yarn_pnp")] #[cfg_attr(feature = "enable_instrument", tracing::instrument(level=tracing::Level::DEBUG, skip_all, fields(path = cached_path.path().as_str())))] - fn find_pnp_manifest(&self, cached_path: &CachedPath) -> Option> { + fn find_pnp_manifest( + &self, + cached_path: &CachedPath, + ) -> Option> { // 1. Already have a manifest → return it (covers global cache paths too) if let Some(manifest) = self.pnp_manifest.load_full() { return Some(manifest); @@ -1053,7 +1056,7 @@ impl ResolverGeneric { tracing::debug!("use manifest path: {:?}", manifest_path); let manifest = pnp::load_pnp_manifest(&manifest_path).ok()?; - let manifest = Arc::new((manifest_path.to_ustr_path(), manifest)); + let manifest = Arc::new((manifest_path.to_resolver_path(), manifest)); let previous = self .pnp_manifest @@ -1538,7 +1541,7 @@ impl ResolverGeneric { fn load_tsconfig<'a>( &'a self, root: bool, - path: UstrPath, + path: ResolverPath, references: &'a TsconfigReferences, ) -> BoxFuture<'a, Result, ResolveError>> { let fut = async move { @@ -1611,7 +1614,7 @@ impl ResolverGeneric { .cache .tsconfig( /* root */ true, - reference_tsconfig_path.to_ustr_path(), + reference_tsconfig_path.to_resolver_path(), |mut reference_tsconfig| { let current_path = current_path.clone(); async move { @@ -1688,7 +1691,7 @@ impl ResolverGeneric { let extended_tsconfig = self .load_tsconfig( /* root */ false, - extended_tsconfig_path.to_ustr_path(), + extended_tsconfig_path.to_resolver_path(), &TsconfigReferences::Disabled, ) .await?; diff --git a/src/package_json/simd.rs b/src/package_json/simd.rs index cc528ebe..a4488cbc 100644 --- a/src/package_json/simd.rs +++ b/src/package_json/simd.rs @@ -16,7 +16,7 @@ use simd_json::{ to_borrowed_value, BorrowedValue, Error as SimdParseError, ObjectHasher, }; -use crate::{path::PathUtil, ResolveError, UstrPath}; +use crate::{path::PathUtil, ResolveError, ResolverPath}; pub type JSONMap<'a> = simd_json::borrowed::Object<'a>; @@ -134,10 +134,10 @@ impl Default for JSONCell { #[derive(Debug, Default)] pub struct PackageJson { /// Path to `package.json`. Contains the `package.json` filename. - pub path: UstrPath, + pub path: ResolverPath, /// Realpath to `package.json`. Contains the `package.json` filename. - pub realpath: UstrPath, + pub realpath: ResolverPath, /// The "name" field defines your package's name. /// The "name" field can be used in addition to the "exports" field to self-reference a package using its name. @@ -190,8 +190,8 @@ impl PackageJson { /// # Panics /// # Errors pub(crate) fn parse( - path: UstrPath, - realpath: UstrPath, + path: ResolverPath, + realpath: ResolverPath, json: Vec, ) -> Result { if json.starts_with(&BOM) { diff --git a/src/resolution.rs b/src/resolution.rs index b9f255ba..cac13138 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -4,12 +4,12 @@ use std::{ sync::Arc, }; -use crate::{package_json::PackageJson, ustr_path::UstrPath}; +use crate::{package_json::PackageJson, resolver_path::ResolverPath}; /// The final path resolution with optional `?query` and `#fragment` #[derive(Clone)] pub struct Resolution { - pub(crate) path: UstrPath, + pub(crate) path: ResolverPath, /// path query `?query`, contains `?`. pub(crate) query: Option, @@ -48,7 +48,7 @@ impl Resolution { /// /// Zero-copy: hand this to a downstream store instead of `path()` to avoid /// re-allocating and re-hashing the string. - pub fn ustr_path(&self) -> UstrPath { + pub fn resolver_path(&self) -> ResolverPath { self.path.clone() } @@ -101,17 +101,17 @@ async fn test() { } #[tokio::test] -async fn ustr_path_accessor_is_the_same_pointer_as_the_stored_path() { +async fn resolver_path_accessor_is_the_same_pointer_as_the_stored_path() { let resolution = Resolution { path: "foo".into(), query: None, fragment: None, package_json: None, }; - assert_eq!(resolution.ustr_path().as_str(), "foo"); + assert_eq!(resolution.resolver_path().as_str(), "foo"); assert_eq!( - resolution.ustr_path().as_str().as_ptr(), - UstrPath::new("foo").as_str().as_ptr() + resolution.resolver_path().as_str().as_ptr(), + ResolverPath::new("foo").as_str().as_ptr() ); // The legacy accessors keep their signatures. assert_eq!(resolution.path(), Path::new("foo")); diff --git a/src/ustr_path.rs b/src/resolver_path.rs similarity index 78% rename from src/ustr_path.rs rename to src/resolver_path.rs index da3d0275..c4f633e2 100644 --- a/src/ustr_path.rs +++ b/src/resolver_path.rs @@ -18,7 +18,7 @@ use crate::interner::{self, Interned}; /// costs N pointers rather than N copies. /// /// Equality is a pointer comparison whenever the strings are byte-identical, -/// and hashing is a single `u64` load from the entry header, so `UstrPathSet` +/// and hashing is a single `u64` load from the entry header, so `ResolverPathSet` /// lookups cost one `write_u64`. On Windows, spellings that differ only in /// separators or drive case are distinct entries, so both fall back to /// `Path`'s component semantics to keep them one key. Paths are stored @@ -35,7 +35,7 @@ use crate::interner::{self, Interned}; /// climb monotonically in a dev server. The flip side is that `as_str()` /// borrows from `self` rather than being `'static`. #[derive(Clone)] -pub struct UstrPath(Interned); +pub struct ResolverPath(Interned); /// Hash a path the way `PartialEq` compares it, so `a == b` implies equal /// hashes on every platform. @@ -77,8 +77,8 @@ const fn is_sep(c: u8) -> bool { /// Platform-independent on purpose. camino's `Utf8Path::components()` picks its /// separator set at compile time, so it cannot express Windows semantics on a /// unix host — this would otherwise be untestable off Windows. -// NOT CURRENTLY WIRED UP. `UstrPath::new` used to call this on Windows, but -// `UstrPath` also carries strings that are not canonical filesystem paths — +// NOT CURRENTLY WIRED UP. `ResolverPath::new` used to call this on Windows, but +// `ResolverPath` also carries strings that are not canonical filesystem paths — // rspack stores caller-supplied dependency specifiers such as // `addBuildDependency("./build.txt")` in path sets — and rewriting those // changed values observable through the public JS API (`./build.txt` came back @@ -143,7 +143,7 @@ fn normalize_windows_separators(s: &str) -> Option { Some(out) } -impl UstrPath { +impl ResolverPath { /// Intern `path` verbatim and return a handle to it. /// /// The string is stored exactly as given — rspack puts caller-supplied @@ -195,7 +195,7 @@ impl UstrPath { } } -impl Default for UstrPath { +impl Default for ResolverPath { /// The empty path — a handle to a real interned `""`, not a dangling one. #[inline] fn default() -> Self { @@ -203,14 +203,14 @@ impl Default for UstrPath { } } -impl PartialEq for UstrPath { +impl PartialEq for ResolverPath { /// Byte-identical strings share one interner entry, so the pointer check /// settles them — on unix that is the whole story, matching the byte-wise /// comparison `hash_utf8_path` has always used for resolver paths. /// /// Windows additionally folds spellings: `Path`'s `Eq` walks components, so /// `C:/a/b`, `C:\a\b`, `C:\a\\b`, `C:\a\b\` and `c:\a\b` are all one path. - /// Since [`UstrPath::new`] stores the string verbatim those are distinct + /// Since [`ResolverPath::new`] stores the string verbatim those are distinct /// entries, and only the component walk can tell they are equal. The hash /// check in front of it is a cheap reject, valid because `hash_path_str` /// hashes components on Windows too. @@ -231,12 +231,12 @@ impl PartialEq for UstrPath { } } -impl Eq for UstrPath {} +impl Eq for ResolverPath {} -impl Hash for UstrPath { +impl Hash for ResolverPath { /// One `write_u64` of the hash computed at construction. /// - /// The single write is load-bearing, not stylistic: [`UstrPathSet`] is keyed + /// The single write is load-bearing, not stylistic: [`ResolverPathSet`] is keyed /// by [`IdentityHasher`], whose `write` only reads a value when handed /// exactly 8 bytes and **silently yields 0** otherwise. Hashing the path /// inline here would feed it component-sized writes and collapse every key @@ -247,7 +247,7 @@ impl Hash for UstrPath { } } -impl Deref for UstrPath { +impl Deref for ResolverPath { type Target = Utf8Path; #[inline] @@ -256,54 +256,54 @@ impl Deref for UstrPath { } } -impl AsRef for UstrPath { +impl AsRef for ResolverPath { #[inline] fn as_ref(&self) -> &Utf8Path { self.as_utf8_path() } } -impl AsRef for UstrPath { +impl AsRef for ResolverPath { #[inline] fn as_ref(&self) -> &Path { self.as_std_path() } } -impl AsRef for UstrPath { +impl AsRef for ResolverPath { #[inline] fn as_ref(&self) -> &str { self.as_str() } } -impl fmt::Debug for UstrPath { +impl fmt::Debug for ResolverPath { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.as_utf8_path().fmt(f) } } -impl fmt::Display for UstrPath { +impl fmt::Display for ResolverPath { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(self.as_str()) } } -/// A `HashSet` keyed by the interner's precomputed hash. +/// A `HashSet` keyed by the interner's precomputed hash. /// /// Spelled with [`IdentityHasher`] rather than a set-local hasher so rspack's /// `ArcPathSet` is the same concrete type and can take these sets by /// `mem::take` instead of re-bucketing every element. -pub type UstrPathSet = HashSet>; +pub type ResolverPathSet = HashSet>; /// Passes an already-computed hash straight through. /// -/// [`UstrPath::hash`] writes the hash it computed at construction, so re-mixing +/// [`ResolverPath::hash`] writes the hash it computed at construction, so re-mixing /// it here would be wasted work. Downstream spells its own path sets with this /// exact type so it can `mem::take` ours instead of re-bucketing every element. /// /// Only `write_u64` is meaningful. Anything else is a misuse — the key type is -/// not `UstrPath` — and would silently produce 0 for every key, so it panics in +/// not `ResolverPath` — and would silently produce 0 for every key, so it panics in /// debug builds rather than quietly degrading the map into a linked list. #[derive(Default, Clone, Copy)] pub struct IdentityHasher(u64); @@ -314,7 +314,7 @@ impl Hasher for IdentityHasher { debug_assert!( false, "IdentityHasher only accepts write_u64; got a {}-byte write. The key \ - type is probably not UstrPath.", + type is probably not ResolverPath.", bytes.len() ); // Release builds: fold the bytes rather than yielding 0, so a misuse @@ -335,82 +335,82 @@ impl Hasher for IdentityHasher { } } -/// Convert any path-shaped value into an interned [`UstrPath`]. +/// Convert any path-shaped value into an interned [`ResolverPath`]. /// /// The two `std::path` implementations panic on non-UTF-8 input, matching what /// the resolver already does at every `Path -> Utf8Path` boundary (see the /// `expect("path should be UTF-8")` calls in `lib.rs` and `cache.rs`). -pub trait ToUstrPath { - fn to_ustr_path(&self) -> UstrPath; +pub trait ToResolverPath { + fn to_resolver_path(&self) -> ResolverPath; } -impl ToUstrPath for str { +impl ToResolverPath for str { #[inline] - fn to_ustr_path(&self) -> UstrPath { - UstrPath::new(self) + fn to_resolver_path(&self) -> ResolverPath { + ResolverPath::new(self) } } -impl ToUstrPath for String { +impl ToResolverPath for String { #[inline] - fn to_ustr_path(&self) -> UstrPath { - UstrPath::new(self) + fn to_resolver_path(&self) -> ResolverPath { + ResolverPath::new(self) } } -impl ToUstrPath for Utf8Path { +impl ToResolverPath for Utf8Path { #[inline] - fn to_ustr_path(&self) -> UstrPath { - UstrPath::new(self.as_str()) + fn to_resolver_path(&self) -> ResolverPath { + ResolverPath::new(self.as_str()) } } -impl ToUstrPath for Utf8PathBuf { +impl ToResolverPath for Utf8PathBuf { #[inline] - fn to_ustr_path(&self) -> UstrPath { - UstrPath::new(self.as_str()) + fn to_resolver_path(&self) -> ResolverPath { + ResolverPath::new(self.as_str()) } } -impl ToUstrPath for Path { +impl ToResolverPath for Path { #[inline] - fn to_ustr_path(&self) -> UstrPath { - UstrPath::new(self.to_str().expect("path should be UTF-8")) + fn to_resolver_path(&self) -> ResolverPath { + ResolverPath::new(self.to_str().expect("path should be UTF-8")) } } -impl ToUstrPath for PathBuf { +impl ToResolverPath for PathBuf { #[inline] - fn to_ustr_path(&self) -> UstrPath { - self.as_path().to_ustr_path() + fn to_resolver_path(&self) -> ResolverPath { + self.as_path().to_resolver_path() } } -impl ToUstrPath for UstrPath { +impl ToResolverPath for ResolverPath { #[inline] - fn to_ustr_path(&self) -> UstrPath { + fn to_resolver_path(&self) -> ResolverPath { self.clone() } } -impl From<&T> for UstrPath { +impl From<&T> for ResolverPath { #[inline] fn from(value: &T) -> Self { - value.to_ustr_path() + value.to_resolver_path() } } -impl From for UstrPath { +impl From for ResolverPath { #[inline] fn from(value: Utf8PathBuf) -> Self { - value.to_ustr_path() + value.to_resolver_path() } } -impl From for UstrPath { +impl From for ResolverPath { #[inline] fn from(value: PathBuf) -> Self { - value.to_ustr_path() + value.to_resolver_path() } } @@ -427,39 +427,39 @@ mod tests { #[test] fn default_is_the_empty_path() { - assert_eq!(UstrPath::default().as_str(), ""); + assert_eq!(ResolverPath::default().as_str(), ""); } /// The handle must stay one pointer wide. It is stored by the million - /// downstream — every dependency set, every `Vec` — so widening it + /// downstream — every dependency set, every `Vec` — so widening it /// (by caching the hash beside the pointer instead of in the interner entry, /// say) shows up directly as `memcpy` in the resolver benchmarks. #[test] fn the_handle_is_one_pointer_wide() { assert_eq!( - std::mem::size_of::(), + std::mem::size_of::(), std::mem::size_of::() ); } #[test] fn same_string_is_same_pointer() { - let a = UstrPath::new("/a/b/c.js"); - let b = UstrPath::new("/a/b/c.js"); + let a = ResolverPath::new("/a/b/c.js"); + let b = ResolverPath::new("/a/b/c.js"); assert_eq!(a.as_str().as_ptr(), b.as_str().as_ptr()); assert_eq!(a, b); } #[test] fn different_strings_are_not_equal() { - assert_ne!(UstrPath::new("/a/b"), UstrPath::new("/a/c")); + assert_ne!(ResolverPath::new("/a/b"), ResolverPath::new("/a/c")); } - /// `UstrPathSet` is keyed by [`IdentityHasher`], so this is the hash that + /// `ResolverPathSet` is keyed by [`IdentityHasher`], so this is the hash that /// actually decides bucketing. Folding it through that hasher — rather than a /// generic one — is what proves `Hash` still delivers exactly 8 bytes: the /// identity hasher silently yields 0 for any other width. - fn set_hash(p: &UstrPath) -> u64 { + fn set_hash(p: &ResolverPath) -> u64 { let mut hasher = IdentityHasher::default(); p.hash(&mut hasher); hasher.finish() @@ -470,9 +470,9 @@ mod tests { fn windows_spellings_of_one_path_are_equal_and_hash_alike() { // Stored verbatim, so these are genuinely distinct interned handles — // the folding has to come from `PartialEq`/`Hash`, not from interning. - let canonical = UstrPath::new(r"C:\a\b"); + let canonical = ResolverPath::new(r"C:\a\b"); for spelling in ["C:/a/b", r"C:/a\b", r"C:\a\\b", r"C:\a\b\", r"c:\a\b"] { - let p = UstrPath::new(spelling); + let p = ResolverPath::new(spelling); assert_ne!( p.as_str(), canonical.as_str(), @@ -490,18 +490,18 @@ mod tests { #[cfg(windows)] #[test] fn windows_distinct_paths_stay_distinct() { - assert_ne!(UstrPath::new(r"C:\a\b"), UstrPath::new(r"C:\a\c")); - assert_ne!(UstrPath::new(r"C:\a\b"), UstrPath::new(r"D:\a\b")); + assert_ne!(ResolverPath::new(r"C:\a\b"), ResolverPath::new(r"C:\a\c")); + assert_ne!(ResolverPath::new(r"C:\a\b"), ResolverPath::new(r"D:\a\b")); } #[cfg(windows)] #[test] fn windows_equal_paths_are_one_key_in_a_set() { - let mut set: UstrPathSet = HashSet::default(); - set.insert(UstrPath::new(r"C:\a\b")); - assert!(set.contains(&UstrPath::new("C:/a/b"))); - assert!(set.contains(&UstrPath::new(r"c:\a\b"))); - set.insert(UstrPath::new("C:/a/b")); + let mut set: ResolverPathSet = HashSet::default(); + set.insert(ResolverPath::new(r"C:\a\b")); + assert!(set.contains(&ResolverPath::new("C:/a/b"))); + assert!(set.contains(&ResolverPath::new(r"c:\a\b"))); + set.insert(ResolverPath::new("C:/a/b")); assert_eq!(set.len(), 1, "equal spellings must collapse to one entry"); } @@ -510,13 +510,16 @@ mod tests { // Guards both branches of `Hash`: a regression that forwarded // `Path::hash` straight through would write component-sized chunks, and // `IdentityHasher` would silently return 0 rather than fail. - assert_ne!(set_hash(&UstrPath::new("/some/long/path/segment.js")), 0); + assert_ne!( + set_hash(&ResolverPath::new("/some/long/path/segment.js")), + 0 + ); } #[test] fn equal_paths_always_hash_equal() { - let a = UstrPath::new("/a/b"); - let b = UstrPath::new("/a/b"); + let a = ResolverPath::new("/a/b"); + let b = ResolverPath::new("/a/b"); assert_eq!(a, b); assert_eq!(set_hash(&a), set_hash(&b)); } @@ -529,7 +532,7 @@ mod tests { // `hash_delivers_eight_bytes_to_the_identity_hasher`. #[cfg(not(windows))] fn hash_is_the_precomputed_hash() { - let p = UstrPath::new("/x/y"); + let p = ResolverPath::new("/x/y"); let mut hasher = std::collections::hash_map::DefaultHasher::new(); p.hash(&mut hasher); // The value written into the hasher is the precomputed one, not a @@ -541,15 +544,15 @@ mod tests { #[test] fn works_in_an_identity_hashed_set() { - let mut set: UstrPathSet = HashSet::default(); - set.insert(UstrPath::new("/a/b")); - assert!(set.contains(&UstrPath::new("/a/b"))); - assert!(!set.contains(&UstrPath::new("/a/c"))); + let mut set: ResolverPathSet = HashSet::default(); + set.insert(ResolverPath::new("/a/b")); + assert!(set.contains(&ResolverPath::new("/a/b"))); + assert!(!set.contains(&ResolverPath::new("/a/c"))); } #[test] fn derefs_to_utf8_path() { - let p = UstrPath::new("/a/b/c.js"); + let p = ResolverPath::new("/a/b/c.js"); assert_eq!(p.file_name(), Some("c.js")); assert_eq!(p.parent(), Some(Utf8Path::new("/a/b"))); assert_eq!(p.extension(), Some("js")); @@ -557,18 +560,18 @@ mod tests { } #[test] - fn debug_prints_the_path_not_the_ustr_wrapper() { - // `Debug`/`Display` render the path, not `Ustr`'s own `u!("...")` wrapper. + fn debug_prints_the_path_not_the_handle() { + // `Debug`/`Display` render the path, not the handle wrapping it. // Platform-independent because interning is verbatim — the string comes // back exactly as passed in on every platform. - let p = UstrPath::new("/a/b"); + let p = ResolverPath::new("/a/b"); assert_eq!(format!("{p:?}"), "\"/a/b\""); assert_eq!(format!("{p}"), "/a/b"); } #[test] fn as_ref_targets_compile_and_agree() { - let p = UstrPath::new("/a/b"); + let p = ResolverPath::new("/a/b"); let as_utf8: &Utf8Path = p.as_ref(); let std_path: &std::path::Path = p.as_ref(); let str_ref: &str = p.as_ref(); @@ -579,11 +582,11 @@ mod tests { #[test] fn identity_hasher_receives_eight_bytes() { // `IdentityHasher::write` silently produces a 0 hash unless it gets - // exactly 8 bytes. `UstrPath::hash` goes through the default `write_u64`, + // exactly 8 bytes. `ResolverPath::hash` goes through the default `write_u64`, // which forwards `u64::to_ne_bytes()` — exactly 8. Guard that invariant so // a future change to `Hash` cannot silently collapse every key to bucket 0. let mut hasher = IdentityHasher::default(); - UstrPath::new("/some/long/path/that/is/not/eight/bytes").hash(&mut hasher); + ResolverPath::new("/some/long/path/that/is/not/eight/bytes").hash(&mut hasher); assert_ne!(hasher.finish(), 0); } @@ -677,35 +680,41 @@ mod tests { // `equivalent_windows_spellings_intern_to_one_pointer` used to live here. It // asserted every spelling interned to one pointer, which only held while - // `UstrPath::new` normalized. Interning is verbatim now, so the spellings are + // `ResolverPath::new` normalized. Interning is verbatim now, so the spellings are // distinct handles that compare and hash equal instead — asserted by // `windows_spellings_of_one_path_are_equal_and_hash_alike` above, which also // checks the strings really are stored distinctly. #[test] - fn to_ustr_path_accepts_every_path_flavor() { + fn to_resolver_path_accepts_every_path_flavor() { use camino::Utf8PathBuf; - let expected = UstrPath::new("/a/b"); - assert_eq!("/a/b".to_ustr_path(), expected); - assert_eq!(String::from("/a/b").to_ustr_path(), expected); - assert_eq!(Utf8Path::new("/a/b").to_ustr_path(), expected); - assert_eq!(Utf8PathBuf::from("/a/b").to_ustr_path(), expected); - assert_eq!(std::path::Path::new("/a/b").to_ustr_path(), expected); - assert_eq!(std::path::PathBuf::from("/a/b").to_ustr_path(), expected); - assert_eq!(expected.to_ustr_path(), expected); + let expected = ResolverPath::new("/a/b"); + assert_eq!("/a/b".to_resolver_path(), expected); + assert_eq!(String::from("/a/b").to_resolver_path(), expected); + assert_eq!(Utf8Path::new("/a/b").to_resolver_path(), expected); + assert_eq!(Utf8PathBuf::from("/a/b").to_resolver_path(), expected); + assert_eq!(std::path::Path::new("/a/b").to_resolver_path(), expected); + assert_eq!( + std::path::PathBuf::from("/a/b").to_resolver_path(), + expected + ); + assert_eq!(expected.to_resolver_path(), expected); } #[test] - fn from_impls_match_to_ustr_path() { + fn from_impls_match_to_resolver_path() { use camino::Utf8PathBuf; - let expected = UstrPath::new("/a/b"); - assert_eq!(UstrPath::from("/a/b"), expected); - assert_eq!(UstrPath::from(Utf8Path::new("/a/b")), expected); - assert_eq!(UstrPath::from(Utf8PathBuf::from("/a/b")), expected); - assert_eq!(UstrPath::from(std::path::Path::new("/a/b")), expected); - assert_eq!(UstrPath::from(std::path::PathBuf::from("/a/b")), expected); + let expected = ResolverPath::new("/a/b"); + assert_eq!(ResolverPath::from("/a/b"), expected); + assert_eq!(ResolverPath::from(Utf8Path::new("/a/b")), expected); + assert_eq!(ResolverPath::from(Utf8PathBuf::from("/a/b")), expected); + assert_eq!(ResolverPath::from(std::path::Path::new("/a/b")), expected); + assert_eq!( + ResolverPath::from(std::path::PathBuf::from("/a/b")), + expected + ); } #[test] @@ -715,6 +724,6 @@ mod tests { use std::{ffi::OsStr, os::unix::ffi::OsStrExt}; let bad = std::path::Path::new(OsStr::from_bytes(b"/a/\xff/b")); - let _ = bad.to_ustr_path(); + let _ = bad.to_resolver_path(); } } diff --git a/src/tests/dependencies.rs b/src/tests/dependencies.rs index 46a00216..683bf9d6 100644 --- a/src/tests/dependencies.rs +++ b/src/tests/dependencies.rs @@ -60,7 +60,7 @@ mod windows { use std::path::PathBuf; use super::super::memory_fs::MemoryFS; - use crate::{ResolveContext, ResolveOptions, ResolverGeneric, UstrPath, UstrPathSet}; + use crate::{ResolveContext, ResolveOptions, ResolverGeneric, ResolverPath, ResolverPathSet}; fn file_system() -> MemoryFS { MemoryFS::new(&[ @@ -157,13 +157,13 @@ mod windows { .await .map(|r| r.full_path()); assert_eq!(resolved, Ok(PathBuf::from(result))); - let file_dependencies: UstrPathSet = file_dependencies + let file_dependencies: ResolverPathSet = file_dependencies .iter() - .map(|p| UstrPath::from(PathBuf::from(p))) + .map(|p| ResolverPath::from(PathBuf::from(p))) .collect(); - let missing_dependencies: UstrPathSet = missing_dependencies + let missing_dependencies: ResolverPathSet = missing_dependencies .iter() - .map(|p| UstrPath::from(PathBuf::from(p))) + .map(|p| ResolverPath::from(PathBuf::from(p))) .collect(); assert_eq!(ctx.file_dependencies, file_dependencies, "{name}"); assert_eq!(ctx.missing_dependencies, missing_dependencies, "{name}"); diff --git a/src/tests/extensions.rs b/src/tests/extensions.rs index b98df3b4..300e53ff 100644 --- a/src/tests/extensions.rs +++ b/src/tests/extensions.rs @@ -1,8 +1,8 @@ //! use crate::{ - EnforceExtension, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, UstrPath, - UstrPathSet, + EnforceExtension, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, + ResolverPath, ResolverPathSet, }; #[tokio::test] @@ -65,9 +65,9 @@ async fn default_enforce_extension() { ); assert_eq!( ctx.file_dependencies, - UstrPathSet::from_iter([ - UstrPath::from(f.join("foo.ts")), - UstrPath::from(f.join("package.json")), + ResolverPathSet::from_iter([ + ResolverPath::from(f.join("foo.ts")), + ResolverPath::from(f.join("package.json")), ]) ); assert!(ctx.missing_dependencies.is_empty()); @@ -93,14 +93,14 @@ async fn respect_enforce_extension() { ); assert_eq!( ctx.file_dependencies, - UstrPathSet::from_iter([ - UstrPath::from(f.join("foo.ts")), - UstrPath::from(f.join("package.json")), + ResolverPathSet::from_iter([ + ResolverPath::from(f.join("foo.ts")), + ResolverPath::from(f.join("package.json")), ]) ); assert_eq!( ctx.missing_dependencies, - UstrPathSet::from_iter([UstrPath::from(f.join("foo"))]) + ResolverPathSet::from_iter([ResolverPath::from(f.join("foo"))]) ); } diff --git a/src/tests/incorrect_description_file.rs b/src/tests/incorrect_description_file.rs index 070567b4..782962eb 100644 --- a/src/tests/incorrect_description_file.rs +++ b/src/tests/incorrect_description_file.rs @@ -1,8 +1,8 @@ //! use crate::{ - JSONError, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, UstrPath, - UstrPathSet, + JSONError, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver, ResolverPath, + ResolverPathSet, }; // should not resolve main in incorrect description file #1 @@ -24,9 +24,9 @@ async fn incorrect_description_file_1() { assert!(matches!(resolution, Err(ResolveError::JSON(_)))); assert_eq!( ctx.file_dependencies, - UstrPathSet::from_iter([ - UstrPath::from(f.join("pack1")), - UstrPath::from(f.join("pack1/package.json")), + ResolverPathSet::from_iter([ + ResolverPath::from(f.join("pack1")), + ResolverPath::from(f.join("pack1/package.json")), ]) ); assert!(!ctx.missing_dependencies.is_empty()); diff --git a/src/tests/interning.rs b/src/tests/interning.rs index 166e12b7..c85d55da 100644 --- a/src/tests/interning.rs +++ b/src/tests/interning.rs @@ -1,6 +1,6 @@ -//! End-to-end guarantees for `UstrPath` interning. +//! End-to-end guarantees for `ResolverPath` interning. //! -//! These assert on **pointer identity**, never on `ustr::num_entries()` or +//! These assert on **pointer identity**, never on a global entry count or //! `total_allocated()` — the interner is process-global and shared with every //! other test running in parallel, so any global-count assertion is flaky by //! construction. @@ -73,7 +73,7 @@ async fn equal_paths_from_different_sources_are_one_pointer() { ); for dep in &ctx.file_dependencies { - let reinterned = crate::UstrPath::new(dep.as_str()); + let reinterned = crate::ResolverPath::new(dep.as_str()); assert_eq!( dep.as_str().as_ptr(), reinterned.as_str().as_ptr(), diff --git a/src/tests/missing.rs b/src/tests/missing.rs index e664c6d8..f403d256 100644 --- a/src/tests/missing.rs +++ b/src/tests/missing.rs @@ -2,7 +2,7 @@ use normalize_path::NormalizePath; -use crate::{AliasValue, ResolveContext, ResolveOptions, Resolver, UstrPath}; +use crate::{AliasValue, ResolveContext, ResolveOptions, Resolver, ResolverPath}; #[tokio::test] async fn test() { @@ -69,7 +69,9 @@ async fn test() { for path in missing_dependencies { assert_eq!(path, path.normalize(), "{path:?}"); assert!( - ctx.missing_dependencies.contains(&UstrPath::from(&path)), + ctx + .missing_dependencies + .contains(&ResolverPath::from(&path)), "{specifier}: {path:?} not in {:?}", &ctx.missing_dependencies ); diff --git a/src/tests/package_json.rs b/src/tests/package_json.rs index 39c6d82a..3fcb9614 100644 --- a/src/tests/package_json.rs +++ b/src/tests/package_json.rs @@ -1,10 +1,10 @@ #[cfg(test)] mod tests { - use crate::{package_json::ParseError, PackageJson, UstrPath}; + use crate::{package_json::ParseError, PackageJson, ResolverPath}; #[tokio::test] async fn test_json_with_bom() { - let mock_path = UstrPath::new("package.json"); + let mock_path = ResolverPath::new("package.json"); let json_with_bom = b"\xEF\xBB\xBF{\"name\": \"example-package\"}".to_vec(); let result = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom).err(); @@ -20,7 +20,7 @@ mod tests { #[tokio::test] async fn test_normal_json() { - let mock_path = UstrPath::new("package.json"); + let mock_path = ResolverPath::new("package.json"); let json_with_bom = r##"{"name": "example-package"}"##.as_bytes().to_vec(); let parsed = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom).unwrap(); @@ -30,7 +30,7 @@ mod tests { #[tokio::test] async fn test_broken_json() { - let mock_path = UstrPath::new("package.json"); + let mock_path = ResolverPath::new("package.json"); let json_with_bom = r##"{"broken":"string"##.as_bytes().to_vec(); let parsed_err = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom).err(); @@ -47,7 +47,7 @@ mod tests { #[tokio::test] async fn test_empty_string() { - let mock_path = UstrPath::new("package.json"); + let mock_path = ResolverPath::new("package.json"); let json_with_bom = " ".as_bytes().to_vec(); let parse_error = PackageJson::parse(mock_path.clone(), mock_path, json_with_bom) diff --git a/src/tests/pnp.rs b/src/tests/pnp.rs index 1f69b73d..13c70a62 100644 --- a/src/tests/pnp.rs +++ b/src/tests/pnp.rs @@ -6,7 +6,7 @@ use camino::Utf8Path; use crate::{ - path::PathUtil, ResolveContext, ResolveError::NotFound, ResolveOptions, Resolver, UstrPath, + path::PathUtil, ResolveContext, ResolveError::NotFound, ResolveOptions, Resolver, ResolverPath, }; #[tokio::test] @@ -100,7 +100,7 @@ async fn pnp_file_dependencies() { assert!( ctx .file_dependencies - .contains(&UstrPath::from(fixture.join(".pnp.cjs"))), + .contains(&ResolverPath::from(fixture.join(".pnp.cjs"))), ".pnp.cjs should be in file_dependencies, got: {:?}", ctx.file_dependencies ); diff --git a/src/tests/symlink.rs b/src/tests/symlink.rs index 82210b0e..949c3677 100644 --- a/src/tests/symlink.rs +++ b/src/tests/symlink.rs @@ -1,6 +1,6 @@ use std::{fs, io, path::Path}; -use crate::{ResolveOptions, Resolver, UstrPath}; +use crate::{ResolveOptions, Resolver, ResolverPath}; #[derive(Debug, Clone, Copy)] enum FileType { @@ -150,7 +150,7 @@ async fn test() -> io::Result<()> { assert!( ctx .file_dependencies - .contains(&UstrPath::from(resolved_path.unwrap())), + .contains(&ResolverPath::from(resolved_path.unwrap())), "file dependencies should contain resolved path {comment:?}" ); } diff --git a/src/tests/tsconfig_project_references.rs b/src/tests/tsconfig_project_references.rs index 7a1dcff2..258babbe 100644 --- a/src/tests/tsconfig_project_references.rs +++ b/src/tests/tsconfig_project_references.rs @@ -1,8 +1,8 @@ //! Tests for tsconfig project references use crate::{ - ResolveContext, ResolveError, ResolveOptions, Resolver, TsconfigOptions, TsconfigReferences, - UstrPath, + ResolveContext, ResolveError, ResolveOptions, Resolver, ResolverPath, TsconfigOptions, + TsconfigReferences, }; #[tokio::test] @@ -75,7 +75,9 @@ async fn tsconfig_file_as_file_dependencies() { ]; for dependency in expected_dependencies { assert!( - ctx.file_dependencies.contains(&UstrPath::from(&dependency)), + ctx + .file_dependencies + .contains(&ResolverPath::from(&dependency)), "missing tsconfig file dependency {dependency:?}: {:?}", ctx.file_dependencies ); @@ -95,7 +97,7 @@ async fn tsconfig_dependency_is_one_interned_pointer_across_resolves() { }); let shared_tsconfig = f.join("app/tsconfig.json"); - let expected = UstrPath::from(&shared_tsconfig); + let expected = ResolverPath::from(&shared_tsconfig); let mut pointers = vec![]; for (dir, request) in [ diff --git a/src/tsconfig.rs b/src/tsconfig.rs index d81901c6..9ebdd4da 100644 --- a/src/tsconfig.rs +++ b/src/tsconfig.rs @@ -7,12 +7,12 @@ use serde::Deserialize; use crate::{ path::PathUtil, - ustr_path::{ToUstrPath, UstrPath}, + resolver_path::{ResolverPath, ToResolverPath}, }; pub type CompilerOptionsPathsMap = IndexMap, BuildHasherDefault>; pub type FileDependencies = - IndexSet>; + IndexSet>; #[derive(Debug, Clone, Eq, PartialEq, Deserialize)] #[serde(untagged)] @@ -91,13 +91,13 @@ impl TsConfig { let mut tsconfig: Self = serde_json::from_str("{}")?; tsconfig.root = root; tsconfig.path = path.to_path_buf(); - tsconfig.file_dependencies.insert(path.to_ustr_path()); + tsconfig.file_dependencies.insert(path.to_resolver_path()); return Ok(tsconfig); } let mut tsconfig: Self = serde_json::from_str(json)?; tsconfig.root = root; tsconfig.path = path.to_path_buf(); - tsconfig.file_dependencies.insert(path.to_ustr_path()); + tsconfig.file_dependencies.insert(path.to_resolver_path()); let directory = tsconfig.directory().to_path_buf(); if let Some(base_url) = &tsconfig.compiler_options.base_url { // keep the `${configDir}` template variable in the baseUrl