perf(core): avoid deep cloning json values

Author: JamieMason

src/package_json.rs

@@ -4,7 +4,7 @@ use {
   serde::Serialize,
   serde_json::{ser::PrettyFormatter, Serializer, Value},
   std::{
-    cell::RefCell,
+    cell::{Ref, RefCell},
     fs,
     path::{Path, PathBuf},
     rc::Rc,
@@ -99,6 +99,26 @@ impl PackageJson {
     self.contents.borrow().pointer(pointer).cloned()
   }
 
+  /// Get a string property without cloning the entire Value tree.
+  pub fn get_string_prop(&self, pointer: &str) -> Option<String> {
+    self
+      .contents
+      .borrow()
+      .pointer(pointer)
+      .and_then(|v| v.as_str())
+      .map(|s| s.to_string())
+  }
+
+  /// Get a borrowed reference to an object property.
+  /// Returns a Ref guard that keeps the borrow alive.
+  /// Use this when you only need to iterate/read the object, not store it.
+  pub fn get_object_prop_ref(&self, pointer: &str) -> Option<Ref<serde_json::Map<String, Value>>> {
+    Ref::filter_map(self.contents.borrow(), |contents| {
+      contents.pointer(pointer).and_then(|v| v.as_object())
+    })
+    .ok()
+  }
+
   /// Deeply set a property in the parsed package.json
   pub fn set_prop(&self, pointer: &str, next_value: Value) {
     if pointer == "/" {

src/packages.rs

@@ -87,9 +87,9 @@ impl Packages {
       .iter()
       .filter_map(|package| -> Option<(String, Rc<Specifier>)> {
         let package = package.borrow();
-        let name = package.get_prop("/name");
-        let version = package.get_prop("/version");
-        if let (Some(Value::String(name)), Some(Value::String(version))) = (name, version) {
+        let name = package.get_string_prop("/name");
+        let version = package.get_string_prop("/version");
+        if let (Some(name), Some(version)) = (name, version) {
           Some((name, Specifier::new(&version)))
         } else {
           None
@@ -108,30 +108,31 @@ impl Packages {
       for dependency_type in all_dependency_types {
         match dependency_type.strategy {
           Strategy::NameAndVersionProps => {
-            if let (Some(Value::String(name)), Some(Value::String(raw_specifier))) = (
-              package.borrow().get_prop(dependency_type.name_path.as_ref().unwrap()),
-              package.borrow().get_prop(&dependency_type.path).or_else(|| {
-                // Ensure that instances are still created for local packages
-                // which are missing a version
-                if dependency_type.name == "local" {
-                  Some(Value::String("".to_string()))
-                } else {
-                  None
-                }
-              }),
-            ) {
+            let package_borrowed = package.borrow();
+            let name = package_borrowed.get_string_prop(dependency_type.name_path.as_ref().unwrap());
+            let raw_specifier = package_borrowed.get_string_prop(&dependency_type.path).or_else(|| {
+              // Ensure that instances are still created for local packages
+              // which are missing a version
+              if dependency_type.name == "local" {
+                Some("".to_string())
+              } else {
+                None
+              }
+            });
+
+            if let (Some(name), Some(raw_specifier)) = (name, raw_specifier) {
               on_instance(InstanceDescriptor {
                 dependency_type: dependency_type.clone(),
-                internal_name: name.to_string(),
+                internal_name: name.clone(),
                 matches_cli_filter: false,
-                name: name.to_string(),
+                name,
                 package: Rc::clone(package),
                 specifier: Specifier::new(&raw_specifier),
               });
             }
           }
           Strategy::NamedVersionString => {
-            if let Some(Value::String(specifier)) = package.borrow().get_prop(&dependency_type.path) {
+            if let Some(specifier) = package.borrow().get_string_prop(&dependency_type.path) {
               if let Some((name, raw_specifier)) = specifier.split_once('@') {
                 on_instance(InstanceDescriptor {
                   dependency_type: dependency_type.clone(),
@@ -145,7 +146,7 @@ impl Packages {
             }
           }
           Strategy::UnnamedVersionString => {
-            if let Some(Value::String(raw_specifier)) = package.borrow().get_prop(&dependency_type.path) {
+            if let Some(raw_specifier) = package.borrow().get_string_prop(&dependency_type.path) {
               on_instance(InstanceDescriptor {
                 dependency_type: dependency_type.clone(),
                 internal_name: dependency_type.name.clone(),
@@ -157,16 +158,16 @@ impl Packages {
             }
           }
           Strategy::VersionsByName => {
-            if let Some(Value::Object(versions_by_name)) = package.borrow().get_prop(&dependency_type.path) {
-              for (name, raw_specifier) in versions_by_name {
+            if let Some(versions_by_name_ref) = package.borrow().get_object_prop_ref(&dependency_type.path) {
+              for (name, raw_specifier) in versions_by_name_ref.iter() {
                 if let Value::String(raw_specifier) = raw_specifier {
                   on_instance(InstanceDescriptor {
                     dependency_type: dependency_type.clone(),
                     internal_name: name.to_string(),
                     matches_cli_filter: false,
                     name: name.to_string(),
                     package: Rc::clone(package),
-                    specifier: Specifier::new(&raw_specifier),
+                    specifier: Specifier::new(raw_specifier),
                   });
                 }
               }