Auto merge of #433 - Amanieu:internal_cleanups, r=Amanieu

Clean up some internals

Author: bors

Cargo.toml

@@ -36,7 +36,7 @@ rayon = "1.0"
 fnv = "1.0.7"
 serde_test = "1.0"
 doc-comment = "0.3.1"
-bumpalo = "3.6.0"
+bumpalo = { version = "3.13.0", features = ["allocator-api2"] }
 rkyv = { version = "0.7.42", features = ["validation"] }
 
 [features]

src/map.rs

@@ -260,29 +260,6 @@ where
     hash_builder.hash_one(val)
 }
 
-#[cfg(not(feature = "nightly"))]
-#[cfg_attr(feature = "inline-more", inline)]
-pub(crate) fn make_insert_hash<K, S>(hash_builder: &S, val: &K) -> u64
-where
-    K: Hash,
-    S: BuildHasher,
-{
-    use core::hash::Hasher;
-    let mut state = hash_builder.build_hasher();
-    val.hash(&mut state);
-    state.finish()
-}
-
-#[cfg(feature = "nightly")]
-#[cfg_attr(feature = "inline-more", inline)]
-pub(crate) fn make_insert_hash<K, S>(hash_builder: &S, val: &K) -> u64
-where
-    K: Hash,
-    S: BuildHasher,
-{
-    hash_builder.hash_one(val)
-}
-
 #[cfg(feature = "ahash")]
 impl<K, V> HashMap<K, V, DefaultHashBuilder> {
     /// Creates an empty `HashMap`.
@@ -368,8 +345,6 @@ impl<K, V, A: Allocator + Clone> HashMap<K, V, DefaultHashBuilder, A> {
     /// # Examples
     ///
     /// ```
-    /// # #[cfg(feature = "nightly")]
-    /// # fn test() {
     /// use hashbrown::HashMap;
     /// use bumpalo::Bump;
     ///
@@ -388,11 +363,6 @@ impl<K, V, A: Allocator + Clone> HashMap<K, V, DefaultHashBuilder, A> {
     /// assert_eq!(map.len(), 1);
     /// // And it also allocates some capacity
     /// assert!(map.capacity() > 1);
-    /// # }
-    /// # fn main() {
-    /// #     #[cfg(feature = "nightly")]
-    /// #     test()
-    /// # }
     /// ```
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn new_in(alloc: A) -> Self {
@@ -419,8 +389,6 @@ impl<K, V, A: Allocator + Clone> HashMap<K, V, DefaultHashBuilder, A> {
     /// # Examples
     ///
     /// ```
-    /// # #[cfg(feature = "nightly")]
-    /// # fn test() {
     /// use hashbrown::HashMap;
     /// use bumpalo::Bump;
     ///
@@ -444,11 +412,6 @@ impl<K, V, A: Allocator + Clone> HashMap<K, V, DefaultHashBuilder, A> {
     /// assert_eq!(map.len(), 5);
     /// // But its capacity isn't changed
     /// assert_eq!(map.capacity(), empty_map_capacity)
-    /// # }
-    /// # fn main() {
-    /// #     #[cfg(feature = "nightly")]
-    /// #     test()
-    /// # }
     /// ```
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn with_capacity_in(capacity: usize, alloc: A) -> Self {
@@ -1262,7 +1225,7 @@ where
     /// ```
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn entry(&mut self, key: K) -> Entry<'_, K, V, S, A> {
-        let hash = make_insert_hash::<K, S>(&self.hash_builder, &key);
+        let hash = make_hash::<K, S>(&self.hash_builder, &key);
         if let Some(elem) = self.table.find(hash, equivalent_key(&key)) {
             Entry::Occupied(OccupiedEntry {
                 hash,
@@ -1782,7 +1745,7 @@ where
     /// ```
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn insert(&mut self, k: K, v: V) -> Option<V> {
-        let hash = make_insert_hash::<K, S>(&self.hash_builder, &k);
+        let hash = make_hash::<K, S>(&self.hash_builder, &k);
         let hasher = make_hasher::<_, V, S>(&self.hash_builder);
         match self
             .table
@@ -1849,7 +1812,7 @@ where
     /// ```
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn insert_unique_unchecked(&mut self, k: K, v: V) -> (&K, &mut V) {
-        let hash = make_insert_hash::<K, S>(&self.hash_builder, &k);
+        let hash = make_hash::<K, S>(&self.hash_builder, &k);
         let bucket = self
             .table
             .insert(hash, (k, v), make_hasher::<_, V, S>(&self.hash_builder));
@@ -4003,7 +3966,7 @@ impl<'a, K, V, S, A: Allocator + Clone> RawVacantEntryMut<'a, K, V, S, A> {
         K: Hash,
         S: BuildHasher,
     {
-        let hash = make_insert_hash::<K, S>(self.hash_builder, &key);
+        let hash = make_hash::<K, S>(self.hash_builder, &key);
         self.insert_hashed_nocheck(hash, key, value)
     }
 
@@ -4111,7 +4074,7 @@ impl<'a, K, V, S, A: Allocator + Clone> RawVacantEntryMut<'a, K, V, S, A> {
         K: Hash,
         S: BuildHasher,
     {
-        let hash = make_insert_hash::<K, S>(self.hash_builder, &key);
+        let hash = make_hash::<K, S>(self.hash_builder, &key);
         let elem = self.table.insert(
             hash,
             (key, value),
@@ -8194,7 +8157,7 @@ mod test_map {
         let mut map: HashMap<_, _> = xs.iter().copied().collect();
 
         let compute_hash = |map: &HashMap<i32, i32>, k: i32| -> u64 {
-            super::make_insert_hash::<i32, _>(map.hasher(), &k)
+            super::make_hash::<i32, _>(map.hasher(), &k)
         };
 
         // Existing key (insert)
@@ -8356,7 +8319,7 @@ mod test_map {
             loop {
                 // occasionally remove some elements
                 if i < n && rng.gen_bool(0.1) {
-                    let hash_value = super::make_insert_hash(&hash_builder, &i);
+                    let hash_value = super::make_hash(&hash_builder, &i);
 
                     unsafe {
                         let e = map.table.find(hash_value, |q| q.0.eq(&i));

src/raw/bitmask.rs

@@ -1,6 +1,6 @@
-use super::imp::{BitMaskWord, BITMASK_ITER_MASK, BITMASK_MASK, BITMASK_STRIDE};
-#[cfg(feature = "nightly")]
-use core::intrinsics;
+use super::imp::{
+    BitMaskWord, NonZeroBitMaskWord, BITMASK_ITER_MASK, BITMASK_MASK, BITMASK_STRIDE,
+};
 
 /// A bit mask which contains the result of a `Match` operation on a `Group` and
 /// allows iterating through them.
@@ -47,26 +47,13 @@ impl BitMask {
     /// Returns the first set bit in the `BitMask`, if there is one.
     #[inline]
     pub(crate) fn lowest_set_bit(self) -> Option<usize> {
-        if self.0 == 0 {
-            None
+        if let Some(nonzero) = NonZeroBitMaskWord::new(self.0) {
+            Some(Self::nonzero_trailing_zeros(nonzero))
         } else {
-            Some(unsafe { self.lowest_set_bit_nonzero() })
+            None
         }
     }
 
-    /// Returns the first set bit in the `BitMask`, if there is one. The
-    /// bitmask must not be empty.
-    #[inline]
-    #[cfg(feature = "nightly")]
-    pub(crate) unsafe fn lowest_set_bit_nonzero(self) -> usize {
-        intrinsics::cttz_nonzero(self.0) as usize / BITMASK_STRIDE
-    }
-    #[inline]
-    #[cfg(not(feature = "nightly"))]
-    pub(crate) unsafe fn lowest_set_bit_nonzero(self) -> usize {
-        self.trailing_zeros()
-    }
-
     /// Returns the number of trailing zeroes in the `BitMask`.
     #[inline]
     pub(crate) fn trailing_zeros(self) -> usize {
@@ -82,6 +69,18 @@ impl BitMask {
         }
     }
 
+    /// Same as above but takes a `NonZeroBitMaskWord`.
+    #[inline]
+    fn nonzero_trailing_zeros(nonzero: NonZeroBitMaskWord) -> usize {
+        if cfg!(target_arch = "arm") && BITMASK_STRIDE % 8 == 0 {
+            // SAFETY: A byte-swapped non-zero value is still non-zero.
+            let swapped = unsafe { NonZeroBitMaskWord::new_unchecked(nonzero.get().swap_bytes()) };
+            swapped.leading_zeros() as usize / BITMASK_STRIDE
+        } else {
+            nonzero.trailing_zeros() as usize / BITMASK_STRIDE
+        }
+    }
+
     /// Returns the number of leading zeroes in the `BitMask`.
     #[inline]
     pub(crate) fn leading_zeros(self) -> usize {

src/raw/generic.rs

@@ -5,22 +5,24 @@ use core::{mem, ptr};
 // Use the native word size as the group size. Using a 64-bit group size on
 // a 32-bit architecture will just end up being more expensive because
 // shifts and multiplies will need to be emulated.
-#[cfg(any(
-    target_pointer_width = "64",
-    target_arch = "aarch64",
-    target_arch = "x86_64",
-    target_arch = "wasm32",
-))]
-type GroupWord = u64;
-#[cfg(all(
-    any(target_pointer_width = "32", target_pointer_width = "16"),
-    not(target_arch = "aarch64"),
-    not(target_arch = "x86_64"),
-    not(target_arch = "wasm32"),
-))]
-type GroupWord = u32;
+
+cfg_if! {
+    if #[cfg(any(
+        target_pointer_width = "64",
+        target_arch = "aarch64",
+        target_arch = "x86_64",
+        target_arch = "wasm32",
+    ))] {
+        type GroupWord = u64;
+        type NonZeroGroupWord = core::num::NonZeroU64;
+    } else {
+        type GroupWord = u32;
+        type NonZeroGroupWord = core::num::NonZeroU32;
+    }
+}
 
 pub(crate) type BitMaskWord = GroupWord;
+pub(crate) type NonZeroBitMaskWord = NonZeroGroupWord;
 pub(crate) const BITMASK_STRIDE: usize = 8;
 // We only care about the highest bit of each byte for the mask.
 #[allow(clippy::cast_possible_truncation, clippy::unnecessary_cast)]

src/raw/mod.rs

@@ -44,32 +44,12 @@ use self::imp::Group;
 
 // Branch prediction hint. This is currently only available on nightly but it
 // consistently improves performance by 10-15%.
-#[cfg(feature = "nightly")]
-use core::intrinsics::{likely, unlikely};
-
-// On stable we can use #[cold] to get a equivalent effect: this attributes
-// suggests that the function is unlikely to be called
 #[cfg(not(feature = "nightly"))]
-#[inline]
-#[cold]
-fn cold() {}
-
-#[cfg(not(feature = "nightly"))]
-#[inline]
-fn likely(b: bool) -> bool {
-    if !b {
-        cold();
-    }
-    b
-}
+use core::convert::identity as likely;
 #[cfg(not(feature = "nightly"))]
-#[inline]
-fn unlikely(b: bool) -> bool {
-    if b {
-        cold();
-    }
-    b
-}
+use core::convert::identity as unlikely;
+#[cfg(feature = "nightly")]
+use core::intrinsics::{likely, unlikely};
 
 // Use strict provenance functions if available.
 #[cfg(feature = "nightly")]
@@ -1668,14 +1648,14 @@ impl<A: Allocator + Clone> RawTableInner<A> {
             //   we will never end up in the given branch, since
             //   `(probe_seq.pos + bit) & self.bucket_mask` in `find_insert_slot_in_group` cannot
             //   return a full bucket index. For tables smaller than the group width, calling the
-            //   `lowest_set_bit_nonzero` function (when `nightly` feature enabled) is also
+            //   `unwrap_unchecked` function is also
             //   safe, as the trailing control bytes outside the range of the table are filled
             //   with EMPTY bytes, so this second scan either finds an empty slot (due to the
-            //   load factor) or hits the trailing control bytes (containing EMPTY). See
-            //   `intrinsics::cttz_nonzero` for more information.
+            //   load factor) or hits the trailing control bytes (containing EMPTY).
             index = Group::load_aligned(self.ctrl(0))
                 .match_empty_or_deleted()
-                .lowest_set_bit_nonzero();
+                .lowest_set_bit()
+                .unwrap_unchecked();
         }
         InsertSlot { index }
     }

src/raw/neon.rs

@@ -2,8 +2,10 @@ use super::bitmask::BitMask;
 use super::EMPTY;
 use core::arch::aarch64 as neon;
 use core::mem;
+use core::num::NonZeroU64;
 
 pub(crate) type BitMaskWord = u64;
+pub(crate) type NonZeroBitMaskWord = NonZeroU64;
 pub(crate) const BITMASK_STRIDE: usize = 8;
 pub(crate) const BITMASK_MASK: BitMaskWord = !0;
 pub(crate) const BITMASK_ITER_MASK: BitMaskWord = 0x8080_8080_8080_8080;

src/raw/sse2.rs

@@ -1,13 +1,15 @@
 use super::bitmask::BitMask;
 use super::EMPTY;
 use core::mem;
+use core::num::NonZeroU16;
 
 #[cfg(target_arch = "x86")]
 use core::arch::x86;
 #[cfg(target_arch = "x86_64")]
 use core::arch::x86_64 as x86;
 
 pub(crate) type BitMaskWord = u16;
+pub(crate) type NonZeroBitMaskWord = NonZeroU16;
 pub(crate) const BITMASK_STRIDE: usize = 1;
 pub(crate) const BITMASK_MASK: BitMaskWord = 0xffff;
 pub(crate) const BITMASK_ITER_MASK: BitMaskWord = !0;

src/rustc_entry.rs

@@ -1,5 +1,5 @@
 use self::RustcEntry::*;
-use crate::map::{make_insert_hash, Drain, HashMap, IntoIter, Iter, IterMut};
+use crate::map::{make_hash, Drain, HashMap, IntoIter, Iter, IterMut};
 use crate::raw::{Allocator, Bucket, Global, RawTable};
 use core::fmt::{self, Debug};
 use core::hash::{BuildHasher, Hash};
@@ -32,7 +32,7 @@ where
     /// ```
     #[cfg_attr(feature = "inline-more", inline)]
     pub fn rustc_entry(&mut self, key: K) -> RustcEntry<'_, K, V, A> {
-        let hash = make_insert_hash(&self.hash_builder, &key);
+        let hash = make_hash(&self.hash_builder, &key);
         if let Some(elem) = self.table.find(hash, |q| q.0.eq(&key)) {
             RustcEntry::Occupied(RustcOccupiedEntry {
                 key: Some(key),