Skip to content

Commit e47bec1

Browse files
nnethercotennethercote
committed
Avoid closures to improve compile times.
`HashMap` and `HashSet` are used widely, and often instantiated many times. As a result, small differences in how the code is written can have a significant effect on how much LLVM IR is generated, which affects compile times. This commit avoids a lot of small closures by replacing calls to `Option::map`, `Option::ok_or_else`, `Option::unwrap_or_else` and `Result::unwrap_or_else` with `match` expressions. Although this makes the code less concise, it improves compile times. For example, several of the benchmarks in rustc-perf compile up to 3.5% faster after this change is incorporated into std. Every change is accompanied by a short comment to explain why a `match` is used. This may seem excessive, but without these comments it would be easy for a well-meaning person in the future to change some or all of these back to the original versions without understanding the consequences.
1 parent 7593ec9 commit e47bec1

File tree

3 files changed

+139
-60
lines changed

3 files changed

+139
-60
lines changed

‎src/map.rs

+66-32
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,11 @@ where
802802
K: Borrow<Q>,
803803
Q: Hash + Eq,
804804
{
805-
self.get_key_value(k).map(|(_, v)| v)
805+
// Avoid `Option::map` because it bloats LLVM IR.
806+
match self.get_key_value(k) {
807+
Some((_, v)) => Some(v),
808+
None => None,
809+
}
806810
}
807811

808812
/// Returns the key-value pair corresponding to the supplied key.
@@ -831,12 +835,14 @@ where
831835
Q: Hash + Eq,
832836
{
833837
let hash = make_hash(&self.hash_builder, k);
834-
self.table
835-
.find(hash, |x| k.eq(x.0.borrow()))
836-
.map(|item| unsafe {
838+
// Avoid `Option::map` because it bloats LLVM IR.
839+
match self.table.find(hash, |x| k.eq(x.0.borrow())) {
840+
Some(item) => unsafe {
837841
let &(ref key, ref value) = item.as_ref();
838-
(key, value)
839-
})
842+
Some((key, value))
843+
}
844+
None => None,
845+
}
840846
}
841847

842848
/// Returns the key-value pair corresponding to the supplied key, with a mutable reference to value.
@@ -869,12 +875,14 @@ where
869875
Q: Hash + Eq,
870876
{
871877
let hash = make_hash(&self.hash_builder, k);
872-
self.table
873-
.find(hash, |x| k.eq(x.0.borrow()))
874-
.map(|item| unsafe {
878+
// Avoid `Option::map` because it bloats LLVM IR.
879+
match self.table.find(hash, |x| k.eq(x.0.borrow())) {
880+
Some(item) => unsafe {
875881
let &mut (ref key, ref mut value) = item.as_mut();
876-
(key, value)
877-
})
882+
Some((key, value))
883+
}
884+
None => None,
885+
}
878886
}
879887

880888
/// Returns `true` if the map contains a value for the specified key.
@@ -933,9 +941,11 @@ where
933941
Q: Hash + Eq,
934942
{
935943
let hash = make_hash(&self.hash_builder, k);
936-
self.table
937-
.find(hash, |x| k.eq(x.0.borrow()))
938-
.map(|item| unsafe { &mut item.as_mut().1 })
944+
// Avoid `Option::map` because it bloats LLVM IR.
945+
match self.table.find(hash, |x| k.eq(x.0.borrow())) {
946+
Some(item) => Some(unsafe { &mut item.as_mut().1 }),
947+
None => None,
948+
}
939949
}
940950

941951
/// Inserts a key-value pair into the map.
@@ -1004,7 +1014,11 @@ where
10041014
K: Borrow<Q>,
10051015
Q: Hash + Eq,
10061016
{
1007-
self.remove_entry(k).map(|(_, v)| v)
1017+
// Avoid `Option::map` because it bloats LLVM IR.
1018+
match self.remove_entry(k) {
1019+
Some((_, v)) => Some(v),
1020+
None => None,
1021+
}
10081022
}
10091023

10101024
/// Removes a key from the map, returning the stored key and value if the
@@ -1561,13 +1575,13 @@ impl<'a, K, V, S> RawEntryBuilder<'a, K, V, S> {
15611575
where
15621576
F: FnMut(&K) -> bool,
15631577
{
1564-
self.map
1565-
.table
1566-
.find(hash, |(k, _)| is_match(k))
1567-
.map(|item| unsafe {
1578+
match self.map.table.find(hash, |(k, _)| is_match(k)) {
1579+
Some(item) => unsafe {
15681580
let &(ref key, ref value) = item.as_ref();
1569-
(key, value)
1570-
})
1581+
Some((key, value))
1582+
}
1583+
None => None,
1584+
}
15711585
}
15721586

15731587
/// Access an entry by hash.
@@ -2030,10 +2044,14 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> {
20302044

20312045
#[cfg_attr(feature = "inline-more", inline)]
20322046
fn next(&mut self) -> Option<(&'a K, &'a V)> {
2033-
self.inner.next().map(|x| unsafe {
2034-
let r = x.as_ref();
2035-
(&r.0, &r.1)
2036-
})
2047+
// Avoid `Option::map` because it bloats LLVM IR.
2048+
match self.inner.next() {
2049+
Some(x) => unsafe {
2050+
let r = x.as_ref();
2051+
Some((&r.0, &r.1))
2052+
}
2053+
None => None,
2054+
}
20372055
}
20382056
#[cfg_attr(feature = "inline-more", inline)]
20392057
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2054,10 +2072,14 @@ impl<'a, K, V> Iterator for IterMut<'a, K, V> {
20542072

20552073
#[cfg_attr(feature = "inline-more", inline)]
20562074
fn next(&mut self) -> Option<(&'a K, &'a mut V)> {
2057-
self.inner.next().map(|x| unsafe {
2058-
let r = x.as_mut();
2059-
(&r.0, &mut r.1)
2060-
})
2075+
// Avoid `Option::map` because it bloats LLVM IR.
2076+
match self.inner.next() {
2077+
Some(x) => unsafe {
2078+
let r = x.as_mut();
2079+
Some((&r.0, &mut r.1))
2080+
}
2081+
None => None,
2082+
}
20612083
}
20622084
#[cfg_attr(feature = "inline-more", inline)]
20632085
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2113,7 +2135,11 @@ impl<'a, K, V> Iterator for Keys<'a, K, V> {
21132135

21142136
#[cfg_attr(feature = "inline-more", inline)]
21152137
fn next(&mut self) -> Option<&'a K> {
2116-
self.inner.next().map(|(k, _)| k)
2138+
// Avoid `Option::map` because it bloats LLVM IR.
2139+
match self.inner.next() {
2140+
Some((k, _)) => Some(k),
2141+
None => None,
2142+
}
21172143
}
21182144
#[cfg_attr(feature = "inline-more", inline)]
21192145
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2133,7 +2159,11 @@ impl<'a, K, V> Iterator for Values<'a, K, V> {
21332159

21342160
#[cfg_attr(feature = "inline-more", inline)]
21352161
fn next(&mut self) -> Option<&'a V> {
2136-
self.inner.next().map(|(_, v)| v)
2162+
// Avoid `Option::map` because it bloats LLVM IR.
2163+
match self.inner.next() {
2164+
Some((_, v)) => Some(v),
2165+
None => None,
2166+
}
21372167
}
21382168
#[cfg_attr(feature = "inline-more", inline)]
21392169
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2153,7 +2183,11 @@ impl<'a, K, V> Iterator for ValuesMut<'a, K, V> {
21532183

21542184
#[cfg_attr(feature = "inline-more", inline)]
21552185
fn next(&mut self) -> Option<&'a mut V> {
2156-
self.inner.next().map(|(_, v)| v)
2186+
// Avoid `Option::map` because it bloats LLVM IR.
2187+
match self.inner.next() {
2188+
Some((_, v)) => Some(v),
2189+
None => None,
2190+
}
21572191
}
21582192
#[cfg_attr(feature = "inline-more", inline)]
21592193
fn size_hint(&self) -> (usize, Option<usize>) {

‎src/raw/mod.rs

+53-24
Original file line numberDiff line numberDiff line change
@@ -402,9 +402,16 @@ impl<T> RawTable<T> {
402402
fallability: Fallibility,
403403
) -> Result<Self, TryReserveError> {
404404
debug_assert!(buckets.is_power_of_two());
405-
let (layout, ctrl_offset) =
406-
calculate_layout::<T>(buckets).ok_or_else(|| fallability.capacity_overflow())?;
407-
let ptr = NonNull::new(alloc(layout)).ok_or_else(|| fallability.alloc_err(layout))?;
405+
406+
// Avoid `Option::ok_or_else` because it bloats LLVM IR.
407+
let (layout, ctrl_offset) = match calculate_layout::<T>(buckets) {
408+
Some(lco) => lco,
409+
None => return Err(fallability.capacity_overflow())
410+
};
411+
let ptr = match NonNull::new(alloc(layout)) {
412+
Some(ptr) => ptr,
413+
None => return Err(fallability.alloc_err(layout)),
414+
};
408415
let ctrl = NonNull::new_unchecked(ptr.as_ptr().add(ctrl_offset));
409416
Ok(Self {
410417
ctrl,
@@ -425,8 +432,11 @@ impl<T> RawTable<T> {
425432
Ok(Self::new())
426433
} else {
427434
unsafe {
428-
let buckets =
429-
capacity_to_buckets(capacity).ok_or_else(|| fallability.capacity_overflow())?;
435+
// Avoid `Option::ok_or_else` because it bloats LLVM IR.
436+
let buckets = match capacity_to_buckets(capacity) {
437+
Some(buckets) => buckets,
438+
None => return Err(fallability.capacity_overflow()),
439+
};
430440
let result = Self::new_uninitialized(buckets, fallability)?;
431441
result.ctrl(0).write_bytes(EMPTY, result.num_ctrl_bytes());
432442

@@ -445,15 +455,21 @@ impl<T> RawTable<T> {
445455
/// Allocates a new hash table with at least enough capacity for inserting
446456
/// the given number of elements without reallocating.
447457
pub fn with_capacity(capacity: usize) -> Self {
448-
Self::fallible_with_capacity(capacity, Fallibility::Infallible)
449-
.unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() })
458+
// Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
459+
match Self::fallible_with_capacity(capacity, Fallibility::Infallible) {
460+
Ok(capacity) => capacity,
461+
Err(_) => unsafe { hint::unreachable_unchecked() },
462+
}
450463
}
451464

452465
/// Deallocates the table without dropping any entries.
453466
#[cfg_attr(feature = "inline-more", inline)]
454467
unsafe fn free_buckets(&mut self) {
455-
let (layout, ctrl_offset) =
456-
calculate_layout::<T>(self.buckets()).unwrap_or_else(|| hint::unreachable_unchecked());
468+
// Avoid `Option::unwrap_or_else` because it bloats LLVM IR.
469+
let (layout, ctrl_offset) = match calculate_layout::<T>(self.buckets()) {
470+
Some(lco) => lco,
471+
None => hint::unreachable_unchecked(),
472+
};
457473
dealloc(self.ctrl.as_ptr().sub(ctrl_offset), layout);
458474
}
459475

@@ -671,8 +687,10 @@ impl<T> RawTable<T> {
671687
if self.items == 0 {
672688
*self = Self::with_capacity(min_size)
673689
} else {
674-
self.resize(min_size, hasher, Fallibility::Infallible)
675-
.unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() });
690+
// Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
691+
if self.resize(min_size, hasher, Fallibility::Infallible).is_err() {
692+
unsafe { hint::unreachable_unchecked() }
693+
}
676694
}
677695
}
678696
}
@@ -682,8 +700,10 @@ impl<T> RawTable<T> {
682700
#[cfg_attr(feature = "inline-more", inline)]
683701
pub fn reserve(&mut self, additional: usize, hasher: impl Fn(&T) -> u64) {
684702
if additional > self.growth_left {
685-
self.reserve_rehash(additional, hasher, Fallibility::Infallible)
686-
.unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() });
703+
// Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
704+
if self.reserve_rehash(additional, hasher, Fallibility::Infallible).is_err() {
705+
unsafe { hint::unreachable_unchecked() }
706+
}
687707
}
688708
}
689709

@@ -711,11 +731,11 @@ impl<T> RawTable<T> {
711731
hasher: impl Fn(&T) -> u64,
712732
fallability: Fallibility,
713733
) -> Result<(), TryReserveError> {
714-
let new_items = self
715-
.items
716-
.checked_add(additional)
717-
.ok_or_else(|| fallability.capacity_overflow())?;
718-
734+
// Avoid `Option::ok_or_else` because it bloats LLVM IR.
735+
let new_items = match self.items.checked_add(additional) {
736+
Some(new_items) => new_items,
737+
None => return Err(fallability.capacity_overflow()),
738+
};
719739
let full_capacity = bucket_mask_to_capacity(self.bucket_mask);
720740
if new_items <= full_capacity / 2 {
721741
// Rehash in-place without re-allocating if we have plenty of spare
@@ -1065,8 +1085,11 @@ impl<T> RawTable<T> {
10651085
let alloc = if self.is_empty_singleton() {
10661086
None
10671087
} else {
1068-
let (layout, ctrl_offset) = calculate_layout::<T>(self.buckets())
1069-
.unwrap_or_else(|| unsafe { hint::unreachable_unchecked() });
1088+
// Avoid `Option::unwrap_or_else` because it bloats LLVM IR.
1089+
let (layout, ctrl_offset) = match calculate_layout::<T>(self.buckets()) {
1090+
Some(lco) => lco,
1091+
None => unsafe { hint::unreachable_unchecked() },
1092+
};
10701093
Some((
10711094
unsafe { NonNull::new_unchecked(self.ctrl.as_ptr().sub(ctrl_offset)) },
10721095
layout,
@@ -1087,8 +1110,11 @@ impl<T: Clone> Clone for RawTable<T> {
10871110
} else {
10881111
unsafe {
10891112
let mut new_table = ManuallyDrop::new(
1090-
Self::new_uninitialized(self.buckets(), Fallibility::Infallible)
1091-
.unwrap_or_else(|_| hint::unreachable_unchecked()),
1113+
// Avoid `Result::ok_or_else` because it bloats LLVM IR.
1114+
match Self::new_uninitialized(self.buckets(), Fallibility::Infallible) {
1115+
Ok(table) => table,
1116+
Err(_) => hint::unreachable_unchecked(),
1117+
}
10921118
);
10931119

10941120
new_table.clone_from_spec(self, |new_table| {
@@ -1121,8 +1147,11 @@ impl<T: Clone> Clone for RawTable<T> {
11211147
self.free_buckets();
11221148
}
11231149
(self as *mut Self).write(
1124-
Self::new_uninitialized(source.buckets(), Fallibility::Infallible)
1125-
.unwrap_or_else(|_| hint::unreachable_unchecked()),
1150+
// Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
1151+
match Self::new_uninitialized(source.buckets(), Fallibility::Infallible) {
1152+
Ok(table) => table,
1153+
Err(_) => hint::unreachable_unchecked(),
1154+
}
11261155
);
11271156
}
11281157

‎src/set.rs

+20-4
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,11 @@ where
687687
T: Borrow<Q>,
688688
Q: Hash + Eq,
689689
{
690-
self.map.get_key_value(value).map(|(k, _)| k)
690+
// Avoid `Option::map` because it bloats LLVM IR.
691+
match self.map.get_key_value(value) {
692+
Some((k, _)) => Some(k),
693+
None => None,
694+
}
691695
}
692696

693697
/// Inserts the given `value` into the set if it is not present, then
@@ -951,7 +955,11 @@ where
951955
T: Borrow<Q>,
952956
Q: Hash + Eq,
953957
{
954-
self.map.remove_entry(value).map(|(k, _)| k)
958+
// Avoid `Option::map` because it bloats LLVM IR.
959+
match self.map.remove_entry(value) {
960+
Some((k, _)) => Some(k),
961+
None => None,
962+
}
955963
}
956964
}
957965

@@ -1365,7 +1373,11 @@ impl<K> Iterator for IntoIter<K> {
13651373

13661374
#[cfg_attr(feature = "inline-more", inline)]
13671375
fn next(&mut self) -> Option<K> {
1368-
self.iter.next().map(|(k, _)| k)
1376+
// Avoid `Option::map` because it bloats LLVM IR.
1377+
match self.iter.next() {
1378+
Some((k, _)) => Some(k),
1379+
None => None,
1380+
}
13691381
}
13701382
#[cfg_attr(feature = "inline-more", inline)]
13711383
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -1392,7 +1404,11 @@ impl<K> Iterator for Drain<'_, K> {
13921404

13931405
#[cfg_attr(feature = "inline-more", inline)]
13941406
fn next(&mut self) -> Option<K> {
1395-
self.iter.next().map(|(k, _)| k)
1407+
// Avoid `Option::map` because it bloats LLVM IR.
1408+
match self.iter.next() {
1409+
Some((k, _)) => Some(k),
1410+
None => None,
1411+
}
13961412
}
13971413
#[cfg_attr(feature = "inline-more", inline)]
13981414
fn size_hint(&self) -> (usize, Option<usize>) {

0 commit comments

Comments
 (0)