Add wide-ids feature, simplify id type impl

Cargo.toml

@@ -27,12 +27,16 @@ env_logger = "0.9"
 assert_matches = "1.5"
 
 [features]
-# dump the graph to a DOT representation
+# Enable the parallel runtime implementation todo make default
 parallel-runtime=["rayon"]
+# Enables 64-bit wide reaction ids on 64 bit architectures.
+# This may reduce performance, but allows for 2^32 reactor
+# instances compared to the default of 2^16, which may feel
+# a bit tight for some applications.
+wide-ids=[]
 no-unsafe=[]
-# used mostly only for benchmarking
+# used internally for benchmarking, to access private APIs
 public-internals=[]
-benchmarking=["public-internals"]
 
 [[bench]]
 name = "savina_pong"
@@ -41,11 +45,13 @@ harness = false
 [[bench]]
 name = "global_id"
 path = "benches/micro/global_id.rs"
+required-features = ["public-internals"]
 harness = false
 
 [[bench]]
 name = "exec_reactions"
 path = "benches/micro/exec_reactions.rs"
+required-features = ["public-internals"]
 harness = false
 
 [profile.bench]

benches/micro/global_id.rs

@@ -24,6 +24,12 @@
 
 //! Compares the implementation of global ID based on u32
 //! vs a struct with two u16.
+//! This benchmark verified that the hashing function provided
+//! by #\[derive(Hash)] is slower than transmuting to u32 and
+//! writing that into the hasher.
+//! With --features wide-ids you can run the same benchmark
+//! with ids that are twice as wide, if your machine supports
+//! it.
 
 #![allow(unused, non_snake_case, non_camel_case_types)]
 #[macro_use]
@@ -34,17 +40,7 @@ use std::hash::{Hash, Hasher};
 
 use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
 
-cfg_if::cfg_if! {
-    if #[cfg(target_pointer_width = "64")] {
-        type GlobalIdImpl = u64;
-        type ReactorIdImpl = u32;
-        type ReactionIdImpl = u32;
-    } else {
-        type GlobalIdImpl = u32;
-        type ReactorIdImpl = u16;
-        type ReactionIdImpl = u16;
-    }
-}
+use reactor_rt::internals::*;
 
 #[derive(Hash, Eq, PartialEq, Copy, Clone)]
 struct GID_raw {

src/ids.rs

@@ -165,12 +165,15 @@ impl FromStr for GlobalId {
     }
 }
 
-// hashing global ids is a very hot operation in the framework,
-// therefore we give it an optimal implementation
+// Hashing global ids is a very hot operation in the framework,
+// therefore we give it an optimal implementation.
+// The implementation was verified to be faster than the default
+// derive by a micro benchmark in this repo.
 impl Hash for GlobalId {
     #[inline]
     fn hash<H: Hasher>(&self, state: &mut H) {
         let as_impl: &GlobalIdImpl = unsafe { std::mem::transmute(self) };
+        // this is written so that it works regardless of the concrete type of GlobalIdImpl
         Hash::hash(as_impl, state);
     }
 }
@@ -191,12 +194,9 @@ impl Display for GlobalId {
 
 /// private implementation types
 pub(crate) mod impl_types {
-    use std::fmt::{Display, Formatter};
-
-    use petgraph::graph::IndexType;
     cfg_if! {
-        if #[cfg(target_pointer_width = "64")] {
-            type MyUsize = IndexTypeU64;
+        if #[cfg(all(target_pointer_width = "64", feature = "wide-ids"))] {
+            type MyUsize = usize;
             type HalfUsize = u32;
         } else {
             type MyUsize = u32;
@@ -208,42 +208,6 @@ pub(crate) mod impl_types {
     pub type ReactorIdImpl = HalfUsize;
     pub type GlobalIdImpl = MyUsize;
     assert_eq_size!(GlobalIdImpl, (ReactorIdImpl, ReactionIdImpl));
-
-    // petgraph doesn't implement IndexType for u64, so we need our own wrapper
-    #[cfg(target_pointer_width = "64")]
-    #[derive(Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Debug, Default, Hash)]
-    #[repr(transparent)]
-    #[doc(hidden)]
-    pub struct IndexTypeU64(u64);
-
-    #[cfg(target_pointer_width = "64")]
-    unsafe impl IndexType for IndexTypeU64 {
-        #[inline(always)]
-        fn new(x: usize) -> Self {
-            assert_eq_size!(usize, u64);
-            IndexTypeU64(x as u64)
-        }
-
-        #[inline(always)]
-        fn index(&self) -> usize {
-            self.0 as usize
-        }
-
-        #[inline(always)]
-        fn max() -> Self {
-            IndexTypeU64(u64::MAX)
-        }
-    }
-
-    impl From<u32> for IndexTypeU64 {
-        fn from(i: u32) -> Self {
-            Self(i as u64)
-        }
-    }
-
-    impl Display for IndexTypeU64 {
-        fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-            write!(f, "{}", self.0)
-        }
-    }
+    assert_impl_all!(GlobalIdImpl: petgraph::graph::IndexType);
+    assert_impl_all!(ReactorIdImpl: petgraph::graph::IndexType);
 }

src/lib.rs

@@ -32,9 +32,15 @@
 //!
 //! Crate-level features include:
 //! - `parallel-runtime`: use Rayon to execute reactions in parallel
-//! when possible. This is off by default. For some applications,
+//! when possible. This is not yet the default. For some applications,
 //! where there is no data parallelism, this may harm performance
-//! (as well as pull in unneeded dependencies) and should stay off.
+//! (as well as pull in unneeded dependencies) and should be off.
+//! - `wide-ids`: Enables 64-bit wide reaction ids on 64-bit
+//! architectures. This may reduce performance, but allows for
+//! 2^32 reactor instances compared to the default of 2^16,
+//! which may feel a bit tight for some applications. On machines
+//! with a pointer-width of less than 64 bits, ID types are
+//! always 32 bits.
 //! - `no-unsafe`: disable optimisations that use unsafe code in this runtime.
 //! Just provided for comparison, should probably be removed (unsafe code is fine).
 
@@ -134,6 +140,7 @@ pub trait ReactorBehavior {
 }
 assert_obj_safe!(ReactorBehavior);
 
+/// Used for benchmarking to access private API of the crate.
 #[cfg(feature = "public-internals")]
 #[doc(hidden)]
 pub mod internals {