Simplify opaque node pointer casting to concrete

**Description**
 - Remove dependency on `sptr` by adding shim functions in the
   nightly module as replacement
 - Remove some `#[inline(always)]` complete and change all other
   instances to `#[inline]`
 - Modify `count_words` example to use nul-terminated string
   internally
 - Remove the `TaggedPointer::<T>::cast` cast function in favour of
   casting directly on the resulting `NonNull<T>`

**Motivation**
 - The `sptr` crate hasn't been updated to include strict provenance
   methods for `NonNull`, see Gankra/sptr#17
 - `#[inline(always)]` is generally too strong of a requirement, I'd
   rather leave it to the compiler heuristic without strong evidence
   to the contrary.
 - The `count_words` example was failing on some inputs because of
   words that were prefixes of other words
 - The `cast` function was showing up in a hot part of the call stack
   and it seemed inefficient to carry the data tag over for the cast
   for my use-case

**Testing Done**
`./scripts/full-test.sh nightly`

See PR for benchmark results

.gitignore

@@ -16,3 +16,6 @@ profile.json
 
 # coverage info
 lcov.info
+
+# perf data
+perf.data*

Cargo.toml

@@ -28,7 +28,6 @@ autobenches = false
 [dependencies]
 bytemuck = { version = "1.16.1", features = ["min_const_generics"] }
 paste = "1.0.15"
-sptr = "0.3.2"
 
 [features]
 nightly = []

README.md

@@ -100,6 +100,22 @@ cargo +nightly fuzz coverage fuzz_tree_map_api && cargo cov -- show fuzz/target/
     > index.html
 ```
 
+```bash
+TARGET_TRIPLE="x86_64-unknown-linux-gnu"
+/home/declan/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-cov show -format=html \
+  -instr-profile=fuzz/coverage/fuzz_tree_map_api/coverage.profdata \
+  -Xdemangler=rustfilt \
+  -ignore-filename-regex=\.cargo/registry \
+  fuzz/target/x86_64-unknown-linux-gnu/release/fuzz_tree_map_api
+  > cov.html
+
+/home/declan/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-cov show \
+    --instr-profile=/home/declan/repos/github/declanvk/blart/fuzz/coverage/fuzz_tree_map_api/coverage.profdata \
+    --show-instantiations --show-line-counts-or-regions --Xdemangler=rustfilt \
+    --format=html --ignore-filename-regex=/home/declan/.cargo/registry --ignore-filename-regex=/home/declan/.rustup/\
+    target/x86_64-unknown-linux-gnu/coverage/x86_64-unknown-linux-gnu/release/fuzz_tree_map_api > coverage.html
+```
+
 ## Benchmarks
 
 To run the benchmarks, install [`cargo-criterion`][cargo-criterion], then run:
@@ -134,7 +150,7 @@ curl -o data/Ulysses.txt https://www.gutenberg.org/cache/epub/4300/pg4300.txt
 Then build the word count example using the `profiling` profile:
 
 ```bash
-cargo build --profile profiling --exampleps
+RUSTFLAGS="-C force-frame-pointers=yes" cargo build --profile profiling --examples
 ```
 
 Then run the count words workload on the downloaded data while profiling:

benches/iai_callgrind.rs

@@ -5,7 +5,9 @@ use crate::common::{
     with_prefixes_tree,
 };
 use blart::{AsBytes, TreeMap};
-use iai_callgrind::{library_benchmark, library_benchmark_group, main, LibraryBenchmarkConfig};
+use iai_callgrind::{
+    library_benchmark, library_benchmark_group, main, FlamegraphConfig, LibraryBenchmarkConfig,
+};
 
 #[macro_use]
 mod common;
@@ -177,6 +179,7 @@ library_benchmark_group!(name = bench_iterator_group; benchmarks = bench_full_it
 fn config() -> LibraryBenchmarkConfig {
     let mut c = LibraryBenchmarkConfig::default();
     c.truncate_description(Some(0));
+    c.flamegraph(FlamegraphConfig::default());
     c
 }
 

benches/tree/generated_get.rs

@@ -8,7 +8,6 @@ use blart::{
 };
 use criterion::{criterion_group, measurement::Measurement, BenchmarkGroup, Criterion};
 
-#[inline(always)]
 fn run_benchmarks<M: Measurement>(
     group: &mut BenchmarkGroup<M>,
     key_vec: &[Box<[u8]>],
@@ -38,7 +37,6 @@ fn run_benchmarks<M: Measurement>(
     //     - a tree node that is full and will need to grow
 }
 
-#[inline(always)]
 fn setup_tree_run_benches_cleanup(
     c: &mut Criterion,
     keys: impl Iterator<Item = Box<[u8]>>,
@@ -60,7 +58,6 @@ fn setup_tree_run_benches_cleanup(
     }
 }
 
-#[inline(always)]
 fn bench(c: &mut Criterion) {
     // number of keys = 256
     setup_tree_run_benches_cleanup(

benches/tree/generated_insert.rs

@@ -26,7 +26,6 @@ fn gen_group(c: &mut Criterion, group: &str, keys: Vec<Box<[u8]>>) {
     });
 }
 
-#[inline(always)]
 fn bench(c: &mut Criterion) {
     let skewed: Vec<_> = generate_keys_skewed(u8::MAX as usize).collect();
     let fixed_length: Vec<_> = generate_key_fixed_length([2; 8]).map(Box::from).collect();

examples/count_words.rs

@@ -1,6 +1,8 @@
 use argh::FromArgs;
 use blart::TreeMap;
-use std::{collections::BTreeMap, error::Error, fs::OpenOptions, io::Read, path::PathBuf};
+use std::{
+    collections::BTreeMap, error::Error, ffi::CString, fs::OpenOptions, io::Read, path::PathBuf,
+};
 
 /// Count words in file
 #[derive(FromArgs)]
@@ -50,23 +52,25 @@ fn main() -> Result<(), Box<dyn Error>> {
 
 #[derive(Debug)]
 #[allow(dead_code)] // this struct is used for its debug repr
-struct WordStats<'b> {
+struct WordStats {
     num_unique: u64,
-    first_word: &'b [u8],
+    first_word: CString,
     first_word_count: u64,
-    last_word: &'b [u8],
+    last_word: CString,
     last_word_count: u64,
 }
 
 fn count_words_blart(contents: &[u8]) -> WordStats {
-    let mut map = TreeMap::<&[u8], u64>::new();
+    let mut map = TreeMap::<CString, u64>::new();
 
     for word in contents.split_inclusive(|b| *b == SPLIT_BYTE) {
-        if let Some(count) = map.get_mut(word) {
-            *count += 1;
-        } else {
-            map.try_insert(word, 1).unwrap();
-        }
+        let word = CString::new(word).unwrap();
+
+        map.entry(word)
+            .and_modify(|count| {
+                *count += 1;
+            })
+            .or_insert(1);
     }
 
     let (first_word, first_word_count) = map
@@ -79,19 +83,21 @@ fn count_words_blart(contents: &[u8]) -> WordStats {
 
     WordStats {
         num_unique: map.len() as u64,
-        last_word,
+        last_word: last_word.clone(),
         last_word_count: *last_word_count,
-        first_word,
+        first_word: first_word.clone(),
         first_word_count: *first_word_count,
     }
 }
 
 const SPLIT_BYTE: u8 = b' ';
 
 fn count_words_std(contents: &[u8]) -> WordStats {
-    let mut map = BTreeMap::<&[u8], u64>::new();
+    let mut map = BTreeMap::<CString, u64>::new();
 
     for word in contents.split_inclusive(|b| *b == SPLIT_BYTE) {
+        let word = CString::new(word).unwrap();
+
         map.entry(word)
             .and_modify(|count| {
                 *count += 1;
@@ -109,9 +115,9 @@ fn count_words_std(contents: &[u8]) -> WordStats {
 
     WordStats {
         num_unique: map.len() as u64,
-        last_word,
+        last_word: last_word.clone(),
         last_word_count: *last_word_count,
-        first_word,
+        first_word: first_word.clone(),
         first_word_count: *first_word_count,
     }
 }

fuzz/fuzz_targets/fuzz_tree_map_api.rs

@@ -45,7 +45,7 @@ enum Action {
 
 libfuzzer_sys::fuzz_target!(|actions: Vec<Action>| {
     let mut tree = TreeMap::<_, u32>::new();
-    let mut next_key = 0;
+    let mut next_value = 0;
 
     for action in actions {
         match action {
@@ -58,25 +58,25 @@ libfuzzer_sys::fuzz_target!(|actions: Vec<Action>| {
             Action::GetMinimum => {
                 let min = tree.first_key_value();
                 if let Some((_, min_value)) = min {
-                    assert!(*min_value < next_key);
+                    assert!(*min_value < next_value);
                 }
             },
             Action::PopMinimum => {
                 let min = tree.pop_first();
                 if let Some((_, min_value)) = min {
-                    assert!(min_value < next_key);
+                    assert!(min_value < next_value);
                 }
             },
             Action::GetMaximum => {
                 let max = tree.last_key_value();
                 if let Some((_, max_value)) = max {
-                    assert!(*max_value < next_key);
+                    assert!(*max_value < next_value);
                 }
             },
             Action::PopMaximum => {
                 let max = tree.pop_last();
                 if let Some((_, max_value)) = max {
-                    assert!(max_value < next_key);
+                    assert!(max_value < next_value);
                 }
             },
             Action::GetKey(key) => {
@@ -104,19 +104,19 @@ libfuzzer_sys::fuzz_target!(|actions: Vec<Action>| {
             },
             Action::Remove(key) => {
                 if let Some(value) = tree.remove(key.as_ref()) {
-                    assert!(value < next_key);
+                    assert!(value < next_value);
                 }
             },
             Action::TryInsert(key) => {
-                let value = next_key;
-                next_key += 1;
+                let value = next_value;
+                next_value += 1;
 
                 let _ = tree.try_insert(key, value);
             },
             Action::Extend(new_keys) => {
                 for key in new_keys {
-                    let value = next_key;
-                    next_key += 1;
+                    let value = next_value;
+                    next_value += 1;
 
                     let _ = tree.try_insert(key, value);
                 }
@@ -137,8 +137,8 @@ libfuzzer_sys::fuzz_target!(|actions: Vec<Action>| {
             },
             Action::Entry(ea, key) => {
                 if let Ok(entry) = tree.try_entry(key) {
-                    let value = next_key;
-                    next_key += 1;
+                    let value = next_value;
+                    next_value += 1;
                     match ea {
                         EntryAction::AndModify => {
                             entry.and_modify(|v| *v = v.saturating_sub(1));
@@ -174,8 +174,8 @@ libfuzzer_sys::fuzz_target!(|actions: Vec<Action>| {
             },
             Action::EntryRef(ea, key) => {
                 if let Ok(entry) = tree.try_entry_ref(&key) {
-                    let value = next_key;
-                    next_key += 1;
+                    let value = next_value;
+                    next_value += 1;
                     match ea {
                         EntryAction::AndModify => {
                             entry.and_modify(|v| *v = v.saturating_sub(1));

scripts/bench-against.sh

@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+set -o errexit # make script exit when a command fails
+set -o nounset # make script exit when using undeclared variables
+set -o pipefail # make script exit when command fails in a pipe
+
+git switch --quiet --detach "$1"
+
+BASELINE_NAME="$(git rev-parse --short HEAD)"
+
+# Create the baseline benchmark and don't output the summary
+cargo bench --quiet --bench iai_callgrind -- --save-baseline="${BASELINE_NAME}" > /dev/null
+
+# Using '-' will switch back to the previous branch or git checkout
+git switch --quiet -
+
+# Run the benchmark again with comparison to baseline
+cargo bench --quiet --bench iai_callgrind -- --baseline="${BASELINE_NAME}"

src/collections/map/iterators/fuzzy.rs

@@ -91,7 +91,7 @@ impl StackArena {
 /// SAFETY: `old` and `new` must have the same length, and be >= 1
 ///
 /// SAFETY: `key` length + 1 == `new` or `old` length
-#[inline(always)]
+#[inline]
 fn edit_dist(
     key: &[u8],
     c: u8,
@@ -142,7 +142,7 @@ fn edit_dist(
 }
 
 /// SAFETY: `old_row` length == `new_row` length
-#[inline(always)]
+#[inline]
 unsafe fn swap(old_row: &mut &mut [usize], new_row: &mut &mut [MaybeUninit<usize>]) {
     // SAFETY: It's safe to transmute initialized data to uninitialized
     let temp = unsafe {
@@ -162,7 +162,7 @@ trait FuzzySearch<K: AsBytes, V, const PREFIX_LEN: usize> {
         max_edit_dist: usize,
     ) -> bool;
 
-    #[inline(always)]
+    #[inline]
     fn fuzzy_search_prefix(
         &self,
         key: &[u8],
@@ -355,7 +355,7 @@ macro_rules! gen_iter {
         {
             type Item = $ret;
 
-            #[inline(always)]
+            #[inline]
             fn next(&mut self) -> Option<Self::Item> {
                 let mut old_row = self.old_row.as_mut();
                 let mut new_row = self.new_row.as_mut();

src/nodes/operations/delete.rs

@@ -401,7 +401,7 @@ where
 ///  - This function cannot be called concurrently with any mutating operation
 ///    on `root` or any child node of `root`. This function will arbitrarily
 ///    read to any child in the given tree.
-#[inline(always)]
+#[inline]
 pub unsafe fn find_minimum_to_delete<K, V, const PREFIX_LEN: usize>(
     root: OpaqueNodePtr<K, V, PREFIX_LEN>,
 ) -> DeletePoint<K, V, PREFIX_LEN> {
@@ -439,7 +439,7 @@ pub unsafe fn find_minimum_to_delete<K, V, const PREFIX_LEN: usize>(
 ///  - This function cannot be called concurrently with any mutating operation
 ///    on `root` or any child node of `root`. This function will arbitrarily
 ///    read to any child in the given tree.
-#[inline(always)]
+#[inline]
 pub unsafe fn find_maximum_to_delete<K, V, const PREFIX_LEN: usize>(
     root: OpaqueNodePtr<K, V, PREFIX_LEN>,
 ) -> DeletePoint<K, V, PREFIX_LEN> {

src/nodes/operations/insert.rs

@@ -607,6 +607,7 @@ pub unsafe fn search_for_insert_point<K, V, const PREFIX_LEN: usize>(
 where
     K: AsBytes,
 {
+    #[inline]
     fn test_prefix_identify_insert<K, V, N, const PREFIX_LEN: usize>(
         inner_ptr: NodePtr<PREFIX_LEN, N>,
         key: &[u8],

src/nodes/operations/lookup.rs

@@ -59,6 +59,7 @@ impl PrefixMatchBehavior {
     /// This function will choose between different "match prefix" methods
     /// depending on the current behavior and attempt to match the prefix of the
     /// given inner node against the given key.
+    #[inline]
     pub fn match_prefix<K, V, const PREFIX_LEN: usize>(
         &mut self,
         inner_node: &impl InnerNode<PREFIX_LEN, Key = K, Value = V>,

src/nodes/operations/minmax.rs

@@ -6,7 +6,7 @@ use crate::{ConcreteNodePtr, InnerNode, LeafNode, NodePtr, OpaqueNodePtr};
 ///  - This function cannot be called concurrently with any mutating operation
 ///    on `root` or any child node of `root`. This function will arbitrarily
 ///    read to any child in the given tree.
-#[inline(always)]
+#[inline]
 pub unsafe fn minimum_unchecked<K, V, const PREFIX_LEN: usize>(
     root: OpaqueNodePtr<K, V, PREFIX_LEN>,
 ) -> NodePtr<PREFIX_LEN, LeafNode<K, V, PREFIX_LEN>> {
@@ -31,7 +31,7 @@ pub unsafe fn minimum_unchecked<K, V, const PREFIX_LEN: usize>(
 ///  - This function cannot be called concurrently with any mutating operation
 ///    on `root` or any child node of `root`. This function will arbitrarily
 ///    read to any child in the given tree.
-#[inline(always)]
+#[inline]
 pub unsafe fn maximum_unchecked<K, V, const PREFIX_LEN: usize>(
     root: OpaqueNodePtr<K, V, PREFIX_LEN>,
 ) -> NodePtr<PREFIX_LEN, LeafNode<K, V, PREFIX_LEN>> {