Refactor

Author: owjs3901

.gitignore

@@ -15,3 +15,4 @@ build_rs_cov.profraw
 tarpaulin-report.html
 .sisyphus/**
 .ruff_cache
+.omc

crates/cli/src/lib.rs

@@ -174,29 +174,47 @@ pub async fn run(cli: &Cli) -> Result<bool, Box<dyn std::error::Error + Send + S
     }
 
     // 3. Resolve ALL versions concurrently across all manifests (Promise.all pattern)
+    //    Create registries once and share across all manifests of the same kind.
     let total_deps: usize = manifest_jobs.iter().map(|j| j.deps.len()).sum();
     info!(
         manifests = manifest_jobs.len(),
         total_deps, "resolving all versions concurrently"
     );
 
+    let npm_registry = NpmRegistry::new();
+    let crates_registry = CratesIoRegistry::new();
+    let pypi_registry = PyPiRegistry::new();
+
     let mut resolve_futures = Vec::new();
     for (job_idx, job) in manifest_jobs.iter().enumerate() {
         if !job.deps.is_empty() {
+            let npm = &npm_registry;
+            let crates_io = &crates_registry;
+            let pypi = &pypi_registry;
             resolve_futures.push(async move {
-                let resolved = resolve_versions(&job.deps, job.manifest_ref.kind, cli.target).await;
+                let resolved = match job.manifest_ref.kind {
+                    ManifestKind::PackageJson => {
+                        npm.resolve_batch(&job.deps, cli.target).await
+                    }
+                    ManifestKind::CargoToml => {
+                        crates_io.resolve_batch(&job.deps, cli.target).await
+                    }
+                    ManifestKind::PyProjectToml => {
+                        pypi.resolve_batch(&job.deps, cli.target).await
+                    }
+                };
                 (job_idx, resolved)
             });
         }
     }
 
     let resolved_results: Vec<_> = futures::future::join_all(resolve_futures).await;
 
-    // Build a map: job_idx -> resolved versions
-    let mut resolved_map: std::collections::HashMap<usize, ResolvedBatch> =
-        std::collections::HashMap::new();
+    // Build a vec: job_idx -> resolved versions (dense indices, no HashMap needed)
+    let mut resolved_map: Vec<Option<ResolvedBatch>> =
+        (0..manifest_jobs.len()).map(|_| None).collect();
     for (job_idx, resolved) in resolved_results {
-        resolved_map.insert(job_idx, resolved);
+        resolved_map[job_idx] = Some(resolved);
     }
 
     // 4. Print results and apply updates (sequential — needs ordered output)
@@ -213,7 +231,7 @@ pub async fn run(cli: &Cli) -> Result<bool, Box<dyn std::error::Error + Send + S
             continue;
         }
 
-        let resolved = resolved_map.get(&job_idx).map_or(&[][..], Vec::as_slice);
+        let resolved = resolved_map[job_idx].as_deref().unwrap_or(&[]);
 
         let success_count = resolved.iter().filter(|(_, r)| r.is_ok()).count();
         let fail_count = resolved.len() - success_count;
@@ -282,28 +300,6 @@ fn filter_deps(
         .collect()
 }
 
-/// Resolve versions for dependencies using the appropriate registry.
-async fn resolve_versions(
-    deps: &[DependencySpec],
-    kind: ManifestKind,
-    target: TargetLevel,
-) -> Vec<(usize, Result<ResolvedVersion, DcuError>)> {
-    match kind {
-        ManifestKind::PackageJson => {
-            let registry = NpmRegistry::new();
-            registry.resolve_batch(deps, target).await
-        }
-        ManifestKind::CargoToml => {
-            let registry = CratesIoRegistry::new();
-            registry.resolve_batch(deps, target).await
-        }
-        ManifestKind::PyProjectToml => {
-            let registry = PyPiRegistry::new();
-            registry.resolve_batch(deps, target).await
-        }
-    }
-}
-
 /// Resolved version batch from a registry.
 type ResolvedBatch = Vec<(usize, Result<ResolvedVersion, DcuError>)>;
 

crates/node/src/lib.rs

@@ -41,8 +41,9 @@ impl ManifestHandler for NodeHandler {
     }
 
     fn apply_updates(&self, text: &str, updates: &[PlannedUpdate]) -> Result<String, DcuError> {
+        // Use scan_for_updates: skips full JSON parse, only locates deps we need.
         let locations =
-            JsonPatcher::scan_version_locations(text).map_err(|e| DcuError::PatchFailed {
+            JsonPatcher::scan_for_updates(text, updates).map_err(|e| DcuError::PatchFailed {
                 path: std::path::PathBuf::from("package.json"),
                 detail: e.to_string(),
             })?;

crates/node/src/patcher.rs

@@ -4,7 +4,7 @@
 //! finds the exact byte positions of dependency version strings in the original
 //! text and replaces only those bytes.
 
-use dependency_check_updates_core::DependencySection;
+use dependency_check_updates_core::{DependencySection, PlannedUpdate};
 
 use crate::parser::DEPENDENCY_SECTIONS;
 
@@ -96,6 +96,70 @@ impl JsonPatcher {
         Ok(locations)
     }
 
+    /// Find byte positions of specific dependencies without a full JSON parse.
+    ///
+    /// This is an optimized path for `apply_updates` where we already know which
+    /// deps to look for. Scans only the relevant sections and deps, avoiding
+    /// the cost of deserializing the entire JSON document.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if section positions cannot be found.
+    pub fn scan_for_updates(
+        text: &str,
+        updates: &[PlannedUpdate],
+    ) -> Result<Vec<VersionLocation>, PatchError> {
+        use std::collections::HashMap;
+
+        if updates.is_empty() {
+            return Ok(Vec::new());
+        }
+
+        // Group updates by section for targeted scanning
+        let mut by_section: HashMap<DependencySection, Vec<&PlannedUpdate>> = HashMap::new();
+        for update in updates {
+            by_section.entry(update.section).or_default().push(update);
+        }
+
+        let mut locations = Vec::with_capacity(updates.len());
+
+        for &(section, section_key) in DEPENDENCY_SECTIONS {
+            let Some(section_updates) = by_section.get(&section) else {
+                continue;
+            };
+
+            // Find the byte position of this section key in the text
+            let Some(section_key_pos) = find_json_key_position(text, section_key, 0) else {
+                continue;
+            };
+
+            let search_from = section_key_pos + section_key.len() + 2;
+            let Some(obj_start) = find_char_skipping_strings(text, '{', search_from) else {
+                continue;
+            };
+
+            let Some(obj_end) = find_matching_brace(text, obj_start) else {
+                continue;
+            };
+
+            // Only scan for deps we need to update
+            for update in section_updates {
+                if let Some(loc) = find_dep_value_position(
+                    text,
+                    obj_start,
+                    obj_end,
+                    &update.name,
+                    &update.from,
+                    section,
+                ) {
+                    locations.push(loc);
+                }
+            }
+        }
+
+        Ok(locations)
+    }
+
     /// Apply patches to the original text, replacing version strings.
     ///
     /// Patches are applied back-to-front (highest offset first) so that earlier

crates/node/src/registry.rs

@@ -16,10 +16,11 @@ const MAX_CONCURRENT_REQUESTS: usize = 10;
 const REQUEST_TIMEOUT_SECS: u64 = 30;
 
 /// npm registry client for looking up package versions.
+#[derive(Clone)]
 pub struct NpmRegistry {
     client: Client,
     semaphore: Arc<Semaphore>,
-    base_url: String,
+    base_url: Arc<str>,
 }
 
 /// Abbreviated npm package metadata response.
@@ -61,7 +62,7 @@ impl NpmRegistry {
         Self {
             client,
             semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_REQUESTS)),
-            base_url: base_url.trim_end_matches('/').to_owned(),
+            base_url: Arc::from(base_url.trim_end_matches('/')),
         }
     }
 
@@ -134,16 +135,25 @@ impl NpmRegistry {
         let info = self.fetch_package_info(&dep.name).await?;
 
         let latest = info.dist_tags.as_ref().and_then(|dt| dt.latest.clone());
-        let all_versions = extract_sorted_versions(&info);
 
-        trace!(
-            package = %dep.name,
-            version_count = all_versions.len(),
-            latest = ?latest,
-            "fetched version list"
-        );
-
-        let selected = select_version(&dep.current_req, latest.as_ref(), &all_versions, target);
+        // Fast path: when target is Latest, skip expensive version parsing/sorting
+        let selected = if target == TargetLevel::Latest {
+            trace!(
+                package = %dep.name,
+                latest = ?latest,
+                "fast path: using dist-tags.latest directly"
+            );
+            latest.clone()
+        } else {
+            let all_versions = extract_sorted_versions(&info);
+            trace!(
+                package = %dep.name,
+                version_count = all_versions.len(),
+                latest = ?latest,
+                "fetched version list"
+            );
+            select_version(&dep.current_req, latest.as_ref(), &all_versions, target)
+        };
 
         // Filter out false positives: if the selected version already satisfies
         // the current range, there's no manifest change needed.
@@ -172,16 +182,9 @@ impl NpmRegistry {
 
         for (idx, dep) in deps.iter().enumerate() {
             let dep = dep.clone();
-            let client = self.client.clone();
-            let semaphore = self.semaphore.clone();
-            let base_url = self.base_url.clone();
+            let registry = self.clone();
 
             let handle = tokio::spawn(async move {
-                let registry = NpmRegistry {
-                    client,
-                    semaphore,
-                    base_url,
-                };
                 let result = registry.resolve_version(&dep, target).await;
                 (idx, result)
             });
@@ -199,7 +202,7 @@ impl NpmRegistry {
             }
         }
 
-        results.sort_by_key(|(idx, _)| *idx);
+        results.sort_unstable_by_key(|(idx, _)| *idx);
         results
     }
 }
@@ -221,7 +224,7 @@ fn extract_sorted_versions(info: &NpmPackageInfo) -> Vec<node_semver::Version> {
         .filter_map(|v| node_semver::Version::parse(v).ok())
         .collect();
 
-    parsed.sort();
+    parsed.sort_unstable();
     parsed
 }
 

crates/python/src/registry.rs

@@ -13,17 +13,16 @@ const MAX_CONCURRENT_REQUESTS: usize = 10;
 const REQUEST_TIMEOUT_SECS: u64 = 30;
 
 /// `PyPI` registry client.
+#[derive(Clone)]
 pub struct PyPiRegistry {
     client: Client,
     semaphore: Arc<Semaphore>,
-    base_url: String,
+    base_url: Arc<str>,
 }
 
 #[derive(Debug, Deserialize)]
 struct PyPiResponse {
     info: PyPiInfo,
-    #[allow(dead_code)]
-    releases: std::collections::HashMap<String, Vec<serde_json::Value>>,
 }
 
 #[derive(Debug, Deserialize)]
@@ -57,7 +56,7 @@ impl PyPiRegistry {
         Self {
             client,
             semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_REQUESTS)),
-            base_url: base_url.trim_end_matches('/').to_owned(),
+            base_url: Arc::from(base_url.trim_end_matches('/')),
         }
     }
 
@@ -140,16 +139,9 @@ impl PyPiRegistry {
 
         for (idx, dep) in deps.iter().enumerate() {
             let dep = dep.clone();
-            let client = self.client.clone();
-            let semaphore = self.semaphore.clone();
-            let base_url = self.base_url.clone();
+            let registry = self.clone();
 
             let handle = tokio::spawn(async move {
-                let registry = PyPiRegistry {
-                    client,
-                    semaphore,
-                    base_url,
-                };
                 let result = registry.resolve_version(&dep, target).await;
                 (idx, result)
             });
@@ -165,7 +157,7 @@ impl PyPiRegistry {
             }
         }
 
-        results.sort_by_key(|(idx, _)| *idx);
+        results.sort_unstable_by_key(|(idx, _)| *idx);
         results
     }
 }

crates/rust/src/registry.rs

@@ -13,10 +13,11 @@ const MAX_CONCURRENT_REQUESTS: usize = 10;
 const REQUEST_TIMEOUT_SECS: u64 = 30;
 
 /// crates.io registry client.
+#[derive(Clone)]
 pub struct CratesIoRegistry {
     client: Client,
     semaphore: Arc<Semaphore>,
-    base_url: String,
+    base_url: Arc<str>,
 }
 
 #[derive(Debug, Deserialize)]
@@ -56,7 +57,7 @@ impl CratesIoRegistry {
         Self {
             client,
             semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_REQUESTS)),
-            base_url: base_url.trim_end_matches('/').to_owned(),
+            base_url: Arc::from(base_url.trim_end_matches('/')),
         }
     }
 
@@ -122,7 +123,7 @@ impl CratesIoRegistry {
             .filter(|v| !v.yanked)
             .filter_map(|v| semver::Version::parse(&v.num).ok())
             .collect();
-        versions.sort();
+        versions.sort_unstable();
 
         trace!(
             crate_name = %dep.name,
@@ -164,16 +165,9 @@ impl CratesIoRegistry {
 
         for (idx, dep) in deps.iter().enumerate() {
             let dep = dep.clone();
-            let client = self.client.clone();
-            let semaphore = self.semaphore.clone();
-            let base_url = self.base_url.clone();
+            let registry = self.clone();
 
             let handle = tokio::spawn(async move {
-                let registry = CratesIoRegistry {
-                    client,
-                    semaphore,
-                    base_url,
-                };
                 let result = registry.resolve_version(&dep, target).await;
                 (idx, result)
             });
@@ -189,7 +183,7 @@ impl CratesIoRegistry {
             }
         }
 
-        results.sort_by_key(|(idx, _)| *idx);
+        results.sort_unstable_by_key(|(idx, _)| *idx);
         results
     }
 }