Skip to content

Comments

feat: Add back support for big-endian targets#32

Merged
DaniPopes merged 3 commits intoalloy-rs:mainfrom
clabby:cl/big-endian-supp
Jul 7, 2025
Merged

feat: Add back support for big-endian targets#32
DaniPopes merged 3 commits intoalloy-rs:mainfrom
clabby:cl/big-endian-supp

Conversation

@clabby
Copy link
Contributor

@clabby clabby commented Jul 6, 2025

Overview

Adds back big-endian support after #17, cc @shekhirin

These changes should have no impact on the performance of this crate when targeting little-endian - All methods that require read-only access to the underlying U256 can use as_le_bytes, and methods that relied on as_le_slice_mut retain the existing implementation when cfg(target_endian = "little").

Also adds a new CI job to build/test the crate on big-endian 32-bit and 64-bit MIPS targets.

Meta

closes #31

Copy link
Contributor

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! @DaniPopes please take a look too

@codspeed-hq
Copy link

codspeed-hq bot commented Jul 7, 2025

CodSpeed Performance Report

Merging #32 will degrade performances by 25.69%

Comparing clabby:cl/big-endian-supp (647b4cd) with main (a2853a6)

Summary

⚡ 18 improvements
❌ 9 regressions
✅ 84 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
common_prefix_length[16] 242.8 ns 213.6 ns +13.65%
common_prefix_length[32] 395.6 ns 337.2 ns +17.3%
common_prefix_length[64] 395.6 ns 337.2 ns +17.3%
common_prefix_length[8] 242.8 ns 213.6 ns +13.65%
ends_with[16] 342.2 ns 400.6 ns -14.56%
ends_with[32] 342.2 ns 403.3 ns -15.15%
ends_with[64] 342.2 ns 460.6 ns -25.69%
extend[16] 471.4 ns 413.1 ns +14.12%
extend[32] 467.2 ns 408.9 ns +14.27%
extend[8] 474.2 ns 415.8 ns +14.03%
get_byte_unchecked[16] 279.4 ns 250.3 ns +11.65%
get_byte_unchecked[32] 419.7 ns 361.4 ns +16.14%
get_byte_unchecked[8] 223.9 ns 194.7 ns +14.98%
last[32] 123.1 ns 152.2 ns -19.16%
pack[32] 463.1 ns 521.4 ns -11.19%
set_at[8] 213.6 ns 242.8 ns -12.01%
to_end[16] 290 ns 260.8 ns +11.18%
to_end[32] 283.1 ns 253.9 ns +11.49%
to_end[64] 283.1 ns 253.9 ns +11.49%
to_end[8] 290 ns 260.8 ns +11.18%
... ... ... ... ...

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

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

lgtm; to_le_bytes works but it kills performance because of the heap copy, you can make a separate function because we know it's 32 bytes (u256) that makes a stack copy and reverses instead of one on the heap

- run: cargo test --all-features
if: matrix.rust == 'nightly'

test-be:
Copy link
Member

Choose a reason for hiding this comment

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

cant we add an include to the test matrix? or is it because of build-std

Copy link
Contributor Author

@clabby clabby Jul 7, 2025

Choose a reason for hiding this comment

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

Yeah, build-std (always needs nightly) + dtolnay/rust-toolchain will error when installing a tier-3 target due to rust-std not being distributed for them. We could add some more conditionals to the original job, but figured splitting them up was a bit more readable.

@clabby clabby force-pushed the cl/big-endian-supp branch 5 times, most recently from f593afe to 0db8252 Compare July 7, 2025 16:10
@clabby
Copy link
Contributor Author

clabby commented Jul 7, 2025

lgtm; to_le_bytes works but it kills performance because of the heap copy, you can make a separate function because we know it's 32 bytes (u256) that makes a stack copy and reverses instead of one on the heap

Updated the impl to have a container that prevents touching the heap in the big-endian case.

e: whoops, fixing CI...

@clabby clabby force-pushed the cl/big-endian-supp branch from 0db8252 to e8aecc5 Compare July 7, 2025 16:16
@clabby clabby requested a review from DaniPopes July 7, 2025 16:21
@clabby clabby force-pushed the cl/big-endian-supp branch 2 times, most recently from 1922c10 to 954d455 Compare July 7, 2025 16:31
@clabby clabby force-pushed the cl/big-endian-supp branch 5 times, most recently from 3ec0dd2 to 2837fc9 Compare July 7, 2025 16:43
@clabby clabby force-pushed the cl/big-endian-supp branch from 2837fc9 to 647b4cd Compare July 7, 2025 16:43
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Thanks! Performance is unaffected

@DaniPopes DaniPopes merged commit 2c7a006 into alloy-rs:main Jul 7, 2025
21 of 22 checks passed
@DaniPopes
Copy link
Member

Released in 0.4.1

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.

bug: 0.4.0 cannot be used on big-endian targets

3 participants