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 Satya Rohith
parent 08f0613b39
commit b25cc8034d
No known key found for this signature in database
GPG key ID: B2705CF40523EB05
5 changed files with 24 additions and 17 deletions

View file

@ -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 {

View file

@ -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)

View file

@ -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()
}

View file

@ -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()

View file

@ -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,