From f094d5881e99accd4a05bf439b2ecbc4e5adb083 Mon Sep 17 00:00:00 2001 From: Boshen Date: Tue, 25 Jul 2023 16:47:17 +0800 Subject: [PATCH] perf(resolver): hash once for the `get` + `insert` case (#606) --- Cargo.lock | 7 +++++ crates/oxc_resolver/Cargo.toml | 11 ++++---- crates/oxc_resolver/src/cache.rs | 44 +++++++++++--------------------- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e9e86e284..0adeff076 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -736,6 +736,12 @@ dependencies = [ "libm", ] +[[package]] +name = "identity-hash" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dfdd7caa900436d8f13b2346fe10257e0c05c1f1f9e351f4f5d57c03bd5f45da" + [[package]] name = "idna" version = "0.4.0" @@ -1500,6 +1506,7 @@ dependencies = [ "criterion", "dashmap", "dunce", + "identity-hash", "jemallocator", "mimalloc", "nodejs-resolver", diff --git a/crates/oxc_resolver/Cargo.toml b/crates/oxc_resolver/Cargo.toml index 529258b62..8580beceb 100644 --- a/crates/oxc_resolver/Cargo.toml +++ b/crates/oxc_resolver/Cargo.toml @@ -11,11 +11,12 @@ license.workspace = true repository.workspace = true [dependencies] -dashmap = { workspace = true } -serde = { workspace = true, features = ["derive"] } -serde_json = { workspace = true } -rustc-hash = { workspace = true } -dunce = "1.0.4" +dashmap = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +rustc-hash = { workspace = true } +dunce = "1.0.4" +identity-hash = "0.1.0" [dev-dependencies] static_assertions = { workspace = true } diff --git a/crates/oxc_resolver/src/cache.rs b/crates/oxc_resolver/src/cache.rs index 472c14aeb..cac22229f 100644 --- a/crates/oxc_resolver/src/cache.rs +++ b/crates/oxc_resolver/src/cache.rs @@ -1,25 +1,26 @@ use std::{ - borrow::Borrow, convert::AsRef, - hash::{BuildHasherDefault, Hash, Hasher}, + hash::{Hash, Hasher}, ops::Deref, path::{Path, PathBuf}, sync::{Arc, OnceLock}, }; -use dashmap::DashSet; +use dashmap::DashMap; +use identity_hash::BuildIdentityHasher; use rustc_hash::FxHasher; use crate::{package_json::PackageJson, FileMetadata, FileSystem, ResolveError}; pub struct Cache { pub(crate) fs: Fs, - cache: DashSet>, + // Using IdentityHasher to avoid double hashing in the `get` + `insert` case. + cache: DashMap>, } impl Default for Cache { fn default() -> Self { - Self { fs: Fs::default(), cache: DashSet::default() } + Self { fs: Fs::default(), cache: DashMap::default() } } } @@ -40,18 +41,23 @@ impl Cache { } pub fn value(&self, path: &Path) -> CacheValue { - if let Some(cache_entry) = self.cache.get(path) { - return cache_entry.key().clone(); + let hash = { + let mut hasher = FxHasher::default(); + path.hash(&mut hasher); + hasher.finish() + }; + if let Some(cache_entry) = self.cache.get(&hash) { + return cache_entry.value().clone(); } let parent = path.parent().map(|p| self.value(p)); let data = CacheValue(Arc::new(CacheValueImpl::new(path.to_path_buf().into_boxed_path(), parent))); - self.cache.insert(data.clone()); + self.cache.insert(hash, data.clone()); data } } -#[derive(Debug, Clone, Eq, PartialEq, Hash)] +#[derive(Debug, Clone)] pub struct CacheValue(Arc); impl Deref for CacheValue { @@ -68,12 +74,6 @@ impl AsRef for CacheValue { } } -impl Borrow for CacheValue { - fn borrow(&self) -> &Path { - &self.path - } -} - #[derive(Debug)] pub struct CacheValueImpl { path: Box, @@ -83,20 +83,6 @@ pub struct CacheValueImpl { package_json: OnceLock, ResolveError>>>, } -impl Hash for CacheValueImpl { - fn hash(&self, h: &mut H) { - self.path.hash(h); - } -} - -impl PartialEq for CacheValueImpl { - fn eq(&self, other: &Self) -> bool { - self.path == other.path - } -} - -impl Eq for CacheValueImpl {} - impl CacheValueImpl { fn new(path: Box, parent: Option) -> Self { Self {