Skip to content

Comments

feat: Propagate map-foldhash Feature Through Dependency Chain#3252

Merged
rakita merged 2 commits intobluealloy:mainfrom
woojinnn:feat/map-foldhash-revme
Dec 24, 2025
Merged

feat: Propagate map-foldhash Feature Through Dependency Chain#3252
rakita merged 2 commits intobluealloy:mainfrom
woojinnn:feat/map-foldhash-revme

Conversation

@woojinnn
Copy link
Contributor

@woojinnn woojinnn commented Dec 24, 2025

Summary

#3246
This PR completes the map-foldhash feature propagation across the revm workspace, enabling users to leverage the faster foldhash algorithm for HashMap operations throughout the entire dependency chain, including the revme binary and statetest-types crate.

Problem

The map-foldhash feature enables the foldhash algorithm (via alloy-primitives/map-foldhash) for improved HashMap performance. Previously, this feature was only partially implemented:

  • Defined in: revm-primitives and revm crates
  • Missing from: revm-state, revm-database, revm-context, statetest-types, and revme

This incomplete propagation caused the following issues:

  1. Users couldn't enable map-foldhash when building revme:

    cargo build -p revme --features map-foldhash
    # error: the package 'revme' does not contain this feature: map-foldhash
  2. The feature didn't propagate through the statetest-types dependency chain, limiting its effectiveness when running Ethereum state tests.

  3. Intermediate crates (state, database, context) couldn't forward the feature to their dependencies, breaking the feature propagation chain.

Broken Feature Chain (Before)

revme
  └─> revm (with test-types)
        └─> statetest-types ❌ (no map-foldhash feature)
              ├─> primitives ✓
              ├─> state ❌ (no map-foldhash feature)
              ├─> context ❌ (no map-foldhash feature)
              └─> database ❌ (no map-foldhash feature)

Solution

Added map-foldhash feature definitions to all crates in the dependency chain.

Validation Commands

All of the following commands should succeed after this change:

# Individual crate checks
cargo check -p revm-state --features map-foldhash
cargo check -p revm-database --features map-foldhash
cargo check -p revm-context --features map-foldhash
cargo check -p revm-statetest-types --features map-foldhash

# Main crate with combined features
cargo check -p revm --features "std,map-foldhash,test-types"

# Revme binary (end-to-end validation)
cargo build -p revme --features map-foldhash

# Workspace-wide check
cargo check --workspace --features map-foldhash

Notes

  • The implementation follows revm's existing feature flag patterns
  • Uses optional dependency syntax (statetest-types?/map-foldhash) to handle conditional dependencies correctly (crates/revm/Cargo.toml)
  • Maintains no_std compatibility (feature only affects HashMap implementation, not allocator requirements)
  • No breaking changes; this is purely an enhancement to feature flag propagation

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 24, 2025

CodSpeed Performance Report

Merging #3252 will improve performance by 21.91%

Comparing woojinnn:feat/map-foldhash-revme (07c00cc) with main (5d7a353)

Summary

⚡ 131 improvements
✅ 42 untouched

Benchmarks breakdown

Benchmark BASE HEAD Efficiency
LT_50 25.1 µs 24.3 µs +3.37%
ADDRESS_50 20 µs 19.2 µs +4.57%
AND_50 24.8 µs 23.9 µs +3.66%
CALLDATASIZE_50 19.8 µs 19 µs +4.14%
ADD_50 24.8 µs 24 µs +3.65%
BYTE_50 25.1 µs 24.2 µs +3.74%
MCOPY_50 22.1 µs 21.4 µs +3.41%
CALLER_50 20 µs 19.2 µs +4.41%
MOD_50 27 µs 26.2 µs +3.12%
MCOPY_COLD_50 26.2 µs 25.3 µs +3.58%
MSIZE_50 19.7 µs 19 µs +3.38%
CALL_50 106.3 µs 98.5 µs +7.89%
CHAINID_50 19.7 µs 19 µs +3.85%
MSTORE_50 20.1 µs 19.4 µs +3.62%
NUMBER_50 19.7 µs 19 µs +3.69%
CALLVALUE_50 19.9 µs 19.1 µs +4.29%
CLZ_50 24.4 µs 23.2 µs +5.23%
MSTORE_COLD_50 24.1 µs 23.4 µs +3.12%
NOT_50 24.7 µs 23.9 µs +3.55%
CODESIZE_50 19.9 µs 19.1 µs +4.29%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@woojinnn woojinnn marked this pull request as ready for review December 24, 2025 06:34
@woojinnn woojinnn force-pushed the feat/map-foldhash-revme branch from 2184120 to efc55d6 Compare December 24, 2025 07:29
@woojinnn
Copy link
Contributor Author

Benchmark tests results

Commands

# Without map-foldhash feature
cargo bench -p revme --no-fail-fast

# With map-foldhash feature
cargo bench -p revme --features map-foldhash --no-fail-fast

Full Benchmark Results

Benchmark legend:

  • Storage Operations (HashMap-heavy): SSTORE_50 (20.98%), SLOAD_50 (17.16%)
  • Transaction Execution: transfer (16.46%), transfer_finalize (5.33%)
  • Complex Contract Execution: burntpix (6.37%), snailtracer (3.58%), snailtracer-inspect (2.32%)
  • Lightweight Operations: analysis (-1.37%)

Without map-foldhash:

analysis                time:   [4.1366 µs 4.1824 µs 4.2375 µs]
burntpix                time:   [188.30 ms 188.62 ms 188.96 ms]
snailtracer             time:   [62.818 ms 62.930 ms 63.036 ms]
snailtracer-inspect     time:   [60.681 ms 60.805 ms 60.934 ms]
transfer                time:   [564.47 ns 569.01 ns 573.69 ns]
transfer_finalize       time:   [758.26 ns 762.39 ns 767.45 ns]
SSTORE_50               time:   [4.8388 µs 4.8566 µs 4.8796 µs]
SLOAD_50                time:   [4.2510 µs 4.2616 µs 4.2729 µs]

With map-foldhash:

analysis                time:   [4.2238 µs 4.2396 µs 4.2585 µs]
burntpix                time:   [176.26 ms 176.60 ms 176.91 ms]
snailtracer             time:   [60.300 ms 60.679 ms 61.083 ms]
snailtracer-inspect     time:   [59.201 ms 59.395 ms 59.606 ms]
transfer                time:   [474.05 ns 475.37 ns 477.01 ns]
transfer_finalize       time:   [719.95 ns 721.75 ns 723.64 ns]
SSTORE_50               time:   [3.8249 µs 3.8379 µs 3.8534 µs]
SLOAD_50                time:   [3.5217 µs 3.5305 µs 3.5406 µs]

@@ -43,6 +43,7 @@ csv = "1.1.6"
[features]
# Optionally enable gmp because it doesn't work on i686 github actions runners
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make map-foldhash be default default = ["map-foldhash"]

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, but lgtm

In future, try to tell AI to do a summary of all the text in the original PR text, it writes much info.

Benchmark comment, for example, was good and easy to read.

@woojinnn
Copy link
Contributor Author

@rakita Applied your review. Got it.

@rakita rakita merged commit 3b17ee6 into bluealloy:main Dec 24, 2025
31 checks passed
@github-actions github-actions bot mentioned this pull request Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants