From 53f4444c5ac9cc1c14749afb7ba2c3ccd1929889 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 25 Jul 2026 19:10:47 +0900 Subject: perf(advisory): share AnySecurityAdvisory via Rc PHP's SecurityAdvisoryPoolFilter stores advisory *object references* in $securityRemovedVersions, and PoolOptimizer::applyRemovalsToPool hands that array to the new Pool by copy-on-write. Porting AnySecurityAdvisory as a value type turned both of those into deep copies. Measured on `require laravel/framework` (offline, warm cache): 113 distinct advisories were duplicated into 314,309 copies of ~1.08 KiB, retaining 331.9 MiB in the filter loop and another 331.9 MiB when apply_removals_to_pool cloned the whole map. 3.54s -> 2.62s (-26%), peak RSS 975 MB -> 343 MB, which matches the upper bound measured by ablation. Composer runs the same workload in 1.49s. Co-Authored-By: Claude Opus 5 (1M context) --- crates/shirabe/src/advisory/auditor.rs | 49 ++++++++++++---------- crates/shirabe/src/dependency_resolver/pool.rs | 10 +++-- .../security_advisory_pool_filter.rs | 8 ++-- .../src/repository/advisory_provider_interface.rs | 2 +- .../shirabe/src/repository/composer_repository.rs | 19 +++++---- .../shirabe/src/repository/package_repository.rs | 8 ++-- crates/shirabe/src/repository/repository_set.rs | 10 +++-- 7 files changed, 58 insertions(+), 48 deletions(-) (limited to 'crates/shirabe/src') diff --git a/crates/shirabe/src/advisory/auditor.rs b/crates/shirabe/src/advisory/auditor.rs index ff93268a..41cef838 100644 --- a/crates/shirabe/src/advisory/auditor.rs +++ b/crates/shirabe/src/advisory/auditor.rs @@ -23,9 +23,9 @@ use shirabe_php_shim::{ #[derive(serde::Serialize)] struct AuditJsonReport<'a> { #[serde(serialize_with = "serialize_advisories_field")] - advisories: &'a IndexMap>, + advisories: &'a IndexMap>>, #[serde(rename = "ignored-advisories", skip_serializing_if = "Option::is_none")] - ignored_advisories: Option<&'a IndexMap>>, + ignored_advisories: Option<&'a IndexMap>>>, #[serde( rename = "unreachable-repositories", skip_serializing_if = "Option::is_none" @@ -55,7 +55,7 @@ where } fn serialize_advisories_field( - map: &&IndexMap>, + map: &&IndexMap>>, serializer: S, ) -> anyhow::Result where @@ -197,7 +197,7 @@ impl Auditor { let error_or_warn = if warning_only { "warning" } else { "error" }; if affected_packages_count > 0 || !ignored_advisories.is_empty() { let passes: Vec<( - &IndexMap>, + &IndexMap>>, String, )> = vec![ ( @@ -263,7 +263,7 @@ impl Auditor { /// @return bool pub fn needs_complete_advisory_load( &self, - advisories: &IndexMap>, + advisories: &IndexMap>>, ignore_list: &IndexMap>, ) -> bool { if advisories.is_empty() { @@ -271,15 +271,15 @@ impl Auditor { } // no partial advisories present - let advisories_values: Vec<&Vec> = advisories.values().collect(); - if array_all( - &advisories_values, - |pkg_advisories: &&Vec| { - array_all(pkg_advisories, |advisory: &AnySecurityAdvisory| { + let advisories_values: Vec<_> = advisories.values().collect(); + if array_all(&advisories_values, |pkg_advisories| { + array_all( + pkg_advisories, + |advisory: &std::rc::Rc| { advisory.as_security_advisory().is_some() - }) - }, - ) { + }, + ) + }) { return false; } @@ -321,7 +321,7 @@ impl Auditor { pub fn process_advisories( &self, - all_advisories: IndexMap>, + all_advisories: IndexMap>>, ignore_list: &IndexMap>, ignored_severities: &IndexMap>, ) -> ProcessAdvisoriesResult { @@ -332,8 +332,9 @@ impl Auditor { }; } - let mut advisories: IndexMap> = IndexMap::new(); - let mut ignored: IndexMap> = IndexMap::new(); + let mut advisories: IndexMap>> = + IndexMap::new(); + let mut ignored: IndexMap>> = IndexMap::new(); let mut ignore_reason: Option = None; for (package, pkg_advisories) in all_advisories { @@ -394,7 +395,9 @@ impl Auditor { // and in that case we do not need to cast the object. let advisory = if advisory.as_security_advisory().is_some() { let full = advisory.as_security_advisory().unwrap(); - AnySecurityAdvisory::Ignored(full.to_ignored_advisory(ignore_reason.clone())) + std::rc::Rc::new(AnySecurityAdvisory::Ignored( + full.to_ignored_advisory(ignore_reason.clone()), + )) } else { advisory }; @@ -412,7 +415,7 @@ impl Auditor { /// @return array{int, int} Count of affected packages and total count of advisories fn count_advisories( &self, - advisories: &IndexMap>, + advisories: &IndexMap>>, ) -> (i64, i64) { let mut count: i64 = 0; for package_advisories in advisories.values() { @@ -427,7 +430,7 @@ impl Auditor { fn output_advisories( &self, io: &std::rc::Rc>, - advisories: &IndexMap>, + advisories: &IndexMap>>, format: &str, ) -> anyhow::Result<()> { match format { @@ -473,7 +476,7 @@ impl Auditor { fn output_advisories_table( &self, io: &ConsoleIO, - advisories: &IndexMap>, + advisories: &IndexMap>>, ) -> anyhow::Result<()> { for package_advisories in advisories.values() { for advisory in package_advisories { @@ -534,7 +537,7 @@ impl Auditor { fn output_advisories_plain( &self, io: &std::rc::Rc>, - advisories: &IndexMap>, + advisories: &IndexMap>>, ) -> anyhow::Result<()> { let mut error: Vec = vec![]; let mut first_advisory = true; @@ -745,6 +748,6 @@ impl Auditor { #[derive(Debug)] pub struct ProcessAdvisoriesResult { - pub advisories: IndexMap>, - pub ignored_advisories: IndexMap>, + pub advisories: IndexMap>>, + pub ignored_advisories: IndexMap>>, } diff --git a/crates/shirabe/src/dependency_resolver/pool.rs b/crates/shirabe/src/dependency_resolver/pool.rs index adf42f54..a9dffe71 100644 --- a/crates/shirabe/src/dependency_resolver/pool.rs +++ b/crates/shirabe/src/dependency_resolver/pool.rs @@ -28,7 +28,8 @@ pub struct Pool { /// @var array> Map of package object hash => removed normalized versions => removed pretty version pub(crate) removed_versions_by_package: IndexMap>, /// @var array>> Map of package name => normalized version => security advisories - security_removed_versions: IndexMap>>, + security_removed_versions: + IndexMap>>>, /// @var array> Map of package name => normalized version => pretty version abandoned_removed_versions: IndexMap>, } @@ -45,7 +46,10 @@ impl Pool { unacceptable_fixed_or_locked_packages: Vec, removed_versions: IndexMap>, removed_versions_by_package: IndexMap>, - security_removed_versions: IndexMap>>, + security_removed_versions: IndexMap< + String, + IndexMap>>, + >, abandoned_removed_versions: IndexMap>, ) -> Self { let mut this = Self { @@ -182,7 +186,7 @@ impl Pool { /// @return array>> pub fn get_all_security_removed_package_versions( &self, - ) -> &IndexMap>> { + ) -> &IndexMap>>> { &self.security_removed_versions } diff --git a/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs b/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs index 59acad89..c2b95bd7 100644 --- a/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs +++ b/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs @@ -87,7 +87,7 @@ impl SecurityAdvisoryPoolFilter { let mut packages: Vec = vec![]; let mut security_removed_versions: IndexMap< String, - IndexMap>, + IndexMap>>, > = IndexMap::new(); let mut abandoned_removed_versions: IndexMap> = IndexMap::new(); @@ -144,13 +144,13 @@ impl SecurityAdvisoryPoolFilter { fn get_matching_advisories( &self, package: BasePackageHandle, - advisory_map: &IndexMap>, - ) -> Vec { + advisory_map: &IndexMap>>, + ) -> Vec> { if package.is_dev() { return vec![]; } - let mut matching_advisories: Vec = vec![]; + let mut matching_advisories = vec![]; for package_name in package.get_names(false) { if !advisory_map.contains_key(&package_name) { continue; diff --git a/crates/shirabe/src/repository/advisory_provider_interface.rs b/crates/shirabe/src/repository/advisory_provider_interface.rs index f1e639a1..466093c1 100644 --- a/crates/shirabe/src/repository/advisory_provider_interface.rs +++ b/crates/shirabe/src/repository/advisory_provider_interface.rs @@ -7,7 +7,7 @@ use shirabe_semver::constraint::AnyConstraint; #[derive(Debug)] pub struct SecurityAdvisoryResult { pub names_found: Vec, - pub advisories: IndexMap>, + pub advisories: IndexMap>>, } pub trait AdvisoryProviderInterface { diff --git a/crates/shirabe/src/repository/composer_repository.rs b/crates/shirabe/src/repository/composer_repository.rs index 876ba3b8..558d7e00 100644 --- a/crates/shirabe/src/repository/composer_repository.rs +++ b/crates/shirabe/src/repository/composer_repository.rs @@ -920,7 +920,7 @@ impl ComposerRepository { }); } - let mut advisories: IndexMap> = IndexMap::new(); + let mut advisories = IndexMap::new(); let mut names_found: IndexMap = IndexMap::new(); let api_url = self @@ -943,7 +943,7 @@ impl ComposerRepository { let create = |data: &IndexMap, name: &str, package_constraint_map: &IndexMap| - -> anyhow::Result> { + -> anyhow::Result>> { let advisory = PartialSecurityAdvisory::create(name, data, &parser)?; let is_full = matches!(advisory, AnySecurityAdvisory::Full(_)); if !allow_partial_advisories && !is_full { @@ -971,7 +971,7 @@ impl ComposerRepository { return Ok(None); } - Ok(Some(advisory)) + Ok(Some(std::rc::Rc::new(advisory))) }; if self @@ -1030,7 +1030,7 @@ impl ComposerRepository { names_found.insert(name.clone(), true); if !sec_advs_arr.is_empty() { - let mut entries: Vec = Vec::new(); + let mut entries: Vec> = Vec::new(); for data_mixed in sec_advs_arr.into_iter() { if let PhpMixed::Array(data_map) = data_mixed && let Some(adv) = create(&data_map, &name, &package_constraint_map)? @@ -1112,7 +1112,7 @@ impl ComposerRepository { None => continue, }; if !list.is_empty() { - let mut entries: Vec = Vec::new(); + let mut entries: Vec> = Vec::new(); for data_mixed in list.iter() { if let Some(data) = data_mixed.as_array() { let data_map: IndexMap = @@ -1128,10 +1128,11 @@ impl ComposerRepository { } } - let advisories_filtered: IndexMap> = advisories - .into_iter() - .filter(|(_, adv)| !adv.is_empty()) - .collect(); + let advisories_filtered: IndexMap>> = + advisories + .into_iter() + .filter(|(_, adv)| !adv.is_empty()) + .collect(); Ok(SecurityAdvisoryResult { names_found: names_found.keys().cloned().collect(), diff --git a/crates/shirabe/src/repository/package_repository.rs b/crates/shirabe/src/repository/package_repository.rs index 7f61bc9e..3397f071 100644 --- a/crates/shirabe/src/repository/package_repository.rs +++ b/crates/shirabe/src/repository/package_repository.rs @@ -211,7 +211,7 @@ impl AdvisoryProviderInterface for PackageRepository { ) -> anyhow::Result { let parser = VersionParser::new(); - let mut advisories: IndexMap> = IndexMap::new(); + let mut advisories = IndexMap::new(); for (package_name, package_advisories) in &self.security_advisories { let Some(package_constraint) = package_constraint_map.get(package_name) else { continue; @@ -221,7 +221,7 @@ impl AdvisoryProviderInterface for PackageRepository { PhpMixed::List(list) => list, _ => continue, }; - let mut items: Vec = Vec::new(); + let mut items: Vec> = Vec::new(); for data in list { let data_map: IndexMap = match data { PhpMixed::Array(m) => m.clone(), @@ -249,13 +249,13 @@ impl AdvisoryProviderInterface for PackageRepository { continue; } - items.push(advisory); + items.push(std::rc::Rc::new(advisory)); } advisories.insert(package_name.clone(), items); } let names_found: Vec = advisories.keys().cloned().collect(); - let advisories: IndexMap> = advisories + let advisories: IndexMap>> = advisories .into_iter() .filter(|(_, adv)| !adv.is_empty()) .collect(); diff --git a/crates/shirabe/src/repository/repository_set.rs b/crates/shirabe/src/repository/repository_set.rs index c407a1ce..8443400f 100644 --- a/crates/shirabe/src/repository/repository_set.rs +++ b/crates/shirabe/src/repository/repository_set.rs @@ -363,8 +363,9 @@ impl RepositorySet { allow_partial_advisories: bool, ignore_unreachable: bool, unreachable_repos: &mut Vec, - ) -> anyhow::Result>> { - let mut repo_advisories: Vec>> = vec![]; + ) -> anyhow::Result>>> { + let mut repo_advisories: Vec>>> = + vec![]; for repository in &self.repositories { let attempt: anyhow::Result<()> = (|| -> anyhow::Result<()> { let mut repo_ref = repository.borrow_mut(); @@ -403,7 +404,8 @@ impl RepositorySet { } } - let mut advisories: IndexMap> = IndexMap::new(); + let mut advisories: IndexMap>> = + IndexMap::new(); for repo in repo_advisories { for (name, list) in repo { advisories.entry(name).or_default().extend(list); @@ -672,6 +674,6 @@ impl RepositorySetInterface for RepositorySet { #[derive(Debug)] pub struct SecurityAdvisoriesResult { - pub advisories: IndexMap>, + pub advisories: IndexMap>>, pub unreachable_repos: Vec, } -- cgit v1.3.1