From ef5cdf4033b4cb06be4ab52221dc7ba4b758aef2 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Tue, 29 Oct 2024 15:31:29 +0100 Subject: [PATCH] Refactor crate features for more control over hash-maps usage (#1265) * refactor crate features for more control over hash-maps usage * fix GitHub CI build job * remove superflous docs Docs are superflous since it is obvious where to look-up the docs. --- .github/workflows/rust.yml | 4 +-- crates/cli/Cargo.toml | 5 +-- crates/collections/Cargo.toml | 44 ++++++++++++++++------- crates/collections/src/lib.rs | 1 + crates/collections/src/map.rs | 20 ++++++++--- crates/collections/src/set.rs | 20 ++++++++--- crates/collections/src/string_interner.rs | 10 ++++-- crates/wasmi/Cargo.toml | 12 ++----- 8 files changed, 81 insertions(+), 35 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index eda97b86c1..b6213b9cbb 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -36,14 +36,14 @@ jobs: --no-default-features --target x86_64-unknown-none --verbose - - name: Build (no_std + no-hash-maps) + - name: Build (no_std + hash-collections) run: >- cargo build --package wasmi --locked --lib --no-default-features - --features no-hash-maps + --features hash-collections --target x86_64-unknown-none --verbose - name: Build (no_std + wasm32) diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 4bd1cd6e72..a1f32579f6 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -24,8 +24,9 @@ wat = "1" assert_cmd = "2.0.7" [features] -default = ["no-hash-maps"] -no-hash-maps = ["wasmi/no-hash-maps"] +default = [] +hash-collections = ["wasmi/hash-collections"] +prefer-btree-collections = ["wasmi/prefer-btree-collections"] # We need to put this [profile.release] section due to this bug in Cargo: # https://github.com/rust-lang/cargo/issues/8264 diff --git a/crates/collections/Cargo.toml b/crates/collections/Cargo.toml index 461ea3e342..809b238d57 100644 --- a/crates/collections/Cargo.toml +++ b/crates/collections/Cargo.toml @@ -14,24 +14,44 @@ categories.workspace = true exclude.workspace = true [dependencies] -hashbrown = { version = "0.14", default-features = false, features = ["ahash", "inline-more"] } -string-interner = { version = "0.17", default-features = false, features = ["inline-more", "backends"] } -ahash = { version = "0.8.11", default-features = false } +hashbrown = { version = "0.14", default-features = false, optional = true, features = ["ahash", "inline-more"] } +string-interner = { version = "0.17", default-features = false, optional = true, features = ["inline-more", "backends"] } +ahash = { version = "0.8.11", default-features = false, optional = true } [features] -default = ["std", "no-hash-maps"] +default = ["std"] std = ["string-interner/std"] -# Tells the `wasmi_collections` crate to avoid using hash based maps and sets. -# -# Some embedded environments cannot provide a random source which is required -# to properly initialize hashmap based data structures for resilience against -# malious actors that control their inputs. + +# Hash collections usage: +# +# - Enable `hash-collections` to make use of hash-based collections in `wasmi_collections`. +# - Enable `prefer-btree-collections` to still use btree-based collections even when +# the `hash-collections` crate feature is enabled. +# +# Note: +# +# - Not enabling `hash-collections` allows `wasmi_collections` to drop lots of +# hash-based dependencies and thus decrease compilation times significantly. +# - Btree-based collections can be useful for environments without a random source. # -# An example of such an environment is `wasm32-unknown-unknown`. -no-hash-maps = [] +# Which collections will be used: +# +# `hash-collections` | `prefer-btree-collections` | usage +# ------------------ | -------------------------- | ------------------- +# false | false | btree +# true | false | hash +# false | true | btree +# true | true | btree +# +hash-collections = [ + 'dep:hashbrown', + 'dep:string-interner', + 'dep:ahash', +] +prefer-btree-collections = [] [package.metadata.cargo-udeps.ignore] normal = [ - # The string-interner dependency is always specified even though it is unused when no-hash-maps is enabled. + # Needed to suppress weird `udep` warnings. Maybe a `cargo-udep` bug? "string-interner" ] diff --git a/crates/collections/src/lib.rs b/crates/collections/src/lib.rs index a78706aed6..c83a6a36b7 100644 --- a/crates/collections/src/lib.rs +++ b/crates/collections/src/lib.rs @@ -37,6 +37,7 @@ extern crate alloc as std; extern crate std; pub mod arena; +#[cfg(feature = "hash-collections")] pub mod hash; mod head_vec; pub mod map; diff --git a/crates/collections/src/map.rs b/crates/collections/src/map.rs index aa079286ce..59f93c1a7e 100644 --- a/crates/collections/src/map.rs +++ b/crates/collections/src/map.rs @@ -3,7 +3,10 @@ use core::{borrow::Borrow, hash::Hash, iter::FusedIterator, ops::Index}; use std::fmt::Debug; -#[cfg(not(feature = "no-hash-maps"))] +#[cfg(all( + feature = "hash-collections", + not(feature = "prefer-btree-collections") +))] mod detail { use crate::hash; use hashbrown::hash_map; @@ -22,7 +25,10 @@ mod detail { pub type IntoValuesImpl = hash_map::IntoValues; } -#[cfg(feature = "no-hash-maps")] +#[cfg(any( + not(feature = "hash-collections"), + feature = "prefer-btree-collections" +))] mod detail { use std::collections::btree_map; @@ -155,9 +161,15 @@ where /// Reserves capacity for at least `additional` more elements to be inserted in the [`Map`]. #[inline] pub fn reserve(&mut self, additional: usize) { - #[cfg(not(feature = "no-hash-maps"))] + #[cfg(all( + feature = "hash-collections", + not(feature = "prefer-btree-collections") + ))] self.inner.reserve(additional); - #[cfg(feature = "no-hash-maps")] + #[cfg(any( + not(feature = "hash-collections"), + feature = "prefer-btree-collections" + ))] let _ = additional; } diff --git a/crates/collections/src/set.rs b/crates/collections/src/set.rs index eb4fbb80a6..a0d91b413f 100644 --- a/crates/collections/src/set.rs +++ b/crates/collections/src/set.rs @@ -8,7 +8,10 @@ use core::{ ops::{BitAnd, BitOr, BitXor, Sub}, }; -#[cfg(not(feature = "no-hash-maps"))] +#[cfg(all( + feature = "hash-collections", + not(feature = "prefer-btree-collections") +))] mod detail { use crate::hash; use hashbrown::hash_set; @@ -23,7 +26,10 @@ mod detail { pub type UnionImpl<'a, T> = hash_set::Union<'a, T, hash::RandomState>; } -#[cfg(feature = "no-hash-maps")] +#[cfg(any( + not(feature = "hash-collections"), + feature = "prefer-btree-collections" +))] mod detail { use std::collections::btree_set; @@ -105,9 +111,15 @@ where /// Reserves capacity for at least `additional` more elements to be inserted in the [`Set`]. #[inline] pub fn reserve(&mut self, additional: usize) { - #[cfg(not(feature = "no-hash-maps"))] + #[cfg(all( + feature = "hash-collections", + not(feature = "prefer-btree-collections") + ))] self.inner.reserve(additional); - #[cfg(feature = "no-hash-maps")] + #[cfg(any( + not(feature = "hash-collections"), + feature = "prefer-btree-collections" + ))] let _ = additional; } diff --git a/crates/collections/src/string_interner.rs b/crates/collections/src/string_interner.rs index 6e0969c10f..2ab006b9ac 100644 --- a/crates/collections/src/string_interner.rs +++ b/crates/collections/src/string_interner.rs @@ -1,6 +1,9 @@ //! Data structure to efficiently store and deduplicate strings. -#[cfg(not(feature = "no-hash-maps"))] +#[cfg(all( + feature = "hash-collections", + not(feature = "prefer-btree-collections") +))] mod detail { use super::{GetOrInternWithHint, Sym}; use crate::hash; @@ -34,7 +37,10 @@ mod detail { } } -#[cfg(feature = "no-hash-maps")] +#[cfg(any( + not(feature = "hash-collections"), + feature = "prefer-btree-collections" +))] mod detail; /// Internment hint to speed-up certain use cases. diff --git a/crates/wasmi/Cargo.toml b/crates/wasmi/Cargo.toml index 1a0a97035d..2b5ba2acbd 100644 --- a/crates/wasmi/Cargo.toml +++ b/crates/wasmi/Cargo.toml @@ -40,7 +40,7 @@ anyhow = "1.0" criterion = { version = "0.5", default-features = false } [features] -default = ["std", "no-hash-maps"] +default = ["std"] std = [ "wasmi_core/std", "wasmi_collections/std", @@ -48,14 +48,8 @@ std = [ "spin/std", "arrayvec/std", ] -# Tells the `wasmi` crate to avoid using hash based data structures. -# -# Some embedded environments cannot provide a random source which is required -# to properly initialize hashmap based data structures for resilience against -# malious actors that control their inputs. -# -# An example of such an environment is `wasm32-unknown-unknown`. -no-hash-maps = ["wasmi_collections/no-hash-maps"] +hash-collections = ["wasmi_collections/hash-collections"] +prefer-btree-collections = ["wasmi_collections/prefer-btree-collections"] # Enables extra checks performed during Wasmi bytecode execution. #