Replace internal maps with unsorted Vecs

HashMap and BTreeMap are overkill in this context. Unsorted vectors are
plenty fast enough and the necessary collection interfaces are
straightforward to implement. This change has two benefits.

First, it improves binary size. For the `print` example from signal-hook
in release mode, the .text section shrinks by about 18 KiB and overall
file size shrinks by about 30 KiB. That's roughly a 6% reduction in both
metrics.

Second, the simpler data structures make it more obvious that the signal
handler only does async-signal-safe operations. In particular, the
default HashMap has a `RandomState`, which can access TLS, do dlsym
lookups, open and read from files, etc. depending on the platform. I
don't think that's a problem for the hash table *lookup* done in the
signal handler since that shouldn't touch the `RandomState`, but it's a
bit subtle and the standard library doesn't make any guarantees about
this. Avoiding hash maps entirely removes the need to think about it.

Performance notes:

* (Un-)registering actions does an insert/remove by ActionId, which is
  asymptotically slower with this PR. However, (un-)registering is a
  slow operation and should be done rarely. Besides locking, it always
  clones the entire `SignalData`, so it already takes O(n) time when
  there's n actions registered across all symbols.
* The signal handler looks up the `Slot` by signal number, which is
  asymptotically slower with this PR. However, there's only a very small
  constant number of signals, so asymptotics don't matter.
* After looking up the right `Slot`, the signal handler only iterates
  sequentially over the actions, it doesn't do any lookups by ActionId.
* For a simple microbenchmark that registers one action each for 20
  signals and then raises one signal 100k times, this implementation
  appears to be slightly *faster* regardless of which signal is raised.

Author: hanna-kruppe

signal-hook-registry/src/lib.rs

@@ -65,9 +65,8 @@
 extern crate libc;
 
 mod half_lock;
+mod vec_map;
 
-use std::collections::hash_map::Entry;
-use std::collections::{BTreeMap, HashMap};
 use std::io::Error;
 use std::mem;
 use std::ptr;
@@ -89,6 +88,7 @@ use libc::{SIGFPE, SIGILL, SIGKILL, SIGSEGV, SIGSTOP};
 use libc::{SIGFPE, SIGILL, SIGSEGV};
 
 use half_lock::HalfLock;
+use vec_map::{Entry, VecMap};
 
 // These constants are not defined in the current version of libc, but it actually
 // exists in Windows CRT.
@@ -140,9 +140,8 @@ type Action = Fn(&siginfo_t) + Send + Sync;
 #[derive(Clone)]
 struct Slot {
     prev: Prev,
-    // We use BTreeMap here, because we want to run the actions in the order they were inserted.
-    // This works, because the ActionIds are assigned in an increasing order.
-    actions: BTreeMap<ActionId, Arc<Action>>,
+    // Actions are stored and executed in the order they were registered.
+    actions: VecMap<ActionId, Arc<Action>>,
 }
 
 impl Slot {
@@ -200,14 +199,14 @@ impl Slot {
         }
         Ok(Slot {
             prev: Prev { signal, info: old },
-            actions: BTreeMap::new(),
+            actions: VecMap::new(),
         })
     }
 }
 
 #[derive(Clone)]
 struct SignalData {
-    signals: HashMap<c_int, Slot>,
+    signals: VecMap<c_int, Slot>,
     next_id: u128,
 }
 
@@ -321,7 +320,7 @@ impl GlobalData {
         GLOBAL_INIT.call_once(|| {
             let data = Box::into_raw(Box::new(GlobalData {
                 data: HalfLock::new(SignalData {
-                    signals: HashMap::new(),
+                    signals: VecMap::new(),
                     next_id: 1,
                 }),
                 race_fallback: HalfLock::new(None),

signal-hook-registry/src/vec_map.rs

@@ -0,0 +1,99 @@
+use std::mem;
+
+/// A small map backed by an unsorted vector.
+///
+/// Maintains key uniqueness at cost of O(n) lookup/insert/remove. Maintains insertion order
+/// (`insert` calls that overwrite an existing value don't change order).
+#[derive(Clone, Default)]
+pub struct VecMap<K, V>(Vec<(K, V)>);
+
+impl<K: Eq, V> VecMap<K, V> {
+    pub fn new() -> Self {
+        VecMap(Vec::new())
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    pub fn clear(&mut self) {
+        self.0.clear();
+    }
+
+    fn find(&self, key: &K) -> Option<usize> {
+        for (i, (k, _)) in self.0.iter().enumerate() {
+            if k == key {
+                return Some(i);
+            }
+        }
+        None
+    }
+
+    pub fn get(&self, key: &K) -> Option<&V> {
+        match self.find(key) {
+            Some(i) => Some(&self.0[i].1),
+            None => None,
+        }
+    }
+
+    pub fn get_mut(&mut self, key: &K) -> Option<&mut V> {
+        match self.find(key) {
+            Some(i) => Some(&mut self.0[i].1),
+            None => None,
+        }
+    }
+
+    pub fn insert(&mut self, key: K, value: V) -> Option<V> {
+        if let Some(old) = self.get_mut(&key) {
+            return Some(mem::replace(old, value));
+        }
+        self.0.push((key, value));
+        None
+    }
+
+    pub fn remove(&mut self, key: &K) -> Option<V> {
+        match self.find(key) {
+            Some(i) => Some(self.0.remove(i).1),
+            None => None,
+        }
+    }
+
+    pub fn entry(&mut self, key: K) -> Entry<'_, K, V> {
+        match self.find(&key) {
+            Some(i) => Entry::Occupied(OccupiedEntry {
+                place: &mut self.0[i],
+            }),
+            None => Entry::Vacant(VacantEntry { map: self, key }),
+        }
+    }
+
+    pub fn values(&self) -> impl Iterator<Item = &V> {
+        self.0.iter().map(|kv| &kv.1)
+    }
+}
+
+pub enum Entry<'a, K: 'a, V: 'a> {
+    Occupied(OccupiedEntry<'a, K, V>),
+    Vacant(VacantEntry<'a, K, V>),
+}
+
+pub struct OccupiedEntry<'a, K: 'a, V: 'a> {
+    place: &'a mut (K, V),
+}
+
+impl<'a, K, V> OccupiedEntry<'a, K, V> {
+    pub fn get_mut(&mut self) -> &mut V {
+        &mut self.place.1
+    }
+}
+
+pub struct VacantEntry<'a, K: 'a, V: 'a> {
+    map: &'a mut VecMap<K, V>,
+    key: K,
+}
+
+impl<'a, K, V> VacantEntry<'a, K, V> {
+    pub fn insert(self, value: V) {
+        self.map.0.push((self.key, value));
+    }
+}