Auto merge of #183 - nnethercote:avoid-closures, r=Amanieu

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.

Author: bors

src/map.rs

@@ -802,7 +802,11 @@ where
         K: Borrow<Q>,
         Q: Hash + Eq,
     {
-        self.get_key_value(k).map(|(_, v)| v)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.get_key_value(k) {
+            Some((_, v)) => Some(v),
+            None => None,
+        }
     }
 
     /// Returns the key-value pair corresponding to the supplied key.
@@ -831,12 +835,14 @@ where
         Q: Hash + Eq,
     {
         let hash = make_hash(&self.hash_builder, k);
-        self.table
-            .find(hash, |x| k.eq(x.0.borrow()))
-            .map(|item| unsafe {
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.table.find(hash, |x| k.eq(x.0.borrow())) {
+            Some(item) => unsafe {
                 let &(ref key, ref value) = item.as_ref();
-                (key, value)
-            })
+                Some((key, value))
+            }
+            None => None,
+        }
     }
 
     /// Returns the key-value pair corresponding to the supplied key, with a mutable reference to value.
@@ -869,12 +875,14 @@ where
         Q: Hash + Eq,
     {
         let hash = make_hash(&self.hash_builder, k);
-        self.table
-            .find(hash, |x| k.eq(x.0.borrow()))
-            .map(|item| unsafe {
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.table.find(hash, |x| k.eq(x.0.borrow())) {
+            Some(item) => unsafe {
                 let &mut (ref key, ref mut value) = item.as_mut();
-                (key, value)
-            })
+                Some((key, value))
+            }
+            None => None,
+        }
     }
 
     /// Returns `true` if the map contains a value for the specified key.
@@ -933,9 +941,11 @@ where
         Q: Hash + Eq,
     {
         let hash = make_hash(&self.hash_builder, k);
-        self.table
-            .find(hash, |x| k.eq(x.0.borrow()))
-            .map(|item| unsafe { &mut item.as_mut().1 })
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.table.find(hash, |x| k.eq(x.0.borrow())) {
+            Some(item) => Some(unsafe { &mut item.as_mut().1 }),
+            None => None,
+        }
     }
 
     /// Inserts a key-value pair into the map.
@@ -1004,7 +1014,11 @@ where
         K: Borrow<Q>,
         Q: Hash + Eq,
     {
-        self.remove_entry(k).map(|(_, v)| v)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.remove_entry(k) {
+            Some((_, v)) => Some(v),
+            None => None,
+        }
     }
 
     /// 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> {
     where
         F: FnMut(&K) -> bool,
     {
-        self.map
-            .table
-            .find(hash, |(k, _)| is_match(k))
-            .map(|item| unsafe {
+        match self.map.table.find(hash, |(k, _)| is_match(k)) {
+            Some(item) => unsafe {
                 let &(ref key, ref value) = item.as_ref();
-                (key, value)
-            })
+                Some((key, value))
+            }
+            None => None,
+        }
     }
 
     /// Access an entry by hash.
@@ -2030,10 +2044,14 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<(&'a K, &'a V)> {
-        self.inner.next().map(|x| unsafe {
-            let r = x.as_ref();
-            (&r.0, &r.1)
-        })
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.inner.next() {
+            Some(x) => unsafe {
+                let r = x.as_ref();
+                Some((&r.0, &r.1))
+            }
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2054,10 +2072,14 @@ impl<'a, K, V> Iterator for IterMut<'a, K, V> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<(&'a K, &'a mut V)> {
-        self.inner.next().map(|x| unsafe {
-            let r = x.as_mut();
-            (&r.0, &mut r.1)
-        })
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.inner.next() {
+            Some(x) => unsafe {
+                let r = x.as_mut();
+                Some((&r.0, &mut r.1))
+            }
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2113,7 +2135,11 @@ impl<'a, K, V> Iterator for Keys<'a, K, V> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<&'a K> {
-        self.inner.next().map(|(k, _)| k)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.inner.next() {
+            Some((k, _)) => Some(k),
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2133,7 +2159,11 @@ impl<'a, K, V> Iterator for Values<'a, K, V> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<&'a V> {
-        self.inner.next().map(|(_, v)| v)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.inner.next() {
+            Some((_, v)) => Some(v),
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -2153,7 +2183,11 @@ impl<'a, K, V> Iterator for ValuesMut<'a, K, V> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<&'a mut V> {
-        self.inner.next().map(|(_, v)| v)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.inner.next() {
+            Some((_, v)) => Some(v),
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {

src/raw/mod.rs

@@ -402,9 +402,16 @@ impl<T> RawTable<T> {
         fallability: Fallibility,
     ) -> Result<Self, TryReserveError> {
         debug_assert!(buckets.is_power_of_two());
-        let (layout, ctrl_offset) =
-            calculate_layout::<T>(buckets).ok_or_else(|| fallability.capacity_overflow())?;
-        let ptr = NonNull::new(alloc(layout)).ok_or_else(|| fallability.alloc_err(layout))?;
+
+        // Avoid `Option::ok_or_else` because it bloats LLVM IR.
+        let (layout, ctrl_offset) = match calculate_layout::<T>(buckets) {
+            Some(lco) => lco,
+            None => return Err(fallability.capacity_overflow())
+        };
+        let ptr = match NonNull::new(alloc(layout)) {
+            Some(ptr) => ptr,
+            None => return Err(fallability.alloc_err(layout)),
+        };
         let ctrl = NonNull::new_unchecked(ptr.as_ptr().add(ctrl_offset));
         Ok(Self {
             ctrl,
@@ -425,8 +432,11 @@ impl<T> RawTable<T> {
             Ok(Self::new())
         } else {
             unsafe {
-                let buckets =
-                    capacity_to_buckets(capacity).ok_or_else(|| fallability.capacity_overflow())?;
+                // Avoid `Option::ok_or_else` because it bloats LLVM IR.
+                let buckets = match capacity_to_buckets(capacity) {
+                    Some(buckets) => buckets,
+                    None => return Err(fallability.capacity_overflow()),
+                };
                 let result = Self::new_uninitialized(buckets, fallability)?;
                 result.ctrl(0).write_bytes(EMPTY, result.num_ctrl_bytes());
 
@@ -445,15 +455,21 @@ impl<T> RawTable<T> {
     /// Allocates a new hash table with at least enough capacity for inserting
     /// the given number of elements without reallocating.
     pub fn with_capacity(capacity: usize) -> Self {
-        Self::fallible_with_capacity(capacity, Fallibility::Infallible)
-            .unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() })
+        // Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
+        match Self::fallible_with_capacity(capacity, Fallibility::Infallible) {
+            Ok(capacity) => capacity,
+            Err(_) => unsafe { hint::unreachable_unchecked() },
+        }
     }
 
     /// Deallocates the table without dropping any entries.
     #[cfg_attr(feature = "inline-more", inline)]
     unsafe fn free_buckets(&mut self) {
-        let (layout, ctrl_offset) =
-            calculate_layout::<T>(self.buckets()).unwrap_or_else(|| hint::unreachable_unchecked());
+        // Avoid `Option::unwrap_or_else` because it bloats LLVM IR.
+        let (layout, ctrl_offset) = match calculate_layout::<T>(self.buckets()) {
+            Some(lco) => lco,
+            None => hint::unreachable_unchecked(),
+        };
         dealloc(self.ctrl.as_ptr().sub(ctrl_offset), layout);
     }
 
@@ -671,8 +687,10 @@ impl<T> RawTable<T> {
             if self.items == 0 {
                 *self = Self::with_capacity(min_size)
             } else {
-                self.resize(min_size, hasher, Fallibility::Infallible)
-                    .unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() });
+                // Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
+                if self.resize(min_size, hasher, Fallibility::Infallible).is_err() {
+                    unsafe { hint::unreachable_unchecked() }
+                }
             }
         }
     }
@@ -682,8 +700,10 @@ impl<T> RawTable<T> {
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn reserve(&mut self, additional: usize, hasher: impl Fn(&T) -> u64) {
         if additional > self.growth_left {
-            self.reserve_rehash(additional, hasher, Fallibility::Infallible)
-                .unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() });
+            // Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
+            if self.reserve_rehash(additional, hasher, Fallibility::Infallible).is_err() {
+                unsafe { hint::unreachable_unchecked() }
+            }
         }
     }
 
@@ -711,11 +731,11 @@ impl<T> RawTable<T> {
         hasher: impl Fn(&T) -> u64,
         fallability: Fallibility,
     ) -> Result<(), TryReserveError> {
-        let new_items = self
-            .items
-            .checked_add(additional)
-            .ok_or_else(|| fallability.capacity_overflow())?;
-
+        // Avoid `Option::ok_or_else` because it bloats LLVM IR.
+        let new_items = match self.items.checked_add(additional) {
+            Some(new_items) => new_items,
+            None => return Err(fallability.capacity_overflow()),
+        };
         let full_capacity = bucket_mask_to_capacity(self.bucket_mask);
         if new_items <= full_capacity / 2 {
             // Rehash in-place without re-allocating if we have plenty of spare
@@ -1065,8 +1085,11 @@ impl<T> RawTable<T> {
         let alloc = if self.is_empty_singleton() {
             None
         } else {
-            let (layout, ctrl_offset) = calculate_layout::<T>(self.buckets())
-                .unwrap_or_else(|| unsafe { hint::unreachable_unchecked() });
+            // Avoid `Option::unwrap_or_else` because it bloats LLVM IR.
+            let (layout, ctrl_offset) = match calculate_layout::<T>(self.buckets()) {
+                Some(lco) => lco,
+                None => unsafe { hint::unreachable_unchecked() },
+            };
             Some((
                 unsafe { NonNull::new_unchecked(self.ctrl.as_ptr().sub(ctrl_offset)) },
                 layout,
@@ -1087,8 +1110,11 @@ impl<T: Clone> Clone for RawTable<T> {
         } else {
             unsafe {
                 let mut new_table = ManuallyDrop::new(
-                    Self::new_uninitialized(self.buckets(), Fallibility::Infallible)
-                        .unwrap_or_else(|_| hint::unreachable_unchecked()),
+                    // Avoid `Result::ok_or_else` because it bloats LLVM IR.
+                    match Self::new_uninitialized(self.buckets(), Fallibility::Infallible) {
+                        Ok(table) => table,
+                        Err(_) => hint::unreachable_unchecked(),
+                    }
                 );
 
                 new_table.clone_from_spec(self, |new_table| {
@@ -1121,8 +1147,11 @@ impl<T: Clone> Clone for RawTable<T> {
                         self.free_buckets();
                     }
                     (self as *mut Self).write(
-                        Self::new_uninitialized(source.buckets(), Fallibility::Infallible)
-                            .unwrap_or_else(|_| hint::unreachable_unchecked()),
+                        // Avoid `Result::unwrap_or_else` because it bloats LLVM IR.
+                        match Self::new_uninitialized(source.buckets(), Fallibility::Infallible) {
+                            Ok(table) => table,
+                            Err(_) => hint::unreachable_unchecked(),
+                        }
                     );
                 }
 

src/set.rs

@@ -687,7 +687,11 @@ where
         T: Borrow<Q>,
         Q: Hash + Eq,
     {
-        self.map.get_key_value(value).map(|(k, _)| k)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.map.get_key_value(value) {
+            Some((k, _)) => Some(k),
+            None => None,
+        }
     }
 
     /// Inserts the given `value` into the set if it is not present, then
@@ -951,7 +955,11 @@ where
         T: Borrow<Q>,
         Q: Hash + Eq,
     {
-        self.map.remove_entry(value).map(|(k, _)| k)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.map.remove_entry(value) {
+            Some((k, _)) => Some(k),
+            None => None,
+        }
     }
 }
 
@@ -1365,7 +1373,11 @@ impl<K> Iterator for IntoIter<K> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<K> {
-        self.iter.next().map(|(k, _)| k)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.iter.next() {
+            Some((k, _)) => Some(k),
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -1392,7 +1404,11 @@ impl<K> Iterator for Drain<'_, K> {
 
     #[cfg_attr(feature = "inline-more", inline)]
     fn next(&mut self) -> Option<K> {
-        self.iter.next().map(|(k, _)| k)
+        // Avoid `Option::map` because it bloats LLVM IR.
+        match self.iter.next() {
+            Some((k, _)) => Some(k),
+            None => None,
+        }
     }
     #[cfg_attr(feature = "inline-more", inline)]
     fn size_hint(&self) -> (usize, Option<usize>) {