From cf590fe5df9f8c60300d9385f33493ab9e98f424 Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 10 Nov 2023 20:19:19 -0800 Subject: [PATCH] [meta] switch to AHashMap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch to ahash, which promises to be faster at lookups. Overall, iteration through the hash map is a bit slower but `make_package_graph` and other queries are faster. ``` make_package_graph time: [6.9865 ms 7.0088 ms 7.0313 ms] change: [-4.0527% -3.5680% -3.1149%] (p = 0.00 < 0.05) Performance has improved. depends_on time: [110.99 µs 111.12 µs 111.27 µs] change: [-12.629% -12.282% -11.952%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) low mild 3 (3.00%) high mild depends_on_cache time: [92.296 µs 92.522 µs 92.777 µs] change: [-18.680% -18.438% -18.227%] (p = 0.00 < 0.05) Performance has improved. Found 6 outliers among 100 measurements (6.00%) 2 (2.00%) low mild 3 (3.00%) high mild 1 (1.00%) high severe into_ids time: [164.61 µs 164.74 µs 164.87 µs] change: [-9.3113% -8.9215% -8.5783%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) low mild 1 (1.00%) high mild 2 (2.00%) high severe resolve_package_name time: [1.4308 µs 1.4327 µs 1.4351 µs] change: [+12.664% +12.952% +13.227%] (p = 0.00 < 0.05) Performance has regressed. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild make_package_name_hashmap time: [50.083 µs 50.112 µs 50.144 µs] change: [-0.0379% +0.0879% +0.2012%] (p = 0.16 > 0.05) No change in performance detected. Found 5 outliers among 100 measurements (5.00%) 3 (3.00%) high mild 2 (2.00%) high severe make_cycles time: [55.688 µs 55.770 µs 55.871 µs] change: [+6.9990% +7.4064% +7.7971%] (p = 0.00 < 0.05) Performance has regressed. ``` --- Cargo.lock | 33 +++++++++++- Cargo.toml | 1 + cargo-guppy/Cargo.toml | 1 + cargo-guppy/src/lib.rs | 15 ++---- clippy.toml | 5 ++ fixtures/Cargo.toml | 1 + fixtures/src/details.rs | 21 ++++---- fixtures/src/json.rs | 46 ++++++++--------- guppy-summaries/Cargo.toml | 1 + guppy-summaries/src/diff.rs | 6 ++- guppy/Cargo.toml | 1 + guppy/src/graph/build.rs | 50 ++++++------------- guppy/src/graph/feature/build.rs | 15 +++--- guppy/src/graph/feature/graph_impl.rs | 5 +- guppy/src/graph/graph_impl.rs | 5 +- guppy/src/graph/summaries/package_set.rs | 19 +++---- guppy/src/petgraph_support/scc.rs | 7 +-- .../benchmarks/benches/package_graph.rs | 2 + tools/determinator/Cargo.toml | 1 + tools/determinator/src/determinator.rs | 13 +++-- tools/hakari/Cargo.toml | 1 + tools/hakari/src/hakari.rs | 11 ++-- tools/hakari/src/toml_out.rs | 11 ++-- workspace-hack/Cargo.toml | 4 ++ 24 files changed, 150 insertions(+), 125 deletions(-) create mode 100644 clippy.toml diff --git a/Cargo.lock b/Cargo.lock index 545ffeb512f..d8e8344b731 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19,14 +19,15 @@ checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" [[package]] name = "ahash" -version = "0.8.3" +version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" +checksum = "91429305e9f0a25f6205c5b8e0d2db09e0708a7a6df0f42212bb56c32c8ac97a" dependencies = [ "cfg-if", "getrandom", "once_cell", "version_check", + "zerocopy", ] [[package]] @@ -422,6 +423,7 @@ dependencies = [ name = "cargo-guppy" version = "0.1.0" dependencies = [ + "ahash", "camino", "clap 3.2.25", "color-eyre", @@ -907,6 +909,7 @@ dependencies = [ name = "determinator" version = "0.12.0" dependencies = [ + "ahash", "camino", "cfg-if", "fixtures", @@ -1172,6 +1175,7 @@ dependencies = [ name = "fixtures" version = "0.1.0" dependencies = [ + "ahash", "camino", "guppy", "guppy-workspace-hack", @@ -1963,6 +1967,7 @@ dependencies = [ name = "guppy" version = "0.17.1" dependencies = [ + "ahash", "camino", "cargo_metadata", "cfg-if", @@ -2016,6 +2021,7 @@ dependencies = [ name = "guppy-summaries" version = "0.7.1" dependencies = [ + "ahash", "camino", "cfg-if", "diffus", @@ -2034,11 +2040,13 @@ version = "0.1.0" dependencies = [ "clap 4.3.19", "clap_builder", + "getrandom", "indexmap 1.9.3", "libc", "log", "memchr", "num-traits", + "once_cell", "owo-colors", "petgraph", "proc-macro2", @@ -2060,6 +2068,7 @@ dependencies = [ name = "hakari" version = "0.17.1" dependencies = [ + "ahash", "atomicwrites", "bimap", "camino", @@ -4362,6 +4371,26 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" +[[package]] +name = "zerocopy" +version = "0.7.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8cd369a67c0edfef15010f980c3cbe45d7f651deac2cd67ce097cd801de16557" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.7.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2f140bda219a26ccc0cdb03dba58af72590c53b22642577d88a927bc5c87d6b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.32", +] + [[package]] name = "zeroize" version = "1.6.0" diff --git a/Cargo.toml b/Cargo.toml index ea59891b2c1..a3e0f1bc60c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ members = [ ] [workspace.dependencies] +ahash = "0.8.6" guppy-workspace-hack = "0.1.0" [patch.crates-io.guppy-workspace-hack] diff --git a/cargo-guppy/Cargo.toml b/cargo-guppy/Cargo.toml index 1b08125b20a..923cbbaf5cb 100644 --- a/cargo-guppy/Cargo.toml +++ b/cargo-guppy/Cargo.toml @@ -6,6 +6,7 @@ license = "MIT OR Apache-2.0" edition = "2018" [dependencies] +ahash.workspace = true camino = "1.1.6" # disable tracing integration since we don't use it color-eyre = { version = "0.6.2", default-features = false } diff --git a/cargo-guppy/src/lib.rs b/cargo-guppy/src/lib.rs index ea27f4794d9..3b8b35e2ebe 100644 --- a/cargo-guppy/src/lib.rs +++ b/cargo-guppy/src/lib.rs @@ -42,6 +42,7 @@ mod mv; pub use crate::{core::*, mv::*}; +use ahash::AHashMap; use camino::Utf8PathBuf; use clap::{ArgEnum, Parser}; use color_eyre::eyre::{bail, Result, WrapErr}; @@ -58,15 +59,7 @@ use guppy::{ use guppy_cmdlib::{ string_to_platform_spec, CargoMetadataOptions, CargoResolverOpts, PackagesAndFeatures, }; -use std::{ - borrow::Cow, - cmp, - collections::{HashMap, HashSet}, - fmt, fs, - io::Write, - iter, - path::PathBuf, -}; +use std::{borrow::Cow, cmp, collections::HashSet, fmt, fs, io::Write, iter, path::PathBuf}; pub fn cmd_diff(json: bool, old: &str, new: &str) -> Result<()> { let old_json = fs::read_to_string(old)?; @@ -140,7 +133,7 @@ pub fn cmd_dups(opts: &DupsOptions) -> Result<()> { let resolver = opts.filter_opts.make_resolver(&pkg_graph)?; let selection = pkg_graph.query_workspace(); - let mut dupe_map: HashMap<_, Vec<_>> = HashMap::new(); + let mut dupe_map: AHashMap<_, Vec<_>> = AHashMap::new(); for package in selection .resolve_with_fn(resolver) .packages(DependencyDirection::Forward) @@ -387,7 +380,7 @@ pub fn cmd_subtree_size(options: &SubtreeSizeOptions) -> Result<()> { pkg_graph.query_workspace() }; - let mut unique_deps: HashMap<&PackageId, HashSet<&PackageId>> = HashMap::new(); + let mut unique_deps: AHashMap<&PackageId, HashSet<&PackageId>> = AHashMap::new(); for package_id in selection .resolve_with_fn(&resolver) .package_ids(DependencyDirection::Forward) diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000000..5e533b4e3ba --- /dev/null +++ b/clippy.toml @@ -0,0 +1,5 @@ +disallowed-methods = [ + # use ahash everywhere instead + "std::collections::hash::map::HashMap::new", + "std::collections::hash::map::HashMap::with_capacity", +] diff --git a/fixtures/Cargo.toml b/fixtures/Cargo.toml index 92dc8e1cabd..57ce4be2fd9 100644 --- a/fixtures/Cargo.toml +++ b/fixtures/Cargo.toml @@ -6,6 +6,7 @@ publish = false edition = "2018" [dependencies] +ahash.workspace = true camino = "1.1.6" guppy = { path = "../guppy" } once_cell = "1.18.0" diff --git a/fixtures/src/details.rs b/fixtures/src/details.rs index 729e480ad65..2a3a7030bfd 100644 --- a/fixtures/src/details.rs +++ b/fixtures/src/details.rs @@ -8,6 +8,7 @@ use crate::{ }, package_id, }; +use ahash::AHashMap; use camino::Utf8PathBuf; use guppy::{ errors::FeatureGraphWarning, @@ -19,24 +20,24 @@ use guppy::{ DependencyKind, PackageId, Version, }; use pretty_assertions::assert_eq; -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; /// This captures metadata fields that are relevant for tests. They are meant to be written out /// lazily as tests are filled out -- feel free to add more details as necessary! pub struct FixtureDetails { workspace_members: Option>, - package_details: HashMap, - link_details: HashMap<(PackageId, PackageId), LinkDetails>, + package_details: AHashMap, + link_details: AHashMap<(PackageId, PackageId), LinkDetails>, feature_graph_warnings: Vec, cycles: Vec>, } impl FixtureDetails { - pub fn new(package_details: HashMap) -> Self { + pub fn new(package_details: AHashMap) -> Self { Self { workspace_members: None, package_details, - link_details: HashMap::new(), + link_details: AHashMap::new(), feature_graph_warnings: vec![], cycles: vec![], } @@ -57,7 +58,7 @@ impl FixtureDetails { pub fn with_link_details( mut self, - link_details: HashMap<(PackageId, PackageId), LinkDetails>, + link_details: AHashMap<(PackageId, PackageId), LinkDetails>, ) -> Self { self.link_details = link_details; self @@ -456,7 +457,7 @@ impl PackageDetails { self } - pub fn insert_into(self, map: &mut HashMap) { + pub fn insert_into(self, map: &mut AHashMap) { map.insert(self.id.clone(), self); } @@ -543,7 +544,7 @@ impl LinkDetails { self } - pub fn insert_into(self, map: &mut HashMap<(PackageId, PackageId), Self>) { + pub fn insert_into(self, map: &mut AHashMap<(PackageId, PackageId), Self>) { map.insert((self.from.clone(), self.to.clone()), self); } @@ -601,7 +602,7 @@ pub struct PlatformResults { // Each pair stands for (required on, enabled on). status: (EnabledTernary, EnabledTernary), default_features: (EnabledTernary, EnabledTernary), - feature_statuses: HashMap, + feature_statuses: AHashMap, } impl PlatformResults { @@ -612,7 +613,7 @@ impl PlatformResults { Self { status, default_features, - feature_statuses: HashMap::new(), + feature_statuses: AHashMap::new(), } } diff --git a/fixtures/src/json.rs b/fixtures/src/json.rs index fed6f021154..59c32a9600a 100644 --- a/fixtures/src/json.rs +++ b/fixtures/src/json.rs @@ -5,6 +5,7 @@ use crate::{ details::{FixtureDetails, LinkDetails, PackageDetails, PlatformResults}, package_id, }; +use ahash::AHashMap; use camino::{Utf8Component, Utf8Path, Utf8PathBuf}; use guppy::{ errors::{FeatureBuildStage, FeatureGraphWarning}, @@ -13,10 +14,7 @@ use guppy::{ CargoMetadata, DependencyKind, }; use once_cell::sync::{Lazy, OnceCell}; -use std::{ - collections::{BTreeMap, HashMap}, - fs, -}; +use std::{collections::BTreeMap, fs}; // Metadata along with interesting crate names. pub static METADATA1_PATH: &str = "../small/metadata1.json"; @@ -409,7 +407,7 @@ impl FixtureDetails { // Specific fixtures follow. pub(crate) fn metadata1() -> Self { - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA1_TESTCRATE, @@ -501,7 +499,7 @@ impl FixtureDetails { } pub(crate) fn metadata2() -> Self { - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA2_TESTCRATE, @@ -583,13 +581,13 @@ impl FixtureDetails { } pub(crate) fn metadata_builddep() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } pub(crate) fn metadata_dups() -> Self { - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA_DUPS_TESTCRATE, @@ -612,7 +610,7 @@ impl FixtureDetails { } pub(crate) fn metadata_cycle1() -> Self { - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA_CYCLE1_BASE, @@ -652,7 +650,7 @@ impl FixtureDetails { // | // v // lower-a <-> lower-b - let mut details = HashMap::new(); + let mut details = AHashMap::new(); // upper-a PackageDetails::new( @@ -760,7 +758,7 @@ impl FixtureDetails { } pub(crate) fn metadata_cycle_features() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) .with_workspace_members(vec![ @@ -821,7 +819,7 @@ impl FixtureDetails { // [target.'cfg(all(unix, target_feature = "sse"))'.build-dependencies] // dep-a = { path = "../dep-a", optional = true, default-features = false, features = ["bar"] } // ``` - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA_TARGETS1_TESTCRATE, @@ -851,7 +849,7 @@ impl FixtureDetails { let x86_64_windows = Platform::new("x86_64-pc-windows-msvc", TargetFeatures::Unknown).unwrap(); - let mut link_details = HashMap::new(); + let mut link_details = AHashMap::new(); use EnabledTernary::*; @@ -1074,7 +1072,7 @@ impl FixtureDetails { // path = "src/lib.rs" // crate-type = ["rlib", "dylib"] - let mut details = HashMap::new(); + let mut details = AHashMap::new(); static BIN_CDYLIB_TYPES: Lazy> = Lazy::new(|| vec!["bin".into(), "cdylib".into()]); @@ -1128,7 +1126,7 @@ impl FixtureDetails { } pub(crate) fn metadata_proc_macro1() -> Self { - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA_PROC_MACRO1_MACRO, @@ -1150,17 +1148,17 @@ impl FixtureDetails { } pub(crate) fn metadata_alternate_registries() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } pub(crate) fn metadata_weak_namespaced_features() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } pub(crate) fn metadata_libra() -> Self { - let mut details = HashMap::new(); + let mut details = AHashMap::new(); PackageDetails::new( METADATA_LIBRA_E2E_TESTS, @@ -1298,7 +1296,7 @@ impl FixtureDetails { } pub(crate) fn metadata_libra_f0091a4() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details).with_cycles(vec![vec![ METADATA_LIBRA_FUNCTIONAL_HYPHEN_TESTS, @@ -1310,7 +1308,7 @@ impl FixtureDetails { } pub(crate) fn metadata_libra_9ffd93b() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details).with_cycles(vec![ vec![METADATA_LIBRA_EXECUTOR_UTILS, METADATA_LIBRA_EXECUTOR], @@ -1329,25 +1327,25 @@ impl FixtureDetails { } pub(crate) fn metadata_guppy_78cb7e8() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } pub(crate) fn metadata_guppy_869476c() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } pub(crate) fn metadata_guppy_c9b4f76() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } pub(crate) fn metadata_guppy_44b62fa() -> Self { - let details = HashMap::new(); + let details = AHashMap::new(); Self::new(details) } diff --git a/guppy-summaries/Cargo.toml b/guppy-summaries/Cargo.toml index 183312d9849..07e9019aee8 100644 --- a/guppy-summaries/Cargo.toml +++ b/guppy-summaries/Cargo.toml @@ -25,6 +25,7 @@ rust-version = "1.70" all-features = true [dependencies] +ahash = "0.8.6" camino = { version = "1.1.6", features = ["serde1"] } cfg-if = "1.0.0" diffus = "0.10.0" diff --git a/guppy-summaries/src/diff.rs b/guppy-summaries/src/diff.rs index b7b1a3f9eaf..b1ceaa11561 100644 --- a/guppy-summaries/src/diff.rs +++ b/guppy-summaries/src/diff.rs @@ -10,11 +10,12 @@ pub use crate::report::SummaryReport; use crate::{PackageInfo, PackageMap, PackageStatus, Summary, SummaryId, SummarySource}; +use ahash::AHashMap; use diffus::{edit, Diffable}; use semver::Version; use serde::{ser::SerializeStruct, Serialize}; use std::{ - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet}, fmt, mem, }; @@ -182,7 +183,8 @@ impl<'a> PackageDiff<'a> { // --- fn combine_insert_remove(changed: &mut BTreeMap<&'a SummaryId, SummaryDiffStatus<'a>>) { - let mut combine_statuses = HashMap::with_capacity(changed.len()); + let mut combine_statuses: AHashMap<&str, CombineStatus<'_>> = + AHashMap::with_capacity(changed.len()); for (summary_id, status) in &*changed { let entry = combine_statuses diff --git a/guppy/Cargo.toml b/guppy/Cargo.toml index 21241ee1448..159f6effc47 100644 --- a/guppy/Cargo.toml +++ b/guppy/Cargo.toml @@ -29,6 +29,7 @@ rustdoc-args = ["--cfg=doc_cfg"] maintenance = { status = "actively-developed" } [dependencies] +ahash.workspace = true camino = "1.1.6" cargo_metadata = "0.18.1" cfg-if = "1.0.0" diff --git a/guppy/src/graph/build.rs b/guppy/src/graph/build.rs index dd5ead98ec0..48687b63b48 100644 --- a/guppy/src/graph/build.rs +++ b/guppy/src/graph/build.rs @@ -11,6 +11,7 @@ use crate::{ sorted_set::SortedSet, Error, PackageId, }; +use ahash::AHashMap; use camino::{Utf8Path, Utf8PathBuf}; use cargo_metadata::{ DepKindInfo, Dependency, DependencyKind, Metadata, Node, NodeDep, Package, Target, @@ -23,7 +24,7 @@ use semver::{Version, VersionReq}; use smallvec::SmallVec; use std::{ borrow::Cow, - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, HashSet}, sync::Arc, }; use target_spec::TargetSpec; @@ -49,7 +50,7 @@ impl PackageGraph { &workspace_members, ); - let packages: HashMap<_, _> = metadata + let packages: AHashMap<_, _> = metadata .packages .into_iter() .map(|package| build_state.process_package(package)) @@ -83,7 +84,7 @@ impl WorkspaceImpl { workspace_root: impl Into, target_directory: impl Into, metadata_table: serde_json::Value, - packages: &HashMap, + packages: &AHashMap, members: impl IntoIterator, ) -> Result { use std::collections::btree_map::Entry; @@ -140,13 +141,13 @@ impl WorkspaceImpl { /// Helper struct for building up dependency graph. struct GraphBuildState<'a> { dep_graph: Graph, - package_data: HashMap>, + package_data: AHashMap>, // The above, except by package name. - by_package_name: HashMap>>, + by_package_name: AHashMap>>, // The values of resolve_data are the resolved dependencies. This is mutated so it is stored // separately from package_data. - resolve_data: HashMap>, + resolve_data: AHashMap>, workspace_root: &'a Utf8Path, workspace_members: &'a HashSet, } @@ -164,11 +165,11 @@ impl<'a> GraphBuildState<'a> { let all_package_data = packages .iter() .map(|package| PackageDataValue::new(package, &mut dep_graph)) - .collect::>(); + .collect::>(); // While it is possible to have duplicate names so the hash map is smaller, just make this // as big as package_data. - let mut by_package_name: HashMap>> = - HashMap::with_capacity(all_package_data.len()); + let mut by_package_name: AHashMap>> = + AHashMap::with_capacity(all_package_data.len()); for package_data in all_package_data.values() { by_package_name .entry(package_data.name.clone()) @@ -176,7 +177,7 @@ impl<'a> GraphBuildState<'a> { .push(package_data.clone()); } - let resolve_data: HashMap<_, _> = resolve_nodes + let resolve_data: AHashMap<_, _> = resolve_nodes .into_iter() .map(|node| { ( @@ -656,7 +657,7 @@ struct DependencyResolver<'g> { from_id: &'g PackageId, /// The package data, inherited from the graph build state. - package_data: &'g HashMap>, + package_data: &'g AHashMap>, /// This is a list of dependency requirements. We don't know the package ID yet so we don't have /// a great key to work with. This could be improved in the future by matching on requirements @@ -668,8 +669,8 @@ impl<'g> DependencyResolver<'g> { /// Constructs a new resolver using the provided package data and dependencies. fn new( from_id: &'g PackageId, - package_data: &'g HashMap>, - by_package_name: &'g HashMap>>, + package_data: &'g AHashMap>, + by_package_name: &'g AHashMap>>, package_deps: impl IntoIterator, ) -> Self { let mut dep_reqs = DependencyReqs::default(); @@ -698,29 +699,6 @@ impl<'g> DependencyResolver<'g> { } } - // let mut resolved_name_map: HashMap<_, DependencyReqs<'_>> = HashMap::new(); - - // for dep in package_deps { - // match &dep.rename { - // Some(rename) => { - // // The resolved name is the renamed name, except dashes are replaced with - // // underscores. - // let resolved_name = rename.replace('-', "_"); - // resolved_name_map - // .entry(resolved_name.into()) - // .or_default() - // .push(dep); - // } - // None => { - // let resolved_name = dep.name.replace('-', "_"); - // resolved_name_map - // .entry(resolved_name.into()) - // .or_default() - // .push(dep); - // } - // } - // } - Self { from_id, package_data, diff --git a/guppy/src/graph/feature/build.rs b/guppy/src/graph/feature/build.rs index 245a2098f00..0f400336ac8 100644 --- a/guppy/src/graph/feature/build.rs +++ b/guppy/src/graph/feature/build.rs @@ -13,11 +13,12 @@ use crate::{ }, platform::PlatformStatusImpl, }; +use ahash::AHashMap; use cargo_metadata::DependencyKind; use once_cell::sync::OnceCell; use petgraph::{prelude::*, visit::IntoEdgeReferences}; use smallvec::SmallVec; -use std::{collections::HashMap, iter}; +use std::iter; pub(super) type FeaturePetgraph = Graph; pub(super) type FeatureEdgeReference<'g> = <&'g FeaturePetgraph as IntoEdgeReferences>::EdgeRef; @@ -27,7 +28,7 @@ pub(super) struct FeatureGraphBuildState { graph: FeaturePetgraph, // Map from package ixs to the base (first) feature for each package. base_ixs: Vec>, - map: HashMap, + map: AHashMap, weak: WeakDependencies, warnings: Vec, } @@ -41,7 +42,7 @@ impl FeatureGraphBuildState { // Each package corresponds to exactly one base feature ix, and there's one last ix at // the end. base_ixs: Vec::with_capacity(package_count + 1), - map: HashMap::with_capacity(package_count), + map: AHashMap::with_capacity(package_count), weak: WeakDependencies::new(), warnings: vec![], } @@ -68,7 +69,7 @@ impl FeatureGraphBuildState { } pub(super) fn add_named_feature_edges(&mut self, metadata: PackageMetadata<'_>) { - let dep_name_to_link: HashMap<_, _> = metadata + let dep_name_to_link: AHashMap<_, _> = metadata .direct_links() .map(|link| (link.dep_name(), link)) .collect(); @@ -102,7 +103,7 @@ impl FeatureGraphBuildState { metadata: PackageMetadata<'_>, from_label: FeatureLabel<'_>, feature_dep: &NamedFeatureDep, - dep_name_to_link: &HashMap<&str, PackageLink>, + dep_name_to_link: &AHashMap<&str, PackageLink>, ) -> SmallVec<[(FeatureNode, FeatureEdge); 3]> { let mut nodes_edges: SmallVec<[(FeatureNode, FeatureEdge); 3]> = SmallVec::new(); @@ -477,7 +478,7 @@ struct FeatureReq<'g> { edge_ix: EdgeIndex, to_default_idx: FeatureIndexInPackage, // This will contain any build states that aren't empty. - features: HashMap, + features: AHashMap, } impl<'g> FeatureReq<'g> { @@ -490,7 +491,7 @@ impl<'g> FeatureReq<'g> { to_default_idx: to .get_feature_idx(FeatureLabel::Named("default")) .unwrap_or(FeatureIndexInPackage::Base), - features: HashMap::new(), + features: AHashMap::new(), } } diff --git a/guppy/src/graph/feature/graph_impl.rs b/guppy/src/graph/feature/graph_impl.rs index 99864a1b8ae..b4df570858a 100644 --- a/guppy/src/graph/feature/graph_impl.rs +++ b/guppy/src/graph/feature/graph_impl.rs @@ -16,13 +16,14 @@ use crate::{ platform::{PlatformStatus, PlatformStatusImpl}, DependencyKind, Error, PackageId, }; +use ahash::AHashMap; use once_cell::sync::OnceCell; use petgraph::{ algo::has_path_connecting, prelude::*, visit::{EdgeFiltered, IntoNodeReferences}, }; -use std::{collections::HashMap, fmt, iter, iter::FromIterator}; +use std::{fmt, iter, iter::FromIterator}; // Some general notes about feature graphs: // @@ -646,7 +647,7 @@ pub(in crate::graph) struct FeatureGraphImpl { pub(super) graph: FeaturePetgraph, // base ixs consists of the base (start) feature indexes for each package. pub(super) base_ixs: Vec>, - pub(super) map: HashMap, + pub(super) map: AHashMap, pub(super) warnings: Vec, // The strongly connected components of the feature graph. Computed on demand. pub(super) sccs: OnceCell>, diff --git a/guppy/src/graph/graph_impl.rs b/guppy/src/graph/graph_impl.rs index a5f1602998b..83f3a8da7f9 100644 --- a/guppy/src/graph/graph_impl.rs +++ b/guppy/src/graph/graph_impl.rs @@ -12,6 +12,7 @@ use crate::{ platform::{EnabledTernary, PlatformSpec, PlatformStatus, PlatformStatusImpl}, CargoMetadata, DependencyKind, Error, JsonValue, MetadataCommand, PackageId, }; +use ahash::AHashMap; use camino::{Utf8Path, Utf8PathBuf}; use fixedbitset::FixedBitSet; use indexmap::{IndexMap, IndexSet}; @@ -25,7 +26,7 @@ use petgraph::{ use semver::{Version, VersionReq}; use smallvec::SmallVec; use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, HashSet}, fmt, iter, iter::FromIterator, }; @@ -54,7 +55,7 @@ pub struct PackageGraph { /// Per-package data for a PackageGraph instance. #[derive(Clone, Debug)] pub(super) struct PackageGraphData { - pub(super) packages: HashMap, + pub(super) packages: AHashMap, pub(super) workspace: WorkspaceImpl, } diff --git a/guppy/src/graph/summaries/package_set.rs b/guppy/src/graph/summaries/package_set.rs index 05bd0630737..d6bd430f461 100644 --- a/guppy/src/graph/summaries/package_set.rs +++ b/guppy/src/graph/summaries/package_set.rs @@ -9,16 +9,13 @@ use crate::{ }, Error, PackageId, }; +use ahash::AHashMap; use camino::Utf8PathBuf; use guppy_summaries::SummaryId; use semver::VersionReq; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use smallvec::SmallVec; -use std::{ - borrow::Cow, - collections::{BTreeSet, HashMap}, - fmt, -}; +use std::{borrow::Cow, collections::BTreeSet, fmt}; /// A set of packages specified in a summary. Can be resolved into a `PackageSet` given a /// `PackageGraph`. @@ -569,10 +566,10 @@ fn version_req_is_star(req: &VersionReq) -> bool { struct PackageMatcher<'a> { // The bools are to ensure that all the packages specified in the summary actually get matched // against something in the metadata. - summary_ids: HashMap<&'a SummaryId, bool>, + summary_ids: AHashMap<&'a SummaryId, bool>, workspace_members: &'a BTreeSet, - third_party: HashMap<&'a str, SmallVec<[(&'a ThirdPartySummary, bool); 2]>>, - registry_names_to_urls: HashMap<&'a str, &'a str>, + third_party: AHashMap<&'a str, SmallVec<[(&'a ThirdPartySummary, bool); 2]>>, + registry_names_to_urls: AHashMap<&'a str, &'a str>, } impl<'a> PackageMatcher<'a> { @@ -587,8 +584,8 @@ impl<'a> PackageMatcher<'a> { .map(|summary_id| (summary_id, false)) .collect(); - let mut third_party: HashMap<_, SmallVec<[_; 2]>> = HashMap::new(); - let mut registry_names_to_urls = HashMap::new(); + let mut third_party: AHashMap<_, SmallVec<[_; 2]>> = AHashMap::new(); + let mut registry_names_to_urls = AHashMap::new(); for tp_summary in &summary.third_party { if let ThirdPartySource::Registry(Some(name)) = &tp_summary.source { if !registry_names_to_urls.contains_key(name.as_str()) { @@ -730,7 +727,7 @@ impl<'a> PackageMatcher<'a> { fn source_matches( package_source: PackageSource<'_>, third_party_source: &ThirdPartySource, - registry_names_to_urls: &HashMap<&'a str, &'a str>, + registry_names_to_urls: &AHashMap<&'a str, &'a str>, ) -> bool { match (package_source, third_party_source) { (PackageSource::Workspace(_), _) => { diff --git a/guppy/src/petgraph_support/scc.rs b/guppy/src/petgraph_support/scc.rs index c2f582f9553..199a4205982 100644 --- a/guppy/src/petgraph_support/scc.rs +++ b/guppy/src/petgraph_support/scc.rs @@ -1,6 +1,7 @@ // Copyright (c) The cargo-guppy Contributors // SPDX-License-Identifier: MIT OR Apache-2.0 +use ahash::AHashMap; use fixedbitset::FixedBitSet; use nested::Nested; use petgraph::{ @@ -9,12 +10,12 @@ use petgraph::{ prelude::*, visit::{IntoNeighborsDirected, IntoNodeIdentifiers, VisitMap, Visitable}, }; -use std::{collections::HashMap, slice}; +use std::slice; #[derive(Clone, Debug)] pub(crate) struct Sccs { sccs: Nested>>, - multi_map: HashMap, usize>, + multi_map: AHashMap, usize>, } impl Sccs { @@ -39,7 +40,7 @@ impl Sccs { // forward topological order. .rev() .collect(); - let mut multi_map = HashMap::new(); + let mut multi_map = AHashMap::new(); for (idx, scc) in sccs.iter().enumerate() { if scc.len() > 1 { multi_map.extend(scc.iter().map(|ix| (*ix, idx))); diff --git a/internal-tools/benchmarks/benches/package_graph.rs b/internal-tools/benchmarks/benches/package_graph.rs index 055033a970b..0d01fa021a6 100644 --- a/internal-tools/benchmarks/benches/package_graph.rs +++ b/internal-tools/benchmarks/benches/package_graph.rs @@ -102,6 +102,8 @@ fn make_package_graph() -> PackageGraph { fn make_package_name_hashmap<'g>( graph: &'g PackageGraph, ) -> HashMap<&'g str, Vec>> { + // Testing the real HashMap is fine here. + #[allow(clippy::disallowed_methods)] let mut hashmap: HashMap<&'g str, Vec<_>> = HashMap::new(); for package in graph.packages() { hashmap.entry(package.name()).or_default().push(package); diff --git a/tools/determinator/Cargo.toml b/tools/determinator/Cargo.toml index c3a37e4b0a4..63612bb5169 100644 --- a/tools/determinator/Cargo.toml +++ b/tools/determinator/Cargo.toml @@ -25,6 +25,7 @@ include = [ rust-version = "1.70" [dependencies] +ahash.workspace = true camino = "1.1.6" globset = "0.4.13" guppy = { version = "0.17.1", path = "../../guppy", features = [ diff --git a/tools/determinator/src/determinator.rs b/tools/determinator/src/determinator.rs index 97223ef642f..aa4acdab457 100644 --- a/tools/determinator/src/determinator.rs +++ b/tools/determinator/src/determinator.rs @@ -8,6 +8,7 @@ use crate::{ RulesImpl, }, }; +use ahash::AHashMap; use camino::Utf8Path; use globset::Candidate; use guppy::{ @@ -21,7 +22,7 @@ use guppy::{ }; use petgraph::{graphmap::GraphMap, Directed}; use rayon::prelude::*; -use std::collections::{hash_map::Entry, HashMap, HashSet}; +use std::collections::{hash_map::Entry, HashSet}; /// Determine target dependencies from changed files and packages in a workspace. /// @@ -426,7 +427,7 @@ fn process_path<'g>( /// Stores a build cache of every package in a workspace. #[derive(Debug)] struct CargoBuildCache<'g> { - result_cache: HashMap<&'g PackageId, BuildResult<'g>>, + result_cache: AHashMap<&'g PackageId, BuildResult<'g>>, } impl<'g> CargoBuildCache<'g> { @@ -441,7 +442,7 @@ impl<'g> CargoBuildCache<'g> { .as_ref() .unwrap_or(&default_features_only); - let result_cache: HashMap<_, _> = workspace + let result_cache: ahash::HashMap<_, _> = workspace .par_iter() .map(|package| { let id = package.id(); @@ -450,7 +451,9 @@ impl<'g> CargoBuildCache<'g> { }) .collect(); - Self { result_cache } + Self { + result_cache: result_cache.into(), + } } } @@ -645,7 +648,7 @@ impl<'g> ReverseIndex<'g> { .collect(); // Do a DFS with two maps, in case there are cycles (can happen with dev deps). - let mut discovered = HashMap::new(); + let mut discovered = AHashMap::new(); let mut finished = HashSet::new(); while let Some(&(id, follow)) = stack.last() { diff --git a/tools/hakari/Cargo.toml b/tools/hakari/Cargo.toml index 9ecdc59e7fb..b9e1bcb2fdb 100644 --- a/tools/hakari/Cargo.toml +++ b/tools/hakari/Cargo.toml @@ -22,6 +22,7 @@ all-features = true rustdoc-args = ["--cfg=doc_cfg"] [dependencies] +ahash = "0.8.6" atomicwrites = "0.4.2" bimap = "0.6.3" camino = "1.1.6" diff --git a/tools/hakari/src/hakari.rs b/tools/hakari/src/hakari.rs index 156cd750239..4d74d0d30a1 100644 --- a/tools/hakari/src/hakari.rs +++ b/tools/hakari/src/hakari.rs @@ -7,6 +7,7 @@ use crate::{ toml_out::{write_toml, HakariOutputOptions}, CargoTomlError, HakariCargoToml, TomlOutError, }; +use ahash::AHashMap; use bimap::BiHashMap; use debug_ignore::DebugIgnore; use guppy::{ @@ -22,7 +23,7 @@ use guppy::{ use rayon::prelude::*; use std::{ borrow::Cow, - collections::{BTreeMap, BTreeSet, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet, HashSet}, fmt, sync::Arc, }; @@ -39,7 +40,7 @@ pub struct HakariBuilder<'g> { pub(crate) verify_mode: bool, pub(crate) traversal_excludes: HashSet<&'g PackageId>, final_excludes: HashSet<&'g PackageId>, - pub(crate) registries: BiHashMap, + pub(crate) registries: BiHashMap, unify_target_host: UnifyTargetHost, output_single_feature: bool, pub(crate) dep_format_version: DepFormatVersion, @@ -78,7 +79,7 @@ impl<'g> HakariBuilder<'g> { verify_mode: false, traversal_excludes: HashSet::new(), final_excludes: HashSet::new(), - registries: BiHashMap::new(), + registries: BiHashMap::with_hashers(Default::default(), Default::default()), unify_target_host: UnifyTargetHost::default(), output_single_feature: false, dep_format_version: DepFormatVersion::default(), @@ -408,7 +409,7 @@ mod summaries { }) .collect::, _>>()?; - let registries: BiHashMap<_, _> = summary + let registries: BiHashMap<_, _, ahash::RandomState, ahash::RandomState> = summary .registries .iter() .map(|(name, url)| (name.clone(), url.clone())) @@ -666,7 +667,7 @@ impl<'g> Hakari<'g> { /// /// Packages which have one version are present as their original names, while packages with /// more than one version have a hash appended to them. - pub fn toml_name_map(&self) -> HashMap, PackageMetadata<'g>> { + pub fn toml_name_map(&self) -> AHashMap, PackageMetadata<'g>> { toml_name_map(&self.output_map, self.builder.dep_format_version) } diff --git a/tools/hakari/src/toml_out.rs b/tools/hakari/src/toml_out.rs index f2249b81d02..fd2c6a77f8d 100644 --- a/tools/hakari/src/toml_out.rs +++ b/tools/hakari/src/toml_out.rs @@ -10,6 +10,7 @@ use crate::{ helpers::VersionDisplay, DepFormatVersion, }; +use ahash::AHashMap; use camino::Utf8PathBuf; use cfg_if::cfg_if; use guppy::{ @@ -19,7 +20,7 @@ use guppy::{ }; use std::{ borrow::Cow, - collections::{HashMap, HashSet}, + collections::HashSet, error, fmt, hash::{Hash, Hasher}, }; @@ -179,8 +180,8 @@ pub enum TomlOutError { pub(crate) fn toml_name_map<'g>( output_map: &OutputMap<'g>, dep_format: DepFormatVersion, -) -> HashMap, PackageMetadata<'g>> { - let mut packages_by_name: HashMap<&'g str, HashMap<_, _>> = HashMap::new(); +) -> AHashMap, PackageMetadata<'g>> { + let mut packages_by_name: AHashMap<&'g str, AHashMap<_, _>> = AHashMap::new(); for vals in output_map.values() { for (&package_id, (package, _)) in vals { packages_by_name @@ -190,7 +191,7 @@ pub(crate) fn toml_name_map<'g>( } } - let mut toml_name_map = HashMap::new(); + let mut toml_name_map = AHashMap::new(); for (name, packages) in packages_by_name { if packages.len() > 1 { // Make hashed names for each package. @@ -286,7 +287,7 @@ pub(crate) fn write_toml( } } - let mut packages_by_name: HashMap<&str, HashSet<_>> = HashMap::new(); + let mut packages_by_name: AHashMap<&str, HashSet<_>> = AHashMap::new(); for vals in output_map.values() { for (&package_id, (package, _)) in vals { packages_by_name diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 4d6f7166762..b791228d534 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -18,6 +18,7 @@ repository = "https://github.com/guppy-rs/guppy" [dependencies] clap = { version = "4.3.19", features = ["derive"] } clap_builder = { version = "4.3.19", default-features = false, features = ["color", "help", "std", "suggestions", "usage"] } +getrandom = { version = "0.2.10", default-features = false, features = ["std"] } indexmap = { version = "1.9.3", default-features = false, features = ["std"] } libc = { version = "0.2.149" } log = { version = "0.4.20", default-features = false, features = ["std"] } @@ -39,13 +40,16 @@ syn-dff4ba8e3ae991db = { package = "syn", version = "1.0.109", features = ["extr syn-f595c2ba2a3f28df = { package = "syn", version = "2.0.32", features = ["extra-traits", "full"] } [target.x86_64-unknown-linux-gnu.dependencies] +once_cell = { version = "1.18.0", features = ["unstable"] } rustix = { version = "0.38.21", features = ["fs", "termios"] } [target.x86_64-apple-darwin.dependencies] libc = { version = "0.2.149", default-features = false, features = ["extra_traits"] } +once_cell = { version = "1.18.0", features = ["unstable"] } rustix = { version = "0.38.21", features = ["fs", "termios"] } [target.x86_64-pc-windows-msvc.dependencies] +once_cell = { version = "1.18.0", features = ["unstable"] } winapi = { version = "0.3.9", default-features = false, features = ["consoleapi", "errhandlingapi", "fileapi", "handleapi", "minwinbase", "minwindef", "processenv", "std", "synchapi", "winbase", "wincon", "winerror", "winnt"] } windows-sys = { version = "0.48.0", features = ["Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Console", "Win32_System_Pipes", "Win32_System_Threading"] }