From 0feaef06d0a4b3476a5d2ac53119a67559cd0a82 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 18 Aug 2026 09:36:34 +0900 Subject: 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 --- .../src/metadata_minifier.rs | 304 ++++++++++++++++++++- crates/shirabe/Cargo.toml | 9 + crates/shirabe/benches/composer_repository.rs | 195 +++++++++++++ crates/shirabe/benches/metadata_minifier.rs | 98 +++++++ crates/shirabe/benches/packagist_fixture.rs | 26 ++ crates/shirabe/src/package/loader/array_loader.rs | 26 +- .../shirabe/src/repository/composer_repository.rs | 138 +++++++--- .../shirabe/tests/util/metadata_minifier_test.rs | 2 +- 8 files changed, 747 insertions(+), 51 deletions(-) create mode 100644 crates/shirabe/benches/composer_repository.rs create mode 100644 crates/shirabe/benches/metadata_minifier.rs create mode 100644 crates/shirabe/benches/packagist_fixture.rs diff --git a/crates/shirabe-metadata-minifier/src/metadata_minifier.rs b/crates/shirabe-metadata-minifier/src/metadata_minifier.rs index 3f1a4fc6..ef72eb4a 100644 --- a/crates/shirabe-metadata-minifier/src/metadata_minifier.rs +++ b/crates/shirabe-metadata-minifier/src/metadata_minifier.rs @@ -7,32 +7,308 @@ use shirabe_php_shim::PhpMixed; pub struct MetadataMinifier; impl MetadataMinifier { - pub fn expand(versions: Vec>) -> Vec> { - let mut expanded: Vec> = Vec::new(); - let mut expanded_version: Option> = None; - for version_data in versions { - if expanded_version.as_ref().is_none_or(|ev| ev.is_empty()) { - expanded.push(version_data.clone()); - expanded_version = Some(version_data); + /// Expands an array of minified versions, keeping each expanded version as a table of + /// references into `versions` rather than a copy of the values. Callers that discard most + /// versions (a repository filtering by constraint and stability, say) pay for a deep copy only + /// on the ones they keep. + pub fn expand(versions: Vec>) -> ExpandedVersions { + let mut keys: IndexMap = IndexMap::new(); + let mut views: Vec> = Vec::with_capacity(versions.len()); + let mut current: Vec = Vec::new(); + + for (version_index, version_data) in versions.iter().enumerate() { + let version_index = version_index as u32; + + if current.is_empty() { + for (entry_index, (key, _)) in version_data.iter().enumerate() { + let key = intern(&mut keys, key); + current.push(Entry { + key, + value: Slot::Minified { + version_index, + entry_index: entry_index as u32, + }, + }); + } + views.push(current.clone()); continue; } // add any changes from the previous version to the expanded one - let ev = expanded_version.as_mut().unwrap(); - for (key, val) in version_data { - if matches!(&val, PhpMixed::String(s) if s == "__unset") { - ev.shift_remove(&key); + for (entry_index, (key, val)) in version_data.iter().enumerate() { + let key = intern(&mut keys, key); + if matches!(val, PhpMixed::String(s) if s == "__unset") { + if let Some(position) = current.iter().position(|entry| entry.key == key) { + current.remove(position); + } } else { - ev.insert(key, val); + let value = Slot::Minified { + version_index, + entry_index: entry_index as u32, + }; + match current.iter_mut().find(|entry| entry.key == key) { + Some(entry) => entry.value = value, + None => current.push(Entry { key, value }), + } } } - expanded.push(ev.clone()); + views.push(current.clone()); } - expanded + ExpandedVersions { + source: versions, + keys, + views, + overrides: Vec::new(), + } } // MetadataMinifier::minify() is not ported because it is not used in Composer itself. // The function is mainly for package repositories. } + +fn intern(keys: &mut IndexMap, key: &str) -> u32 { + if let Some(&id) = keys.get(key) { + return id; + } + let id = keys.len() as u32; + keys.insert(key.to_string(), id); + id +} + +/// One field of one expanded version. +#[derive(Debug, Clone, Copy)] +struct Entry { + key: u32, + value: Slot, +} + +#[derive(Debug, Clone, Copy)] +enum Slot { + /// A value of the minified input: the version that last wrote the field, and the position the + /// field has within it. + Minified { + version_index: u32, + entry_index: u32, + }, + /// A value written by [`ExpandedVersions::set`], at this position in `overrides`. + Overridden(u32), +} + +/// The result of [`MetadataMinifier::expand`]: the minified input plus, for each expanded +/// version, the fields it ends up with and where their values live. +#[derive(Debug)] +pub struct ExpandedVersions { + source: Vec>, + keys: IndexMap, + views: Vec>, + overrides: Vec, +} + +impl ExpandedVersions { + pub fn len(&self) -> usize { + self.views.len() + } + + pub fn is_empty(&self) -> bool { + self.views.is_empty() + } + + pub fn version(&self, index: usize) -> ExpandedVersion<'_> { + ExpandedVersion { + versions: self, + index, + } + } + + /// Writes one field of one expanded version, leaving the others alone. A field the version + /// does not have yet is appended, as assigning to a missing key would in PHP. + pub fn set(&mut self, index: usize, key: &str, val: PhpMixed) { + let value = Slot::Overridden(self.overrides.len() as u32); + self.overrides.push(val); + + let key = intern(&mut self.keys, key); + match self.views[index].iter_mut().find(|entry| entry.key == key) { + Some(entry) => entry.value = value, + None => self.views[index].push(Entry { key, value }), + } + } + + /// Copies one expanded version out, in the same field order [`MetadataMinifier::expand`] + /// produces. + pub fn materialize(&self, index: usize) -> IndexMap { + let view = &self.views[index]; + let mut version: IndexMap = IndexMap::with_capacity(view.len()); + for entry in view { + version.insert(self.key_of(entry).clone(), self.value_of(entry).clone()); + } + version + } + + pub fn into_vec(self) -> Vec> { + (0..self.len()) + .map(|index| self.materialize(index)) + .collect() + } + + fn key_of(&self, entry: &Entry) -> &String { + self.keys + .get_index(entry.key as usize) + .expect("every interned key id is an index into `keys`") + .0 + } + + fn value_of(&self, entry: &Entry) -> &PhpMixed { + match entry.value { + Slot::Minified { + version_index, + entry_index, + } => { + self.source[version_index as usize] + .get_index(entry_index as usize) + .expect("expand recorded an entry index that is not in the minified input") + .1 + } + Slot::Overridden(index) => &self.overrides[index as usize], + } + } +} + +/// One expanded version of an [`ExpandedVersions`], readable without copying it out. +#[derive(Debug, Clone, Copy)] +pub struct ExpandedVersion<'a> { + versions: &'a ExpandedVersions, + index: usize, +} + +impl<'a> ExpandedVersion<'a> { + pub fn get(&self, key: &str) -> Option<&'a PhpMixed> { + let key = *self.versions.keys.get(key)?; + let entry = self.versions.views[self.index] + .iter() + .find(|entry| entry.key == key)?; + Some(self.versions.value_of(entry)) + } + + pub fn contains_key(&self, key: &str) -> bool { + let Some(&key) = self.versions.keys.get(key) else { + return false; + }; + self.versions.views[self.index] + .iter() + .any(|entry| entry.key == key) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn version(fields: &[(&str, &str)]) -> IndexMap { + fields + .iter() + .map(|(key, val)| (key.to_string(), PhpMixed::String(val.to_string()))) + .collect() + } + + fn versions() -> Vec> { + vec![ + version(&[("name", "foo/bar"), ("version", "3.0"), ("type", "library")]), + version(&[ + ("version", "2.0"), + ("type", "__unset"), + ("abandoned", "yes"), + ]), + version(&[("version", "1.0"), ("type", "library")]), + // every field of the previous version is dropped, so the next one restarts from scratch + version(&[ + ("name", "__unset"), + ("version", "__unset"), + ("abandoned", "__unset"), + ("type", "__unset"), + ]), + version(&[("name", "foo/baz"), ("version", "0.1")]), + ] + } + + fn expanded() -> Vec> { + vec![ + version(&[("name", "foo/bar"), ("version", "3.0"), ("type", "library")]), + version(&[ + ("name", "foo/bar"), + ("version", "2.0"), + ("abandoned", "yes"), + ]), + version(&[ + ("name", "foo/bar"), + ("version", "1.0"), + ("abandoned", "yes"), + ("type", "library"), + ]), + IndexMap::new(), + version(&[("name", "foo/baz"), ("version", "0.1")]), + ] + } + + #[test] + fn expand_applies_each_diff_to_the_previous_version() { + assert_eq!(MetadataMinifier::expand(versions()).into_vec(), expanded()); + } + + #[test] + fn a_version_reads_the_same_as_the_materialized_one() { + let versions = MetadataMinifier::expand(versions()); + + for (index, expected) in expanded().iter().enumerate() { + let version = versions.version(index); + for key in ["name", "version", "type", "abandoned", "missing"] { + assert_eq!(version.get(key), expected.get(key), "{index} {key}"); + assert_eq!( + version.contains_key(key), + expected.contains_key(key), + "{index} {key}", + ); + } + } + } + + #[test] + fn set_overrides_a_field_of_one_version_only() { + let mut versions = MetadataMinifier::expand(versions()); + versions.set(1, "version", PhpMixed::String("2.0.0.0".to_string())); + + assert_eq!( + versions.version(1).get("version"), + Some(&PhpMixed::String("2.0.0.0".to_string())), + ); + assert_eq!( + versions.version(2).get("version"), + Some(&PhpMixed::String("1.0".to_string())), + ); + + let mut expected = expanded(); + expected[1].insert( + "version".to_string(), + PhpMixed::String("2.0.0.0".to_string()), + ); + assert_eq!(versions.into_vec(), expected); + } + + #[test] + fn set_appends_a_field_the_version_does_not_have() { + let mut versions = MetadataMinifier::expand(versions()); + versions.set( + 0, + "version_normalized", + PhpMixed::String("3.0.0.0".to_string()), + ); + + let mut expected = expanded(); + expected[0].insert( + "version_normalized".to_string(), + PhpMixed::String("3.0.0.0".to_string()), + ); + assert_eq!(versions.into_vec(), expected); + } +} diff --git a/crates/shirabe/Cargo.toml b/crates/shirabe/Cargo.toml index d3e425b2..66f2b5fc 100644 --- a/crates/shirabe/Cargo.toml +++ b/crates/shirabe/Cargo.toml @@ -4,6 +4,7 @@ version.workspace = true edition.workspace = true rust-version.workspace = true description = "A Rust port of Composer, the dependency manager for PHP" +autobenches = false repository.workspace = true license.workspace = true @@ -49,5 +50,13 @@ tempfile.workspace = true name = "process_executor" harness = false +[[bench]] +name = "metadata_minifier" +harness = false + +[[bench]] +name = "composer_repository" +harness = false + [lints] workspace = true diff --git a/crates/shirabe/benches/composer_repository.rs b/crates/shirabe/benches/composer_repository.rs new file mode 100644 index 00000000..7c8f05da --- /dev/null +++ b/crates/shirabe/benches/composer_repository.rs @@ -0,0 +1,195 @@ +//! Benchmarks for `ComposerRepository::load_packages` against a v2 (metadata-url) repository. +//! +//! The responses are real packagist p2 files served through the HTTP downloader mock, so one +//! iteration covers what `composer update` does per package: decode the response, expand the +//! minified metadata, filter by constraint and stability, and build the accepted packages. The +//! constraint decides how many versions survive that filter, which is what the lazy expansion is +//! meant to exploit. + +use criterion::BatchSize; +use criterion::BenchmarkId; +use criterion::Criterion; +use criterion::criterion_group; +use criterion::criterion_main; +use indexmap::IndexMap; +use shirabe::config::Config; +use shirabe::io::IOInterface; +use shirabe::io::null_io::NullIO; +use shirabe::json::json_file::JsonFile; +use shirabe::package::base_package::STABILITY_STABLE; +use shirabe::package::version::VersionParser; +use shirabe::repository::composer_repository::ComposerRepository; +use shirabe::util::http_downloader::{ + HttpDownloader, HttpDownloaderMockExpectation, HttpDownloaderMockHandler, +}; +use shirabe_php_shim::PhpMixed; +use shirabe_semver::constraint::AnyConstraint; +use tempfile::TempDir; + +#[path = "packagist_fixture.rs"] +mod packagist_fixture; + +const PACKAGES: &[&str] = &["symfony/console", "laravel/framework"]; + +/// Constraints spanning the acceptance rates a repository sees, from one no version satisfies to +/// one that takes every version there is. Each case is labelled with how many packages survived, +/// since that rate is what decides whether deferring the copy pays off. +const CONSTRAINTS: &[(&str, &str)] = &[ + ("none", "^99.0"), + ("major7", "^7.0"), + ("major6-up", ">=6.0"), + ("major4-up", ">=4.0"), + ("major2-up", ">=2.0"), + ("any", "*"), +]; + +const REPOSITORY_URL: &str = "https://example.org/packages.json"; +const METADATA_URL: &str = "https://example.org/p2/%package%.json"; + +/// A root file with nothing but a metadata-url, which is what puts the repository on the lazy +/// provider path `load_async_packages` serves. +fn root_file() -> String { + let mut root: IndexMap = IndexMap::new(); + root.insert("packages".to_string(), PhpMixed::Array(IndexMap::new())); + root.insert( + "metadata-url".to_string(), + PhpMixed::String(METADATA_URL.to_string()), + ); + JsonFile::encode(&PhpMixed::Array(root)).expect("failed to encode the root file") +} + +/// A config whose cache is read-only, so the measurement stays off the filesystem. +fn config(home: &TempDir) -> Config { + let mut config = Config::new(true, None); + let mut settings: IndexMap = IndexMap::new(); + settings.insert( + "home".to_string(), + PhpMixed::String(home.path().display().to_string()), + ); + settings.insert("cache-read-only".to_string(), PhpMixed::Bool(true)); + let mut top: IndexMap = IndexMap::new(); + top.insert("config".to_string(), PhpMixed::Array(settings)); + config.merge(&top, Config::SOURCE_UNKNOWN); + config +} + +/// A repository answering exactly two requests, in the order `load_packages` makes them: the root +/// file, then the package's metadata. +fn repository(package: &str, root: &str, metadata: &str, config: &Config) -> ComposerRepository { + let expectations = vec![ + HttpDownloaderMockExpectation { + url: REPOSITORY_URL.to_string(), + options: None, + status: 200, + body: root.to_string(), + headers: vec![String::new()], + }, + HttpDownloaderMockExpectation { + url: METADATA_URL.replace("%package%", package), + options: None, + status: 200, + body: metadata.to_string(), + headers: vec![String::new()], + }, + ]; + + let io: std::rc::Rc> = + std::rc::Rc::new(std::cell::RefCell::new(NullIO::new())); + let http_downloader = std::rc::Rc::new(std::cell::RefCell::new(HttpDownloader::__new_mock( + io.clone(), + std::rc::Rc::new(std::cell::RefCell::new(Config::new(false, None))), + ))); + http_downloader.borrow_mut().__expects( + expectations, + true, + HttpDownloaderMockHandler::default(), + ); + + let mut repo_config: IndexMap = IndexMap::new(); + repo_config.insert( + "url".to_string(), + PhpMixed::String(REPOSITORY_URL.to_string()), + ); + + ComposerRepository::new(repo_config, io, config, http_downloader, None) + .expect("failed to build the repository") +} + +fn load( + repository: &mut ComposerRepository, + package: &str, + constraint: &AnyConstraint, + acceptable_stabilities: &IndexMap, +) -> usize { + let mut package_name_map: IndexMap> = IndexMap::new(); + package_name_map.insert(package.to_string(), Some(constraint.clone())); + repository + .load_packages( + package_name_map, + acceptable_stabilities.clone(), + IndexMap::new(), + IndexMap::new(), + ) + .expect("load_packages failed") + .packages + .len() +} + +/// How many versions the p2 file carries, for the `of` labels. +fn version_count(package: &str, metadata: &str) -> usize { + JsonFile::parse_json(Some(metadata), None) + .expect("invalid fixture JSON") + .as_array() + .and_then(|response| response.get("packages")) + .and_then(|v| v.as_array()) + .and_then(|packages| packages.get(package)) + .and_then(|v| v.as_list()) + .expect("the fixture has no version list for the package") + .len() +} + +fn bench_load_packages(c: &mut Criterion) { + let home = TempDir::new().expect("failed to create the config home"); + let config = config(&home); + let root = root_file(); + let version_parser = VersionParser::new(); + + let mut acceptable_stabilities: IndexMap = IndexMap::new(); + acceptable_stabilities.insert("stable".to_string(), STABILITY_STABLE); + + for package in PACKAGES { + let metadata = packagist_fixture::fetch(package); + let total = version_count(package, &metadata); + let mut group = c.benchmark_group(format!("load_packages/{package}")); + + for (label, constraint) in CONSTRAINTS { + let constraint: AnyConstraint = version_parser + .parse_constraints(constraint) + .expect("failed to parse the constraint"); + + let mut probe = repository(package, &root, &metadata, &config); + let loaded = load(&mut probe, package, &constraint, &acceptable_stabilities); + let parameter = format!("{label}/{loaded}of{total}"); + + group.bench_function(BenchmarkId::from_parameter(parameter), |b| { + b.iter_batched( + || repository(package, &root, &metadata, &config), + |mut repository| { + load( + &mut repository, + package, + &constraint, + &acceptable_stabilities, + ) + }, + BatchSize::PerIteration, + ); + }); + } + + group.finish(); + } +} + +criterion_group!(benches, bench_load_packages); +criterion_main!(benches); diff --git a/crates/shirabe/benches/metadata_minifier.rs b/crates/shirabe/benches/metadata_minifier.rs new file mode 100644 index 00000000..dff37740 --- /dev/null +++ b/crates/shirabe/benches/metadata_minifier.rs @@ -0,0 +1,98 @@ +//! Benchmarks for expanding minified package metadata. +//! +//! The workload mirrors `ComposerRepository::load_async_packages`: every expanded version is +//! inspected, but only the ones accepted by the constraint and stability filters are kept. The +//! `keep` parameter is that acceptance rate, which is what decides how much of the expansion has +//! to be copied out. + +use criterion::BatchSize; +use criterion::BenchmarkId; +use criterion::Criterion; +use criterion::criterion_group; +use criterion::criterion_main; +use indexmap::IndexMap; +use shirabe::json::json_file::JsonFile; +use shirabe_metadata_minifier::MetadataMinifier; +use shirabe_php_shim::PhpMixed; +use std::hint::black_box; + +#[path = "packagist_fixture.rs"] +mod packagist_fixture; + +/// Real packagist metadata, fetched on first run. `symfony/console` and `laravel/framework` are +/// long-lived packages whose p2 files carry hundreds of versions each. +const PACKAGES: &[&str] = &["monolog/monolog", "symfony/console", "laravel/framework"]; + +/// Percentages of the expanded versions the caller keeps. +const KEEP_PERCENTS: &[u32] = &[0, 10, 50, 100]; + +/// The minified version list for `package`, in the shape `load_async_packages` hands to the +/// minifier. +fn minified_versions(package: &str) -> Vec> { + let response = JsonFile::parse_json(Some(&packagist_fixture::fetch(package)), None) + .expect("invalid fixture JSON"); + let response = response.as_array().expect("the fixture is not an object"); + assert_eq!( + response.get("minified").and_then(|v| v.as_string()), + Some("composer/2.0"), + "{package} is not served in the minified format", + ); + + response + .get("packages") + .and_then(|v| v.as_array()) + .and_then(|packages| packages.get(package)) + .and_then(|v| v.as_list()) + .expect("the fixture has no version list for the package") + .iter() + .map(|version| { + version + .as_array() + .expect("a version is not an object") + .clone() + }) + .collect() +} + +/// Reads one field of every expanded version and keeps the newest `keep` of them, the way the +/// repository keeps only the versions its filters accept. +fn consume( + versions: Vec>, + keep: usize, +) -> Vec> { + let expanded = MetadataMinifier::expand(versions); + let first_kept = expanded.len() - keep; + let mut kept: Vec> = Vec::with_capacity(keep); + for index in 0..expanded.len() { + black_box(expanded.version(index).get("version_normalized")); + if index >= first_kept { + kept.push(expanded.materialize(index)); + } + } + kept +} + +fn bench_expand(c: &mut Criterion) { + for package in PACKAGES { + let versions = minified_versions(package); + let mut group = c.benchmark_group(format!("expand/{package}")); + + for keep_percent in KEEP_PERCENTS { + let keep = versions.len() * (*keep_percent as usize) / 100; + let parameter = format!("keep{keep_percent}%/{}of{}", keep, versions.len()); + + group.bench_function(BenchmarkId::from_parameter(parameter), |b| { + b.iter_batched( + || versions.clone(), + |versions| consume(versions, keep), + BatchSize::PerIteration, + ); + }); + } + + group.finish(); + } +} + +criterion_group!(benches, bench_expand); +criterion_main!(benches); diff --git a/crates/shirabe/benches/packagist_fixture.rs b/crates/shirabe/benches/packagist_fixture.rs new file mode 100644 index 00000000..86184879 --- /dev/null +++ b/crates/shirabe/benches/packagist_fixture.rs @@ -0,0 +1,26 @@ +//! Real packagist p2 metadata for the benchmarks, fetched on first run and cached under the +//! target directory so later runs work offline. + +use std::path::PathBuf; + +pub fn fetch(package: &str) -> String { + let dir = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("packagist-p2"); + std::fs::create_dir_all(&dir).expect("failed to create the fixture directory"); + let path = dir.join(format!("{}.json", package.replace('/', "-"))); + if let Ok(fixture) = std::fs::read_to_string(&path) { + return fixture; + } + + let url = format!("https://repo.packagist.org/p2/{package}.json"); + let fixture = reqwest::blocking::get(&url) + .and_then(|response| response.error_for_status()) + .and_then(|response| response.text()) + .unwrap_or_else(|e| { + panic!( + "failed to fetch {url}: {e}; download it to {} by hand to run this benchmark offline", + path.display(), + ) + }); + std::fs::write(&path, &fixture).expect("failed to cache the fixture"); + fixture +} 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 { + 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, - ) -> anyhow::Result> { + pub fn get_branch_alias(&self, config: &impl VersionFields) -> anyhow::Result> { 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, } +/// 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>), + 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 { + 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), + 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 { self.repo_config.clone() @@ -1810,7 +1891,7 @@ impl ComposerRepository { None => continue, }; - let mut versions: Vec> = match versions_mixed { + let versions: Vec> = 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> = 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, + version_data: &impl VersionFields, acceptable_stabilities: Option<&IndexMap>, stability_flags: Option<&IndexMap>, ) -> anyhow::Result { @@ -2024,7 +2098,7 @@ impl ComposerRepository { fn is_version_acceptable_static( constraint: Option<&AnyConstraint>, name: &str, - version_data: &IndexMap, + version_data: &impl VersionFields, acceptable_stabilities: Option<&IndexMap>, stability_flags: Option<&IndexMap>, ) -> anyhow::Result { @@ -2042,7 +2116,7 @@ impl ComposerRepository { loader: &ArrayLoader, constraint: Option<&AnyConstraint>, name: &str, - version_data: &IndexMap, + version_data: &impl VersionFields, acceptable_stabilities: Option<&IndexMap>, stability_flags: Option<&IndexMap>, ) -> anyhow::Result { diff --git a/crates/shirabe/tests/util/metadata_minifier_test.rs b/crates/shirabe/tests/util/metadata_minifier_test.rs index f0f3e9cb..143af23f 100644 --- a/crates/shirabe/tests/util/metadata_minifier_test.rs +++ b/crates/shirabe/tests/util/metadata_minifier_test.rs @@ -66,5 +66,5 @@ fn test_minify_expand() { dumper.dump(package3.into()), ]; - assert_eq!(source, MetadataMinifier::expand(minified)); + assert_eq!(source, MetadataMinifier::expand(minified).into_vec()); } -- cgit v1.3.1-4-g156e