diff options
author | David Sherret <dsherret@users.noreply.github.com> | 2024-04-01 09:10:04 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-04-01 09:10:04 -0400 |
commit | 240b362c002d17bc2b676673ed1b9406683ff0c2 (patch) | |
tree | d0c3cdc099b7cac4f29ae17f7ba03ee01a01c877 | |
parent | 8d158058e5abab14d122712a716979b865620995 (diff) |
perf(node): put pkg json into an `Rc` (#23156)
Was doing a bit of debugging on why some stuff is not working in a
personal project and ran a quick debug profile and saw it cloning the
pkg json a lot. We should put this in an Rc.
-rw-r--r-- | cli/npm/byonm.rs | 3 | ||||
-rw-r--r-- | cli/resolver.rs | 3 | ||||
-rw-r--r-- | ext/node/ops/require.rs | 11 | ||||
-rw-r--r-- | ext/node/package_json.rs | 11 | ||||
-rw-r--r-- | ext/node/resolution.rs | 13 |
5 files changed, 24 insertions, 17 deletions
diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index d17be0e95..1e61ce885 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -3,6 +3,7 @@ use std::borrow::Cow; use std::path::Path; use std::path::PathBuf; +use std::rc::Rc; use std::sync::Arc; use deno_ast::ModuleSpecifier; @@ -52,7 +53,7 @@ impl ByonmCliNpmResolver { &self, dep_name: &str, referrer: &ModuleSpecifier, - ) -> Option<PackageJson> { + ) -> Option<Rc<PackageJson>> { let referrer_path = referrer.to_file_path().ok()?; let mut current_folder = referrer_path.parent()?; loop { diff --git a/cli/resolver.rs b/cli/resolver.rs index 6594bf2d4..fc326e1b1 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -36,6 +36,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; +use std::rc::Rc; use std::sync::Arc; use crate::args::package_json::PackageJsonDeps; @@ -98,7 +99,7 @@ impl CliNodeResolver { &self, referrer: &ModuleSpecifier, permissions: &dyn NodePermissions, - ) -> Result<Option<PackageJson>, AnyError> { + ) -> Result<Option<Rc<PackageJson>>, AnyError> { self .node_resolver .get_closest_package_json(referrer, permissions) diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index d6771c668..426c41995 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -542,10 +542,12 @@ where )?; let node_resolver = state.borrow::<Rc<NodeResolver>>(); let permissions = state.borrow::<P>(); - node_resolver.get_closest_package_json( - &Url::from_file_path(filename).unwrap(), - permissions, - ) + node_resolver + .get_closest_package_json( + &Url::from_file_path(filename).unwrap(), + permissions, + ) + .map(|maybe_pkg| maybe_pkg.map(|pkg| (*pkg).clone())) } #[op2] @@ -562,6 +564,7 @@ where let package_json_path = PathBuf::from(package_json_path); node_resolver .load_package_json(permissions, package_json_path) + .map(|pkg| (*pkg).clone()) .ok() } diff --git a/ext/node/package_json.rs b/ext/node/package_json.rs index 9ac3a0969..77352ae1d 100644 --- a/ext/node/package_json.rs +++ b/ext/node/package_json.rs @@ -18,9 +18,10 @@ use std::cell::RefCell; use std::collections::HashMap; use std::io::ErrorKind; use std::path::PathBuf; +use std::rc::Rc; thread_local! { - static CACHE: RefCell<HashMap<PathBuf, PackageJson>> = RefCell::new(HashMap::new()); + static CACHE: RefCell<HashMap<PathBuf, Rc<PackageJson>>> = RefCell::new(HashMap::new()); } #[derive(Clone, Debug, Serialize)] @@ -66,7 +67,7 @@ impl PackageJson { resolver: &dyn NpmResolver, permissions: &dyn NodePermissions, path: PathBuf, - ) -> Result<PackageJson, AnyError> { + ) -> Result<Rc<PackageJson>, AnyError> { resolver.ensure_read_permission(permissions, &path)?; Self::load_skip_read_permission(fs, path) } @@ -74,7 +75,7 @@ impl PackageJson { pub fn load_skip_read_permission( fs: &dyn deno_fs::FileSystem, path: PathBuf, - ) -> Result<PackageJson, AnyError> { + ) -> Result<Rc<PackageJson>, AnyError> { assert!(path.is_absolute()); if CACHE.with(|cache| cache.borrow().contains_key(&path)) { @@ -84,7 +85,7 @@ impl PackageJson { let source = match fs.read_text_file_sync(&path) { Ok(source) => source, Err(err) if err.kind() == ErrorKind::NotFound => { - return Ok(PackageJson::empty(path)); + return Ok(Rc::new(PackageJson::empty(path))); } Err(err) => bail!( "Error loading package.json at {}. {:#}", @@ -93,7 +94,7 @@ impl PackageJson { ), }; - let package_json = Self::load_from_string(path, source)?; + let package_json = Rc::new(Self::load_from_string(path, source)?); CACHE.with(|cache| { cache .borrow_mut() diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs index 50c4e2bb5..37598810d 100644 --- a/ext/node/resolution.rs +++ b/ext/node/resolution.rs @@ -4,6 +4,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; +use std::rc::Rc; use deno_core::anyhow::bail; use deno_core::anyhow::Context; @@ -252,7 +253,7 @@ impl NodeResolver { specifier, referrer, NodeModuleKind::Esm, - pkg_config.as_ref(), + pkg_config.as_deref(), conditions, mode, permissions, @@ -399,7 +400,7 @@ impl NodeResolver { let package_json = self .load_package_json(&AllowAllNodePermissions, package_json_path.clone())?; - Ok(match package_json.bin { + Ok(match &package_json.bin { Some(Value::String(_)) => { let Some(name) = &package_json.name else { bail!("'{}' did not have a name", package_json_path.display()); @@ -407,7 +408,7 @@ impl NodeResolver { vec![name.to_string()] } Some(Value::Object(o)) => { - o.into_iter().map(|(key, _)| key).collect::<Vec<_>>() + o.iter().map(|(key, _)| key.clone()).collect::<Vec<_>>() } _ => Vec::new(), }) @@ -1164,7 +1165,7 @@ impl NodeResolver { &self, url: &ModuleSpecifier, permissions: &dyn NodePermissions, - ) -> Result<Option<PackageJson>, AnyError> { + ) -> Result<Option<Rc<PackageJson>>, AnyError> { let Ok(file_path) = url.to_file_path() else { return Ok(None); }; @@ -1175,7 +1176,7 @@ impl NodeResolver { &self, file_path: &Path, permissions: &dyn NodePermissions, - ) -> Result<Option<PackageJson>, AnyError> { + ) -> Result<Option<Rc<PackageJson>>, AnyError> { let Some(package_json_path) = self.get_closest_package_json_path(file_path)? else { @@ -1213,7 +1214,7 @@ impl NodeResolver { &self, permissions: &dyn NodePermissions, package_json_path: PathBuf, - ) -> Result<PackageJson, AnyError> { + ) -> Result<Rc<PackageJson>, AnyError> { PackageJson::load( &*self.fs, &*self.npm_resolver, |