0
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-03-03 17:34:47 -05:00

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.
This commit is contained in:
David Sherret 2024-04-01 09:10:04 -04:00 committed by GitHub
parent 8d158058e5
commit 240b362c00
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 24 additions and 17 deletions

View file

@ -3,6 +3,7 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use std::rc::Rc;
use std::sync::Arc; use std::sync::Arc;
use deno_ast::ModuleSpecifier; use deno_ast::ModuleSpecifier;
@ -52,7 +53,7 @@ impl ByonmCliNpmResolver {
&self, &self,
dep_name: &str, dep_name: &str,
referrer: &ModuleSpecifier, referrer: &ModuleSpecifier,
) -> Option<PackageJson> { ) -> Option<Rc<PackageJson>> {
let referrer_path = referrer.to_file_path().ok()?; let referrer_path = referrer.to_file_path().ok()?;
let mut current_folder = referrer_path.parent()?; let mut current_folder = referrer_path.parent()?;
loop { loop {

View file

@ -36,6 +36,7 @@ use std::collections::HashMap;
use std::collections::HashSet; use std::collections::HashSet;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use std::rc::Rc;
use std::sync::Arc; use std::sync::Arc;
use crate::args::package_json::PackageJsonDeps; use crate::args::package_json::PackageJsonDeps;
@ -98,7 +99,7 @@ impl CliNodeResolver {
&self, &self,
referrer: &ModuleSpecifier, referrer: &ModuleSpecifier,
permissions: &dyn NodePermissions, permissions: &dyn NodePermissions,
) -> Result<Option<PackageJson>, AnyError> { ) -> Result<Option<Rc<PackageJson>>, AnyError> {
self self
.node_resolver .node_resolver
.get_closest_package_json(referrer, permissions) .get_closest_package_json(referrer, permissions)

View file

@ -542,10 +542,12 @@ where
)?; )?;
let node_resolver = state.borrow::<Rc<NodeResolver>>(); let node_resolver = state.borrow::<Rc<NodeResolver>>();
let permissions = state.borrow::<P>(); let permissions = state.borrow::<P>();
node_resolver.get_closest_package_json( node_resolver
&Url::from_file_path(filename).unwrap(), .get_closest_package_json(
permissions, &Url::from_file_path(filename).unwrap(),
) permissions,
)
.map(|maybe_pkg| maybe_pkg.map(|pkg| (*pkg).clone()))
} }
#[op2] #[op2]
@ -562,6 +564,7 @@ where
let package_json_path = PathBuf::from(package_json_path); let package_json_path = PathBuf::from(package_json_path);
node_resolver node_resolver
.load_package_json(permissions, package_json_path) .load_package_json(permissions, package_json_path)
.map(|pkg| (*pkg).clone())
.ok() .ok()
} }

View file

@ -18,9 +18,10 @@ use std::cell::RefCell;
use std::collections::HashMap; use std::collections::HashMap;
use std::io::ErrorKind; use std::io::ErrorKind;
use std::path::PathBuf; use std::path::PathBuf;
use std::rc::Rc;
thread_local! { 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)] #[derive(Clone, Debug, Serialize)]
@ -66,7 +67,7 @@ impl PackageJson {
resolver: &dyn NpmResolver, resolver: &dyn NpmResolver,
permissions: &dyn NodePermissions, permissions: &dyn NodePermissions,
path: PathBuf, path: PathBuf,
) -> Result<PackageJson, AnyError> { ) -> Result<Rc<PackageJson>, AnyError> {
resolver.ensure_read_permission(permissions, &path)?; resolver.ensure_read_permission(permissions, &path)?;
Self::load_skip_read_permission(fs, path) Self::load_skip_read_permission(fs, path)
} }
@ -74,7 +75,7 @@ impl PackageJson {
pub fn load_skip_read_permission( pub fn load_skip_read_permission(
fs: &dyn deno_fs::FileSystem, fs: &dyn deno_fs::FileSystem,
path: PathBuf, path: PathBuf,
) -> Result<PackageJson, AnyError> { ) -> Result<Rc<PackageJson>, AnyError> {
assert!(path.is_absolute()); assert!(path.is_absolute());
if CACHE.with(|cache| cache.borrow().contains_key(&path)) { if CACHE.with(|cache| cache.borrow().contains_key(&path)) {
@ -84,7 +85,7 @@ impl PackageJson {
let source = match fs.read_text_file_sync(&path) { let source = match fs.read_text_file_sync(&path) {
Ok(source) => source, Ok(source) => source,
Err(err) if err.kind() == ErrorKind::NotFound => { Err(err) if err.kind() == ErrorKind::NotFound => {
return Ok(PackageJson::empty(path)); return Ok(Rc::new(PackageJson::empty(path)));
} }
Err(err) => bail!( Err(err) => bail!(
"Error loading package.json at {}. {:#}", "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.with(|cache| {
cache cache
.borrow_mut() .borrow_mut()

View file

@ -4,6 +4,7 @@ use std::borrow::Cow;
use std::collections::HashMap; use std::collections::HashMap;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use std::rc::Rc;
use deno_core::anyhow::bail; use deno_core::anyhow::bail;
use deno_core::anyhow::Context; use deno_core::anyhow::Context;
@ -252,7 +253,7 @@ impl NodeResolver {
specifier, specifier,
referrer, referrer,
NodeModuleKind::Esm, NodeModuleKind::Esm,
pkg_config.as_ref(), pkg_config.as_deref(),
conditions, conditions,
mode, mode,
permissions, permissions,
@ -399,7 +400,7 @@ impl NodeResolver {
let package_json = self let package_json = self
.load_package_json(&AllowAllNodePermissions, package_json_path.clone())?; .load_package_json(&AllowAllNodePermissions, package_json_path.clone())?;
Ok(match package_json.bin { Ok(match &package_json.bin {
Some(Value::String(_)) => { Some(Value::String(_)) => {
let Some(name) = &package_json.name else { let Some(name) = &package_json.name else {
bail!("'{}' did not have a name", package_json_path.display()); bail!("'{}' did not have a name", package_json_path.display());
@ -407,7 +408,7 @@ impl NodeResolver {
vec![name.to_string()] vec![name.to_string()]
} }
Some(Value::Object(o)) => { Some(Value::Object(o)) => {
o.into_iter().map(|(key, _)| key).collect::<Vec<_>>() o.iter().map(|(key, _)| key.clone()).collect::<Vec<_>>()
} }
_ => Vec::new(), _ => Vec::new(),
}) })
@ -1164,7 +1165,7 @@ impl NodeResolver {
&self, &self,
url: &ModuleSpecifier, url: &ModuleSpecifier,
permissions: &dyn NodePermissions, permissions: &dyn NodePermissions,
) -> Result<Option<PackageJson>, AnyError> { ) -> Result<Option<Rc<PackageJson>>, AnyError> {
let Ok(file_path) = url.to_file_path() else { let Ok(file_path) = url.to_file_path() else {
return Ok(None); return Ok(None);
}; };
@ -1175,7 +1176,7 @@ impl NodeResolver {
&self, &self,
file_path: &Path, file_path: &Path,
permissions: &dyn NodePermissions, permissions: &dyn NodePermissions,
) -> Result<Option<PackageJson>, AnyError> { ) -> Result<Option<Rc<PackageJson>>, AnyError> {
let Some(package_json_path) = let Some(package_json_path) =
self.get_closest_package_json_path(file_path)? self.get_closest_package_json_path(file_path)?
else { else {
@ -1213,7 +1214,7 @@ impl NodeResolver {
&self, &self,
permissions: &dyn NodePermissions, permissions: &dyn NodePermissions,
package_json_path: PathBuf, package_json_path: PathBuf,
) -> Result<PackageJson, AnyError> { ) -> Result<Rc<PackageJson>, AnyError> {
PackageJson::load( PackageJson::load(
&*self.fs, &*self.fs,
&*self.npm_resolver, &*self.npm_resolver,