perf(hstr): trim atom hash and benchmark hashset lookups

crates/hstr/Cargo.toml

@@ -28,16 +28,16 @@ triomphe              = { workspace = true }
 
 
 [dev-dependencies]
-compact_str  = { workspace = true }
-criterion    = { workspace = true }
-kstring      = { workspace = true }
-num_cpus     = { workspace = true }
-par-iter     = { workspace = true }
-rand         = { workspace = true }
-serde_json   = { workspace = true }
-smartstring  = { workspace = true }
-smol_str     = { workspace = true }
-string_cache = { workspace = true }
+codspeed-criterion-compat = { workspace = true }
+compact_str               = { workspace = true }
+kstring                   = { workspace = true }
+num_cpus                  = { workspace = true }
+par-iter                  = { workspace = true }
+rand                      = { workspace = true }
+serde_json                = { workspace = true }
+smartstring               = { workspace = true }
+smol_str                  = { workspace = true }
+string_cache              = { workspace = true }
 
 swc_malloc = { version = "1.2.5", path = "../swc_malloc" }
 

crates/hstr/benches/libs.rs

@@ -2,12 +2,12 @@
 
 extern crate swc_malloc;
 
-#[macro_use]
-extern crate criterion;
 use std::{hash::Hash, mem::forget};
 
+use codspeed_criterion_compat::{
+    black_box, criterion_group, criterion_main, BatchSize, BenchmarkId, Criterion,
+};
 use compact_str::CompactString;
-use criterion::{black_box, BatchSize, BenchmarkId, Criterion};
 use par_iter::prelude::*;
 use rand::distributions::{Alphanumeric, DistString};
 use rustc_hash::FxHashSet;
@@ -175,73 +175,67 @@ fn bench_hash_operation(c: &mut Criterion) {
     {
         for len in length {
             group.bench_with_input(BenchmarkId::new("hstr", len), &len, |b, _| {
-                let mut for_fairness = vec![];
                 let mut store = hstr::AtomStore::default();
-                b.iter_batched(
+                b.iter_batched_ref(
                     || prepare(len, &mut |s| store.atom(s)),
                     |(map, keys)| {
-                        for key in &keys {
+                        for key in keys.iter() {
                             black_box(map.contains(key));
                         }
-                        for_fairness.extend(map);
-                        for_fairness.extend(keys);
                     },
                     BatchSize::SmallInput,
                 );
             });
             group.bench_with_input(BenchmarkId::new("string_cache", len), &len, |b, _| {
-                let mut for_fairness = vec![];
-                b.iter_batched(
+                b.iter_batched_ref(
                     || prepare(len, &mut string_cache::DefaultAtom::from),
                     |(map, keys)| {
-                        for key in &keys {
+                        for key in keys.iter() {
                             black_box(map.contains(key));
                         }
-                        for_fairness.extend(map);
-                        for_fairness.extend(keys);
                     },
                     BatchSize::SmallInput,
                 );
             });
             group.bench_with_input(BenchmarkId::new("compact_str", len), &len, |b, _| {
-                b.iter_batched(
+                b.iter_batched_ref(
                     || prepare(len, &mut CompactString::from),
                     |(map, keys)| {
-                        for key in keys {
-                            black_box(map.contains(&key));
+                        for key in keys.iter() {
+                            black_box(map.contains(key));
                         }
                     },
                     BatchSize::SmallInput,
                 );
             });
             group.bench_with_input(BenchmarkId::new("smartstring", len), &len, |b, _| {
-                b.iter_batched(
+                b.iter_batched_ref(
                     || prepare(len, &mut SmartString::<LazyCompact>::from),
                     |(map, keys)| {
-                        for key in keys {
-                            black_box(map.contains(&key));
+                        for key in keys.iter() {
+                            black_box(map.contains(key));
                         }
                     },
                     BatchSize::SmallInput,
                 );
             });
             group.bench_with_input(BenchmarkId::new("smol_str", len), &len, |b, _| {
-                b.iter_batched(
+                b.iter_batched_ref(
                     || prepare(len, &mut smol_str::SmolStr::from),
                     |(map, keys)| {
-                        for key in keys {
-                            black_box(map.contains(&key));
+                        for key in keys.iter() {
+                            black_box(map.contains(key));
                         }
                     },
                     BatchSize::SmallInput,
                 );
             });
             group.bench_with_input(BenchmarkId::new("kstring", len), &len, |b, _| {
-                b.iter_batched(
+                b.iter_batched_ref(
                     || prepare(len, &mut kstring::KString::from),
                     |(map, keys)| {
-                        for key in keys {
-                            black_box(map.contains(&key));
+                        for key in keys.iter() {
+                            black_box(map.contains(key));
                         }
                     },
                     BatchSize::SmallInput,

crates/hstr/src/lib.rs

@@ -297,13 +297,9 @@ impl Atom {
 }
 
 impl PartialEq for Atom {
-    #[inline(never)]
+    #[inline(always)]
     fn eq(&self, other: &Self) -> bool {
         partial_eq!(self, other);
-
-        // If the store is different, the string may be the same, even though the
-        // `unsafe_data` is different
-        self.as_str() == other.as_str()
     }
 }
 

crates/hstr/src/macros.rs

@@ -20,19 +20,25 @@ macro_rules! get_hash {
 
 macro_rules! partial_eq {
     ($self:expr, $other:expr) => {
-        if $self.unsafe_data == $other.unsafe_data {
+        let self_data = $self.unsafe_data;
+        let other_data = $other.unsafe_data;
+
+        if self_data == other_data {
             return true;
         }
 
+        let self_tag = self_data.tag() & TAG_MASK;
+        let other_tag = other_data.tag() & TAG_MASK;
+
         // If one is inline and the other is not, the length is different.
         // If one is static and the other is not, it's different.
-        if $self.tag() != $other.tag() {
+        if self_tag != other_tag {
             return false;
         }
 
-        if $self.is_dynamic() && $other.is_dynamic() {
-            let te = unsafe { $crate::dynamic::deref_from($self.unsafe_data) };
-            let oe = unsafe { $crate::dynamic::deref_from($other.unsafe_data) };
+        if self_tag == DYNAMIC_TAG {
+            let te = unsafe { $crate::dynamic::deref_from(self_data) };
+            let oe = unsafe { $crate::dynamic::deref_from(other_data) };
 
             if te.header.header.hash != oe.header.header.hash {
                 return false;
@@ -41,9 +47,8 @@ macro_rules! partial_eq {
             return te.slice == oe.slice;
         }
 
-        if $self.get_hash() != $other.get_hash() {
-            return false;
-        }
+        // Inline atoms compare equal only if their raw tagged value matched above.
+        return false;
     };
 }
 

crates/hstr/src/tagged_value.rs

@@ -36,12 +36,21 @@ type RawTaggedNonZeroValue = std::ptr::NonNull<()>;
 
 pub(crate) const MAX_INLINE_LEN: usize = std::mem::size_of::<TaggedValue>() - 1;
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug)]
 #[repr(transparent)]
 pub(crate) struct TaggedValue {
     value: RawTaggedNonZeroValue,
 }
 
+impl PartialEq for TaggedValue {
+    #[inline(always)]
+    fn eq(&self, other: &Self) -> bool {
+        self.get_value() == other.get_value()
+    }
+}
+
+impl Eq for TaggedValue {}
+
 impl TaggedValue {
     #[inline(always)]
     pub fn new_ptr<T>(value: NonNull<T>) -> Self {
@@ -101,14 +110,32 @@ impl TaggedValue {
             feature = "atom_size_64",
             feature = "atom_size_128"
         )))]
-        unsafe {
-            std::mem::transmute(Some(self.value))
+        {
+            self.value.as_ptr() as _
         }
     }
 
     #[inline(always)]
     fn get_value(&self) -> RawTaggedValue {
-        unsafe { std::mem::transmute(Some(self.value)) }
+        #[cfg(any(
+            target_pointer_width = "32",
+            target_pointer_width = "16",
+            feature = "atom_size_64",
+            feature = "atom_size_128"
+        ))]
+        unsafe {
+            std::mem::transmute(Some(self.value))
+        }
+
+        #[cfg(not(any(
+            target_pointer_width = "32",
+            target_pointer_width = "16",
+            feature = "atom_size_64",
+            feature = "atom_size_128"
+        )))]
+        {
+            self.value.as_ptr() as _
+        }
     }
 
     #[inline(always)]

crates/hstr/src/wtf8_atom.rs

@@ -242,13 +242,9 @@ impl<'de> serde::de::Deserialize<'de> for Wtf8Atom {
 }
 
 impl PartialEq for Wtf8Atom {
-    #[inline(never)]
+    #[inline(always)]
     fn eq(&self, other: &Self) -> bool {
         partial_eq!(self, other);
-
-        // If the store is different, the string may be the same, even though the
-        // `unsafe_data` is different
-        self.as_wtf8() == other.as_wtf8()
     }
 }