diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-18 09:36:34 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-18 09:41:33 +0900 |
| commit | 0feaef06d0a4b3476a5d2ac53119a67559cd0a82 (patch) | |
| tree | 049609246698ed1f9da3187d5e917cc0f213b923 /crates/shirabe/src | |
| parent | 4856f05e870ec5d2daaa74bf9a86a1519a813614 (diff) | |
| download | php-shirabe-0feaef06d0a4b3476a5d2ac53119a67559cd0a82.tar.gz php-shirabe-0feaef06d0a4b3476a5d2ac53119a67559cd0a82.tar.zst php-shirabe-0feaef06d0a4b3476a5d2ac53119a67559cd0a82.zip | |
perf(metadata-minifier): defer copying versions out of expand
MetadataMinifier::expand now returns ExpandedVersions, which holds the
minified input plus, for each expanded version, a table of references to
where its fields live. A version is copied only when materialize() asks
for it. ComposerRepository::load_async_packages runs its constraint and
stability filters straight off that view through the new VersionFields
trait, so the versions it rejects are never copied at all.
Benchmarks are new under crates/shirabe/benches. load_packages against
real packagist p2 metadata, before -> after:
symfony/console (768 versions)
0 accepted 9.01 ms -> 4.40 ms -51%
50 accepted 10.70 ms -> 6.30 ms -41%
147 accepted 13.17 ms -> 9.62 ms -27%
329 accepted 18.58 ms -> 16.89 ms -9%
663 accepted 27.27 ms -> 27.89 ms +2%
laravel/framework (1277 versions)
0 accepted 39.44 ms -> 11.22 ms -72%
81 accepted 44.76 ms -> 18.25 ms -59%
840 accepted 125.44 ms -> 120.12 ms -4%
1266 accepted 163.18 ms -> 182.01 ms +12%
The crossover sits near 80% acceptance. Past it the view loses, because
materialize rebuilds a map where the old code cloned one, and the
minified input stays alive alongside the copies; the 1266-of-1277 case
measured between +5% and +12% across runs. Loads with a real constraint
sit far below the crossover.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe/src')
| -rw-r--r-- | crates/shirabe/src/package/loader/array_loader.rs | 26 | ||||
| -rw-r--r-- | crates/shirabe/src/repository/composer_repository.rs | 138 |
2 files changed, 128 insertions, 36 deletions
diff --git a/crates/shirabe/src/package/loader/array_loader.rs b/crates/shirabe/src/package/loader/array_loader.rs index 698e1f03..3e66e506 100644 --- a/crates/shirabe/src/package/loader/array_loader.rs +++ b/crates/shirabe/src/package/loader/array_loader.rs @@ -23,6 +23,27 @@ use shirabe_php_shim::{ strval, substr, trigger_error, trim, }; +/// The fields of one package version, read without regard for how they are stored. A version +/// array decoded from a repository response implements this, and so does one still held as a view +/// into minified metadata. +pub trait VersionFields { + fn get(&self, key: &str) -> Option<&PhpMixed>; + + fn contains_key(&self, key: &str) -> bool { + self.get(key).is_some() + } +} + +impl VersionFields for IndexMap<String, PhpMixed> { + fn get(&self, key: &str) -> Option<&PhpMixed> { + IndexMap::get(self, key) + } + + fn contains_key(&self, key: &str) -> bool { + IndexMap::contains_key(self, key) + } +} + #[derive(Debug)] pub struct ArrayLoader { /// @var VersionParser @@ -689,10 +710,7 @@ impl ArrayLoader { /// @param mixed[] $config the entire package config /// /// @return string|null normalized version of the branch alias or null if there is none - pub fn get_branch_alias( - &self, - config: &IndexMap<String, PhpMixed>, - ) -> anyhow::Result<Option<String>> { + pub fn get_branch_alias(&self, config: &impl VersionFields) -> anyhow::Result<Option<String>> { if !config.contains_key("version") || !is_scalar(config.get("version").unwrap()) { return Err( UnexpectedValueException::new("no/invalid version defined".to_string()).into(), diff --git a/crates/shirabe/src/repository/composer_repository.rs b/crates/shirabe/src/repository/composer_repository.rs index 1b160ae6..411c0abe 100644 --- a/crates/shirabe/src/repository/composer_repository.rs +++ b/crates/shirabe/src/repository/composer_repository.rs @@ -13,6 +13,7 @@ use crate::package::BasePackageHandle; use crate::package::PackageInterfaceHandle; use crate::package::base_package; use crate::package::loader::ArrayLoader; +use crate::package::loader::VersionFields; use crate::package::version::StabilityFilter; use crate::package::version::VersionParser; use crate::plugin::PluginEvents; @@ -36,6 +37,8 @@ use crate::util::sync_executor; use futures::StreamExt; use futures::stream::FuturesOrdered; use indexmap::IndexMap; +use shirabe_metadata_minifier::ExpandedVersion; +use shirabe_metadata_minifier::ExpandedVersions; use shirabe_metadata_minifier::MetadataMinifier; use shirabe_php_shim::{ AnyThrowable, CmpOp, InvalidArgumentException, LogicException, PHP_EOL, PhpMixed, @@ -139,6 +142,84 @@ pub struct LoadAsyncPackagesResult { pub packages: IndexMap<String, BasePackageHandle>, } +/// The version list `load_async_packages` walks. A response in the minified format is expanded +/// lazily, so a version is copied out only once the filters accept it; any other response is +/// walked as it was decoded. +#[derive(Debug)] +enum VersionList { + Plain(Vec<IndexMap<String, PhpMixed>>), + Minified(ExpandedVersions), +} + +impl VersionList { + fn len(&self) -> usize { + match self { + Self::Plain(versions) => versions.len(), + Self::Minified(expanded) => expanded.len(), + } + } + + fn version(&self, index: usize) -> Version<'_> { + match self { + Self::Plain(versions) => Version::Plain(&versions[index]), + Self::Minified(expanded) => Version::Minified(expanded.version(index)), + } + } + + fn set(&mut self, index: usize, key: &str, val: PhpMixed) { + match self { + Self::Plain(versions) => { + versions[index].insert(key.to_string(), val); + } + Self::Minified(expanded) => expanded.set(index, key, val), + } + } + + /// Copies the version out for `create_packages`. It is not read again afterwards. + fn take(&mut self, index: usize) -> IndexMap<String, PhpMixed> { + match self { + Self::Plain(versions) => std::mem::take(&mut versions[index]), + Self::Minified(expanded) => expanded.materialize(index), + } + } +} + +#[derive(Debug, Clone, Copy)] +enum Version<'a> { + Plain(&'a IndexMap<String, PhpMixed>), + Minified(ExpandedVersion<'a>), +} + +impl<'a> Version<'a> { + fn get(&self, key: &str) -> Option<&'a PhpMixed> { + match self { + Self::Plain(version) => version.get(key), + Self::Minified(version) => version.get(key), + } + } + + fn get_string(&self, key: &str) -> Option<&'a str> { + self.get(key).and_then(|v| v.as_string()) + } + + fn contains_key(&self, key: &str) -> bool { + match self { + Self::Plain(version) => version.contains_key(key), + Self::Minified(version) => version.contains_key(key), + } + } +} + +impl VersionFields for Version<'_> { + fn get(&self, key: &str) -> Option<&PhpMixed> { + Version::get(self, key) + } + + fn contains_key(&self, key: &str) -> bool { + Version::contains_key(self, key) + } +} + impl ConfigurableRepositoryInterface for ComposerRepository { fn get_repo_config(&self) -> IndexMap<String, PhpMixed> { self.repo_config.clone() @@ -1810,7 +1891,7 @@ impl ComposerRepository { None => continue, }; - let mut versions: Vec<IndexMap<String, PhpMixed>> = match versions_mixed { + let versions: Vec<IndexMap<String, PhpMixed>> = match versions_mixed { PhpMixed::List(l) => l .into_iter() .filter_map(|v| match v { @@ -1830,47 +1911,40 @@ impl ComposerRepository { let minified = response_arr.get("minified").and_then(|v| v.as_string()) == Some("composer/2.0"); - if minified { - versions = MetadataMinifier::expand(versions); - } + let mut versions = if minified { + VersionList::Minified(MetadataMinifier::expand(versions)) + } else { + VersionList::Plain(versions) + }; names_found.insert(real_name.clone(), true); let mut versions_to_load: Vec<IndexMap<String, PhpMixed>> = Vec::new(); - for version in versions.into_iter() { - let mut version = version; - let has_vn = version.contains_key("version_normalized"); + for index in 0..versions.len() { + let has_vn = versions.version(index).contains_key("version_normalized"); if !has_vn { - let v = version - .get("version") - .and_then(|v| v.as_string()) + let v = versions + .version(index) + .get_string("version") .unwrap_or("") .to_string(); let normalized = version_parser.normalize(&v, None)?; - version.insert( - "version_normalized".to_string(), - PhpMixed::String(normalized), - ); - } else if version - .get("version_normalized") - .and_then(|v| v.as_string()) + versions.set(index, "version_normalized", PhpMixed::String(normalized)); + } else if versions.version(index).get_string("version_normalized") == Some(VersionParser::DEFAULT_BRANCH_ALIAS) { // handling of existing repos which need to remain composer v1 compatible, in case the version_normalized contained VersionParser::DEFAULT_BRANCH_ALIAS, we renormalize it - let v = version - .get("version") - .and_then(|v| v.as_string()) + let v = versions + .version(index) + .get_string("version") .unwrap_or("") .to_string(); let normalized = version_parser.normalize(&v, None)?; - version.insert( - "version_normalized".to_string(), - PhpMixed::String(normalized), - ); + versions.set(index, "version_normalized", PhpMixed::String(normalized)); } - let version_normalized = version - .get("version_normalized") - .and_then(|v| v.as_string()) + let version_normalized = versions + .version(index) + .get_string("version_normalized") .unwrap_or("") .to_string(); // avoid loading packages which have already been loaded @@ -1884,12 +1958,12 @@ impl ComposerRepository { let acceptable = ComposerRepository::is_version_acceptable_static( constraint.as_ref(), &real_name, - &version, + &versions.version(index), acceptable_stabilities, stability_flags, )?; if acceptable { - versions_to_load.push(version); + versions_to_load.push(versions.take(index)); } } @@ -2007,7 +2081,7 @@ impl ComposerRepository { &self, constraint: Option<&AnyConstraint>, name: &str, - version_data: &IndexMap<String, PhpMixed>, + version_data: &impl VersionFields, acceptable_stabilities: Option<&IndexMap<String, i64>>, stability_flags: Option<&IndexMap<String, i64>>, ) -> anyhow::Result<bool> { @@ -2024,7 +2098,7 @@ impl ComposerRepository { fn is_version_acceptable_static( constraint: Option<&AnyConstraint>, name: &str, - version_data: &IndexMap<String, PhpMixed>, + version_data: &impl VersionFields, acceptable_stabilities: Option<&IndexMap<String, i64>>, stability_flags: Option<&IndexMap<String, i64>>, ) -> anyhow::Result<bool> { @@ -2042,7 +2116,7 @@ impl ComposerRepository { loader: &ArrayLoader, constraint: Option<&AnyConstraint>, name: &str, - version_data: &IndexMap<String, PhpMixed>, + version_data: &impl VersionFields, acceptable_stabilities: Option<&IndexMap<String, i64>>, stability_flags: Option<&IndexMap<String, i64>>, ) -> anyhow::Result<bool> { |
