Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasmparser: Allow to drop hash-based dependencies entirely #1866

Merged
merged 8 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,16 @@ jobs:
name: bins-${{ matrix.build }}
path: dist

test-no-hash-maps:
test-prefer-btree-collections:
name: Test (no-hash-maps)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- uses: bytecodealliance/wasmtime/.github/actions/[email protected]
- name: Test (no-hash-maps)
run: cargo test --workspace --locked --features no-hash-maps
- name: Test (prefer-btree-collections)
run: cargo test --workspace --locked --features prefer-btree-collections

test:
name: Test
Expand Down Expand Up @@ -238,14 +238,14 @@ jobs:
- run: cargo check --no-default-features -p wasmparser
- run: cargo check --no-default-features -p wasmparser --target x86_64-unknown-none
- run: cargo check --no-default-features -p wasmparser --target x86_64-unknown-none --features validate,serde
- run: cargo check --no-default-features -p wasmparser --target x86_64-unknown-none --features validate,serde,no-hash-maps
- run: cargo check --no-default-features -p wasmparser --target x86_64-unknown-none --features validate,serde,prefer-btree-collections
- run: cargo check --no-default-features -p wasmparser --features std
- run: cargo check --no-default-features -p wasmparser --features validate
- run: cargo check --no-default-features -p wasmparser --features features
- run: cargo check --no-default-features -p wasmparser --features features,validate
- run: cargo check --no-default-features -p wasmparser --features no-hash-maps
- run: cargo check --no-default-features -p wasmparser --features prefer-btree-collections
- run: cargo check --no-default-features -p wasmparser --features serde
- run: cargo check --no-default-features -p wasmparser --features serde,no-hash-maps
- run: cargo check --no-default-features -p wasmparser --features serde,prefer-btree-collections
- run: cargo check --no-default-features -p wasmparser --features component-model
- run: cargo check --no-default-features -p wasmparser --features component-model,validate
- run: cargo check --no-default-features -p wasmparser --features std,component-model
Expand Down Expand Up @@ -315,7 +315,7 @@ jobs:
- verify-publish
- test_capi
- test_extra_features
- test-no-hash-maps
- test-prefer-btree-collections
- clippy
if: always()

Expand Down
25 changes: 13 additions & 12 deletions crates/wasmparser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,30 @@ name = "benchmark"
harness = false

[features]
default = ['std', 'validate', 'serde', 'features', 'component-model']
default = ['std', 'validate', 'serde', 'features', 'component-model', 'hash-collections']

# A feature which enables implementations of `std::error::Error` as appropriate
# along with other convenience APIs. This additionally uses the standard
# library's source of randomness for seeding hash maps.
std = ['indexmap/std']

# Tells the wasmparser crate to avoid using hash based maps and sets.
# Tells the `wasmparser` crate to provide (and use) hash-based collections internally.
#
# 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.
no-hash-maps = []
# Disabling this crate feature allows to drop `hashbrown`, `indexmap` and `ahash` dependencies
# entirely, reducing compilation times and shrink binary sizes.
hash-collections = [
'dep:hashbrown',
'dep:indexmap',
'dep:ahash',
]
# Tells the `wasmparser` crate to prefer using its built-in btree-based collections
# even if `hash-collections` is enabled.
prefer-btree-collections = []

# A feature that enables validating WebAssembly files. This is enabled by
# default but not required if you're only parsing a file, for example, as
# opposed to validating all of its contents.
validate = [
'dep:indexmap',
'dep:semver',
'dep:hashbrown',
'dep:ahash',
]
validate = ['dep:semver']

# Enable Serialize/Deserialize implementations for types in
# `wasmparser::collections`
Expand Down
3 changes: 0 additions & 3 deletions crates/wasmparser/src/collections/index_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,7 @@ where
/// Reserves capacity for at least `additional` more elements to be inserted in the [`IndexMap`].
#[inline]
pub fn reserve(&mut self, additional: usize) {
#[cfg(not(feature = "no-hash-maps"))]
self.inner.reserve(additional);
#[cfg(feature = "no-hash-maps")]
let _ = additional;
}

/// Returns true if `key` is contains in the [`IndexMap`].
Expand Down
10 changes: 8 additions & 2 deletions crates/wasmparser/src/collections/index_map/detail.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
//! An ordered map based on a B-Tree that keeps insertion order of elements.

#[cfg(not(feature = "no-hash-maps"))]
#[cfg(all(
feature = "hash-collections",
not(feature = "prefer-btree-collections")
))]
mod impls {
use crate::collections::hash;
use indexmap::IndexMap;
Expand All @@ -17,7 +20,10 @@ mod impls {
pub type ValuesMutImpl<'a, K, V> = indexmap::map::ValuesMut<'a, K, V>;
}

#[cfg(feature = "no-hash-maps")]
#[cfg(any(
not(feature = "hash-collections"),
feature = "prefer-btree-collections"
))]
mod impls {
pub type IndexMapImpl<K, V> = super::IndexMap<K, V>;
pub type EntryImpl<'a, K, V> = super::Entry<'a, K, V>;
Expand Down
20 changes: 16 additions & 4 deletions crates/wasmparser/src/collections/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
use core::fmt::Debug;
use core::{borrow::Borrow, hash::Hash, iter::FusedIterator, ops::Index};

#[cfg(not(feature = "no-hash-maps"))]
#[cfg(all(
feature = "hash-collections",
not(feature = "prefer-btree-collections")
))]
mod detail {
use crate::collections::hash;
use hashbrown::hash_map;
Expand All @@ -22,7 +25,10 @@ mod detail {
pub type IntoValuesImpl<K, V> = hash_map::IntoValues<K, V>;
}

#[cfg(feature = "no-hash-maps")]
#[cfg(any(
not(feature = "hash-collections"),
feature = "prefer-btree-collections"
))]
mod detail {
use alloc::collections::btree_map;

Expand Down Expand Up @@ -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;
}

Expand Down
10 changes: 10 additions & 0 deletions crates/wasmparser/src/collections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@
//! [`BTreeMap`]: alloc::collections::BTreeMap
//! [`BTreeSet`]: alloc::collections::BTreeSet

// Which collections will be used feature matrix:
//
// `hash-collections` | `prefer-btree-collections` | usage
// ------------------ | -------------------------- | -------------------
// false | false | btree
// true | false | hash
// false | true | btree
// true | true | btree

#[cfg(feature = "hash-collections")]
pub mod hash;
pub mod index_map;
pub mod index_set;
Expand Down
20 changes: 16 additions & 4 deletions crates/wasmparser/src/collections/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::collections::hash;
use hashbrown::hash_set;
Expand All @@ -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 alloc::collections::btree_set;

Expand Down Expand Up @@ -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;
}

Expand Down
1 change: 0 additions & 1 deletion crates/wasmparser/src/validator/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,6 @@ impl ComponentState {
}

let mut set = Set::default();
#[cfg(not(feature = "no-hash-maps"))] // TODO: remove when unified map type is available
set.reserve(core::cmp::max(ty.params.len(), ty.results.type_count()));

let params = ty
Expand Down