Skip to content

Commit d83f113

Browse files
committed
Auto merge of #5302 - Eh2406:MoreRc, r=alexcrichton
use more Rc in the part of resolver that gets cloned a lot 2 This is the same idea as #5118, I was looking at a profile and noted that ~5% of our time was sent dropping `HashMap<PackageId, HashSet<InternedString>>`. A quick rg and I found the culprit, we are cloning the set of features for every new `Context`. With some Rc we are now just cloning for each time we insert. To benchmark I commented out line https://github.com/rust-lang/cargo/blob/b9aa315158fe4d8d63672a49200401922ef7385d/src/cargo/core/resolver/mod.rs#L453 the smallest change to get #4810 (comment) not to solve instantly. Before 17000000 ticks, 90s, 188.889 ticks/ms After 17000000 ticks, 73s, 232.877 ticks/ms
2 parents b9aa315 + 04fbc5f commit d83f113

File tree

1 file changed

+7
-8
lines changed

1 file changed

+7
-8
lines changed

src/cargo/core/resolver/context.rs

+7-8
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub struct Context {
2323
// switch to persistent hash maps if we can at some point or otherwise
2424
// make these much cheaper to clone in general.
2525
pub activations: Activations,
26-
pub resolve_features: HashMap<PackageId, HashSet<InternedString>>,
26+
pub resolve_features: HashMap<PackageId, Rc<HashSet<InternedString>>>,
2727
pub links: HashMap<InternedString, PackageId>,
2828

2929
// These are two cheaply-cloneable lists (O(1) clone) which are effectively
@@ -36,6 +36,8 @@ pub struct Context {
3636
pub warnings: RcList<String>,
3737
}
3838

39+
pub type Activations = HashMap<(InternedString, SourceId), Rc<Vec<Summary>>>;
40+
3941
impl Context {
4042
pub fn new() -> Context {
4143
Context {
@@ -66,9 +68,7 @@ impl Context {
6668
&*link
6769
);
6870
}
69-
let mut inner: Vec<_> = (**prev).clone();
70-
inner.push(summary.clone());
71-
*prev = Rc::new(inner);
71+
Rc::make_mut(prev).push(summary.clone());
7272
return Ok(false);
7373
}
7474
debug!("checking if {} is already activated", summary.package_id());
@@ -244,9 +244,10 @@ impl Context {
244244
if !reqs.used.is_empty() {
245245
let pkgid = s.package_id();
246246

247-
let set = self.resolve_features
247+
let set = Rc::make_mut(self.resolve_features
248248
.entry(pkgid.clone())
249-
.or_insert_with(HashSet::new);
249+
.or_insert_with(|| Rc::new(HashSet::new())));
250+
250251
for feature in reqs.used {
251252
set.insert(InternedString::new(feature));
252253
}
@@ -405,5 +406,3 @@ impl<'r> Requirements<'r> {
405406
}
406407
}
407408
}
408-
409-
pub type Activations = HashMap<(InternedString, SourceId), Rc<Vec<Summary>>>;

0 commit comments

Comments
 (0)