From cd7c3fce2472656b2a1247429751e43350976677 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Wed, 19 Aug 2026 23:45:38 +0900 Subject: perf(package): hand out link maps behind Rc PackageInterface::getRequires() and friends return the array of Link objects; in PHP that is a copy-on-write array of object references, so a caller pays nothing to look at it. The port returned IndexMap by value, so every call deep-cloned the whole map, keys and constraints included. Pool building calls these accessors once per package per candidate, which put IndexMap::clone at 13.5% of `require laravel/laravel`. Store the maps as Rc> and return a handle. Callers that mutate the map clone it explicitly at the point of mutation, matching where PHP would separate the array. Co-Authored-By: Claude Opus 5 (1M context) --- crates/shirabe/src/package/alias_package.rs | 45 +++++++++--------- crates/shirabe/src/package/complete_package.rs | 20 ++++---- crates/shirabe/src/package/handle.rs | 42 ++++++++++++----- crates/shirabe/src/package/package.rs | 54 +++++++++++----------- crates/shirabe/src/package/package_interface.rs | 17 ++++--- crates/shirabe/src/package/root_alias_package.rs | 45 ++++++++++-------- crates/shirabe/src/package/root_package.rs | 10 ++-- .../src/package/version/version_selector.rs | 2 +- 8 files changed, 132 insertions(+), 103 deletions(-) (limited to 'crates/shirabe/src/package') diff --git a/crates/shirabe/src/package/alias_package.rs b/crates/shirabe/src/package/alias_package.rs index 9df804f1..4eeb408a 100644 --- a/crates/shirabe/src/package/alias_package.rs +++ b/crates/shirabe/src/package/alias_package.rs @@ -39,15 +39,15 @@ pub struct AliasPackage { /// @var BasePackage pub(crate) alias_of: PackageHandle, /// @var Link[] - pub(crate) requires: IndexMap, + pub(crate) requires: std::rc::Rc>, /// @var Link[] - pub(crate) dev_requires: IndexMap, + pub(crate) dev_requires: std::rc::Rc>, /// @var array - pub(crate) conflicts: IndexMap, + pub(crate) conflicts: std::rc::Rc>, /// @var array - pub(crate) provides: IndexMap, + pub(crate) provides: std::rc::Rc>, /// @var array - pub(crate) replaces: IndexMap, + pub(crate) replaces: std::rc::Rc>, } impl AliasPackage { @@ -74,15 +74,15 @@ impl AliasPackage { stability, has_self_version_requires: false, alias_of, - requires: IndexMap::new(), - dev_requires: IndexMap::new(), - conflicts: IndexMap::new(), - provides: IndexMap::new(), - replaces: IndexMap::new(), + requires: std::rc::Rc::new(IndexMap::new()), + dev_requires: std::rc::Rc::new(IndexMap::new()), + conflicts: std::rc::Rc::new(IndexMap::new()), + provides: std::rc::Rc::new(IndexMap::new()), + replaces: std::rc::Rc::new(IndexMap::new()), }; for r#type in Link::types() { - let links: IndexMap = match r#type { + let links: std::rc::Rc> = match r#type { Link::TYPE_REQUIRE => this.alias_of.get_requires(), Link::TYPE_DEV_REQUIRE => this.alias_of.get_dev_requires(), Link::TYPE_PROVIDE => this.alias_of.get_provides(), @@ -90,7 +90,8 @@ impl AliasPackage { Link::TYPE_REPLACE => this.alias_of.get_replaces(), _ => unreachable!(), }; - let replaced = this.replace_self_version_dependencies(links, r#type); + let replaced = + std::rc::Rc::new(this.replace_self_version_dependencies((*links).clone(), r#type)); match r#type { Link::TYPE_REQUIRE => this.requires = replaced, Link::TYPE_DEV_REQUIRE => this.dev_requires = replaced, @@ -252,27 +253,27 @@ impl PackageInterface for AliasPackage { &self.pretty_version } - fn get_requires(&self) -> IndexMap { - self.requires.clone() + fn get_requires(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.requires) } /// @inheritDoc - fn get_conflicts(&self) -> IndexMap { - self.conflicts.clone() + fn get_conflicts(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.conflicts) } /// @inheritDoc - fn get_provides(&self) -> IndexMap { - self.provides.clone() + fn get_provides(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.provides) } /// @inheritDoc - fn get_replaces(&self) -> IndexMap { - self.replaces.clone() + fn get_replaces(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.replaces) } - fn get_dev_requires(&self) -> IndexMap { - self.dev_requires.clone() + fn get_dev_requires(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.dev_requires) } fn get_type(&self) -> String { diff --git a/crates/shirabe/src/package/complete_package.rs b/crates/shirabe/src/package/complete_package.rs index 00b4a67c..b21edb44 100644 --- a/crates/shirabe/src/package/complete_package.rs +++ b/crates/shirabe/src/package/complete_package.rs @@ -279,24 +279,24 @@ impl PackageInterface for CompletePackage { self.inner.get_stability() } - fn get_requires(&self) -> IndexMap { - self.inner.get_requires().clone() + fn get_requires(&self) -> std::rc::Rc> { + PackageInterface::get_requires(&self.inner) } - fn get_conflicts(&self) -> IndexMap { - self.inner.get_conflicts().clone() + fn get_conflicts(&self) -> std::rc::Rc> { + PackageInterface::get_conflicts(&self.inner) } - fn get_provides(&self) -> IndexMap { - self.inner.get_provides().clone() + fn get_provides(&self) -> std::rc::Rc> { + PackageInterface::get_provides(&self.inner) } - fn get_replaces(&self) -> IndexMap { - self.inner.get_replaces().clone() + fn get_replaces(&self) -> std::rc::Rc> { + PackageInterface::get_replaces(&self.inner) } - fn get_dev_requires(&self) -> IndexMap { - self.inner.get_dev_requires().clone() + fn get_dev_requires(&self) -> std::rc::Rc> { + PackageInterface::get_dev_requires(&self.inner) } fn get_suggests(&self) -> IndexMap { diff --git a/crates/shirabe/src/package/handle.rs b/crates/shirabe/src/package/handle.rs index 45d46f49..fd7272de 100644 --- a/crates/shirabe/src/package/handle.rs +++ b/crates/shirabe/src/package/handle.rs @@ -322,19 +322,29 @@ macro_rules! delegate_package_interface_to_inner { fn get_stability(&self) -> &str { self.$field.get_stability() } - fn get_requires(&self) -> indexmap::IndexMap { + fn get_requires( + &self, + ) -> std::rc::Rc> { self.$field.get_requires() } - fn get_conflicts(&self) -> indexmap::IndexMap { + fn get_conflicts( + &self, + ) -> std::rc::Rc> { self.$field.get_conflicts() } - fn get_provides(&self) -> indexmap::IndexMap { + fn get_provides( + &self, + ) -> std::rc::Rc> { self.$field.get_provides() } - fn get_replaces(&self) -> indexmap::IndexMap { + fn get_replaces( + &self, + ) -> std::rc::Rc> { self.$field.get_replaces() } - fn get_dev_requires(&self) -> indexmap::IndexMap { + fn get_dev_requires( + &self, + ) -> std::rc::Rc> { self.$field.get_dev_requires() } fn get_suggests(&self) -> indexmap::IndexMap { @@ -585,23 +595,33 @@ macro_rules! impl_package_interface_handle { .to_string() } - pub fn get_requires(&self) -> indexmap::IndexMap { + pub fn get_requires( + &self, + ) -> std::rc::Rc> { self.0.borrow().as_package_interface().get_requires() } - pub fn get_conflicts(&self) -> indexmap::IndexMap { + pub fn get_conflicts( + &self, + ) -> std::rc::Rc> { self.0.borrow().as_package_interface().get_conflicts() } - pub fn get_provides(&self) -> indexmap::IndexMap { + pub fn get_provides( + &self, + ) -> std::rc::Rc> { self.0.borrow().as_package_interface().get_provides() } - pub fn get_replaces(&self) -> indexmap::IndexMap { + pub fn get_replaces( + &self, + ) -> std::rc::Rc> { self.0.borrow().as_package_interface().get_replaces() } - pub fn get_dev_requires(&self) -> indexmap::IndexMap { + pub fn get_dev_requires( + &self, + ) -> std::rc::Rc> { self.0.borrow().as_package_interface().get_dev_requires() } @@ -612,7 +632,7 @@ macro_rules! impl_package_interface_handle { pub fn get_links_for_type( &self, link_type: &str, - ) -> indexmap::IndexMap { + ) -> std::rc::Rc> { self.0 .borrow() .as_package_interface() diff --git a/crates/shirabe/src/package/package.rs b/crates/shirabe/src/package/package.rs index 388280fd..44770bc1 100644 --- a/crates/shirabe/src/package/package.rs +++ b/crates/shirabe/src/package/package.rs @@ -54,11 +54,11 @@ pub struct Package { stability: String, notification_url: Option, - requires: IndexMap, - conflicts: IndexMap, - provides: IndexMap, - replaces: IndexMap, - dev_requires: IndexMap, + requires: std::rc::Rc>, + conflicts: std::rc::Rc>, + provides: std::rc::Rc>, + replaces: std::rc::Rc>, + dev_requires: std::rc::Rc>, suggests: IndexMap, autoload: IndexMap, dev_autoload: IndexMap, @@ -98,11 +98,11 @@ impl Package { dev, stability, notification_url: None, - requires: IndexMap::new(), - conflicts: IndexMap::new(), - provides: IndexMap::new(), - replaces: IndexMap::new(), - dev_requires: IndexMap::new(), + requires: std::rc::Rc::new(IndexMap::new()), + conflicts: std::rc::Rc::new(IndexMap::new()), + provides: std::rc::Rc::new(IndexMap::new()), + replaces: std::rc::Rc::new(IndexMap::new()), + dev_requires: std::rc::Rc::new(IndexMap::new()), suggests: IndexMap::new(), autoload: IndexMap::new(), dev_autoload: IndexMap::new(), @@ -299,7 +299,7 @@ impl Package { requires = self.convert_links_to_map(requires, "setRequires"); } - self.requires = requires; + self.requires = std::rc::Rc::new(requires); } pub fn get_requires(&self) -> &IndexMap { @@ -311,7 +311,7 @@ impl Package { conflicts = self.convert_links_to_map(conflicts, "setConflicts"); } - self.conflicts = conflicts; + self.conflicts = std::rc::Rc::new(conflicts); } pub fn get_conflicts(&self) -> &IndexMap { @@ -323,7 +323,7 @@ impl Package { provides = self.convert_links_to_map(provides, "setProvides"); } - self.provides = provides; + self.provides = std::rc::Rc::new(provides); } pub fn get_provides(&self) -> &IndexMap { @@ -335,7 +335,7 @@ impl Package { replaces = self.convert_links_to_map(replaces, "setReplaces"); } - self.replaces = replaces; + self.replaces = std::rc::Rc::new(replaces); } pub fn get_replaces(&self) -> &IndexMap { @@ -347,7 +347,7 @@ impl Package { dev_requires = self.convert_links_to_map(dev_requires, "setDevRequires"); } - self.dev_requires = dev_requires; + self.dev_requires = std::rc::Rc::new(dev_requires); } pub fn get_dev_requires(&self) -> &IndexMap { @@ -604,12 +604,12 @@ impl PackageInterface for Package { names.insert(self.get_name().to_string()); if provides { - for (_, link) in self.get_provides() { + for (_, link) in self.get_provides().iter() { names.insert(link.get_target().to_string()); } } - for (_, link) in self.get_replaces() { + for (_, link) in self.get_replaces().iter() { names.insert(link.get_target().to_string()); } @@ -693,20 +693,20 @@ impl PackageInterface for Package { fn get_stability(&self) -> &str { &self.stability } - fn get_requires(&self) -> IndexMap { - self.requires.clone() + fn get_requires(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.requires) } - fn get_conflicts(&self) -> IndexMap { - self.conflicts.clone() + fn get_conflicts(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.conflicts) } - fn get_provides(&self) -> IndexMap { - self.provides.clone() + fn get_provides(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.provides) } - fn get_replaces(&self) -> IndexMap { - self.replaces.clone() + fn get_replaces(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.replaces) } - fn get_dev_requires(&self) -> IndexMap { - self.dev_requires.clone() + fn get_dev_requires(&self) -> std::rc::Rc> { + std::rc::Rc::clone(&self.dev_requires) } fn get_suggests(&self) -> IndexMap { self.suggests.clone() diff --git a/crates/shirabe/src/package/package_interface.rs b/crates/shirabe/src/package/package_interface.rs index ffbec286..d138fd83 100644 --- a/crates/shirabe/src/package/package_interface.rs +++ b/crates/shirabe/src/package/package_interface.rs @@ -159,31 +159,31 @@ pub trait PackageInterface: std::fmt::Display + std::fmt::Debug { /// this package can be installed /// /// @return array A map of package links defining required packages, indexed by the require package's name - fn get_requires(&self) -> IndexMap; + fn get_requires(&self) -> std::rc::Rc>; /// Returns a set of links to packages which must not be installed at the /// same time as this package /// /// @return array A map of package links defining conflicting packages - fn get_conflicts(&self) -> IndexMap; + fn get_conflicts(&self) -> std::rc::Rc>; /// Returns a set of links to virtual packages that are provided through /// this package /// /// @return array A map of package links defining provided packages - fn get_provides(&self) -> IndexMap; + fn get_provides(&self) -> std::rc::Rc>; /// Returns a set of links to packages which can alternatively be /// satisfied by installing this package /// /// @return array A map of package links defining replaced packages - fn get_replaces(&self) -> IndexMap; + fn get_replaces(&self) -> std::rc::Rc>; /// Returns a set of links to packages which are required to develop /// this package. These are installed if in dev mode. /// /// @return array A map of package links defining packages required for development, indexed by the require package's name - fn get_dev_requires(&self) -> IndexMap; + fn get_dev_requires(&self) -> std::rc::Rc>; /// Returns a set of package names and reasons why they are useful in /// combination with this package. @@ -192,14 +192,17 @@ pub trait PackageInterface: std::fmt::Display + std::fmt::Debug { fn get_suggests(&self) -> IndexMap; /// PHP helper that switches on the link kind (require/require-dev/conflict/etc.). - fn get_links_for_type(&self, link_type: &str) -> IndexMap { + fn get_links_for_type( + &self, + link_type: &str, + ) -> std::rc::Rc> { match link_type { "require" => self.get_requires(), "require-dev" => self.get_dev_requires(), "conflict" => self.get_conflicts(), "provide" => self.get_provides(), "replace" => self.get_replaces(), - _ => IndexMap::new(), + _ => std::rc::Rc::new(IndexMap::new()), } } diff --git a/crates/shirabe/src/package/root_alias_package.rs b/crates/shirabe/src/package/root_alias_package.rs index 4a4115d7..d5ea720e 100644 --- a/crates/shirabe/src/package/root_alias_package.rs +++ b/crates/shirabe/src/package/root_alias_package.rs @@ -78,44 +78,49 @@ impl RootPackageInterface for RootAliasPackage { } fn set_requires(&mut self, requires: IndexMap) { - self.inner.inner.requires = self - .inner - .inner - .replace_self_version_dependencies(requires.clone(), Link::TYPE_REQUIRE); + self.inner.inner.requires = std::rc::Rc::new( + self.inner + .inner + .replace_self_version_dependencies(requires.clone(), Link::TYPE_REQUIRE), + ); self.alias_of.set_requires(requires); } fn set_dev_requires(&mut self, dev_requires: IndexMap) { - self.inner.inner.dev_requires = self - .inner - .inner - .replace_self_version_dependencies(dev_requires.clone(), Link::TYPE_DEV_REQUIRE); + self.inner.inner.dev_requires = std::rc::Rc::new( + self.inner + .inner + .replace_self_version_dependencies(dev_requires.clone(), Link::TYPE_DEV_REQUIRE), + ); self.alias_of.set_dev_requires(dev_requires); } fn set_conflicts(&mut self, conflicts: IndexMap) { - self.inner.inner.conflicts = self - .inner - .inner - .replace_self_version_dependencies(conflicts.clone(), Link::TYPE_CONFLICT); + self.inner.inner.conflicts = std::rc::Rc::new( + self.inner + .inner + .replace_self_version_dependencies(conflicts.clone(), Link::TYPE_CONFLICT), + ); self.alias_of.set_conflicts(conflicts); } fn set_provides(&mut self, provides: IndexMap) { - self.inner.inner.provides = self - .inner - .inner - .replace_self_version_dependencies(provides.clone(), Link::TYPE_PROVIDE); + self.inner.inner.provides = std::rc::Rc::new( + self.inner + .inner + .replace_self_version_dependencies(provides.clone(), Link::TYPE_PROVIDE), + ); self.alias_of.set_provides(provides); } fn set_replaces(&mut self, replaces: IndexMap) { - self.inner.inner.replaces = self - .inner - .inner - .replace_self_version_dependencies(replaces.clone(), Link::TYPE_REPLACE); + self.inner.inner.replaces = std::rc::Rc::new( + self.inner + .inner + .replace_self_version_dependencies(replaces.clone(), Link::TYPE_REPLACE), + ); self.alias_of.set_replaces(replaces); } diff --git a/crates/shirabe/src/package/root_package.rs b/crates/shirabe/src/package/root_package.rs index 0807d15b..e1c4b35e 100644 --- a/crates/shirabe/src/package/root_package.rs +++ b/crates/shirabe/src/package/root_package.rs @@ -338,19 +338,19 @@ impl PackageInterface for RootPackage { fn get_stability(&self) -> &str { self.inner.get_stability() } - fn get_requires(&self) -> IndexMap { + fn get_requires(&self) -> std::rc::Rc> { self.inner.get_requires() } - fn get_conflicts(&self) -> IndexMap { + fn get_conflicts(&self) -> std::rc::Rc> { self.inner.get_conflicts() } - fn get_provides(&self) -> IndexMap { + fn get_provides(&self) -> std::rc::Rc> { self.inner.get_provides() } - fn get_replaces(&self) -> IndexMap { + fn get_replaces(&self) -> std::rc::Rc> { self.inner.get_replaces() } - fn get_dev_requires(&self) -> IndexMap { + fn get_dev_requires(&self) -> std::rc::Rc> { self.inner.get_dev_requires() } fn get_suggests(&self) -> IndexMap { diff --git a/crates/shirabe/src/package/version/version_selector.rs b/crates/shirabe/src/package/version/version_selector.rs index 61435870..230e886f 100644 --- a/crates/shirabe/src/package/version/version_selector.rs +++ b/crates/shirabe/src/package/version/version_selector.rs @@ -142,7 +142,7 @@ impl VersionSelector { for pkg in candidates.iter() { let reqs = pkg.get_requires(); let mut skip = false; - 'reqs: for (name, link) in &reqs { + 'reqs: for (name, link) in reqs.iter() { if !PlatformRepository::is_platform_package(name) || platform_requirement_filter.is_ignored(name) { -- cgit v1.3.1-4-g156e