Skip to content

Commit f660b3e

Browse files
committed
Turn Workspace::is_member into an O(1) query
This commit moves it from a linear query which does a lot of hashing of `PathBuf` to an `O(1)` query that only hashes `PackageId`. This improves Servo's null build performance by 50ms or so, causing a number of functions to disappear from profiles.
1 parent e454446 commit f660b3e

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

src/cargo/core/workspace.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::cell::RefCell;
22
use std::collections::hash_map::{Entry, HashMap};
3-
use std::collections::BTreeMap;
3+
use std::collections::{BTreeMap, HashSet};
44
use std::path::{Path, PathBuf};
55
use std::slice;
66

@@ -10,7 +10,7 @@ use url::Url;
1010

1111
use crate::core::profiles::Profiles;
1212
use crate::core::registry::PackageRegistry;
13-
use crate::core::{Dependency, PackageIdSpec};
13+
use crate::core::{Dependency, PackageIdSpec, PackageId};
1414
use crate::core::{EitherManifest, Package, SourceId, VirtualManifest};
1515
use crate::ops;
1616
use crate::sources::PathSource;
@@ -51,6 +51,7 @@ pub struct Workspace<'cfg> {
5151
// paths. The packages themselves can be looked up through the `packages`
5252
// set above.
5353
members: Vec<PathBuf>,
54+
member_ids: HashSet<PackageId>,
5455

5556
// The subset of `members` that are used by the
5657
// `build`, `check`, `test`, and `bench` subcommands
@@ -148,6 +149,7 @@ impl<'cfg> Workspace<'cfg> {
148149
root_manifest: None,
149150
target_dir,
150151
members: Vec::new(),
152+
member_ids: HashSet::new(),
151153
default_members: Vec::new(),
152154
is_ephemeral: false,
153155
require_optional_deps: true,
@@ -185,6 +187,7 @@ impl<'cfg> Workspace<'cfg> {
185187
root_manifest: None,
186188
target_dir: None,
187189
members: Vec::new(),
190+
member_ids: HashSet::new(),
188191
default_members: Vec::new(),
189192
is_ephemeral: true,
190193
require_optional_deps,
@@ -193,6 +196,7 @@ impl<'cfg> Workspace<'cfg> {
193196
};
194197
{
195198
let key = ws.current_manifest.parent().unwrap();
199+
let id = package.package_id();
196200
let package = MaybePackage::Package(package);
197201
ws.packages.packages.insert(key.to_path_buf(), package);
198202
ws.target_dir = if let Some(dir) = target_dir {
@@ -201,6 +205,7 @@ impl<'cfg> Workspace<'cfg> {
201205
ws.config.target_dir()?
202206
};
203207
ws.members.push(ws.current_manifest.clone());
208+
ws.member_ids.insert(id);
204209
ws.default_members.push(ws.current_manifest.clone());
205210
}
206211
Ok(ws)
@@ -315,7 +320,7 @@ impl<'cfg> Workspace<'cfg> {
315320

316321
/// Returns true if the package is a member of the workspace.
317322
pub fn is_member(&self, pkg: &Package) -> bool {
318-
self.members().any(|p| p == pkg)
323+
self.member_ids.contains(&pkg.package_id())
319324
}
320325

321326
pub fn is_ephemeral(&self) -> bool {
@@ -430,6 +435,10 @@ impl<'cfg> Workspace<'cfg> {
430435
debug!("find_members - only me as a member");
431436
self.members.push(self.current_manifest.clone());
432437
self.default_members.push(self.current_manifest.clone());
438+
if let Ok(pkg) = self.current() {
439+
let id = pkg.package_id();
440+
self.member_ids.insert(id);
441+
}
433442
return Ok(());
434443
}
435444
};
@@ -515,6 +524,7 @@ impl<'cfg> Workspace<'cfg> {
515524
MaybePackage::Package(ref p) => p,
516525
MaybePackage::Virtual(_) => return Ok(()),
517526
};
527+
self.member_ids.insert(pkg.package_id());
518528
pkg.dependencies()
519529
.iter()
520530
.map(|d| d.source_id())

0 commit comments

Comments
 (0)