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.

Sign up for GitHub

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

Jump to bottom

Add bit_order attribute #483

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add bit_order attribute #483

wants to merge 2 commits into from
+1,780 −265

Conversation

wcampbell0x2a
Copy link
Collaborator

@wcampbell0x2a wcampbell0x2a commented Sep 26, 2024 •
edited
Loading

Closes #134, #371, #485

@wcampbell0x2a wcampbell0x2a mentioned this pull request Sep 26, 2024
Closed
@github-actions GitHub Actions
Copy link

Benchmark for 3a408bd

Click to view benchmark
Test Base PR %
count 5.0±0.02µs 5.0±0.02µs 0.00%
deku_read_bits 1131.8±10.44ns 1163.6±14.73ns +2.81%
deku_read_byte 5.9±0.18ns 5.3±0.07ns -10.17%
deku_read_enum 4.7±0.04ns 3.9±0.04ns -17.02%
deku_read_vec 41.9±0.33ns 41.6±1.86ns -0.72%
deku_write_bits 189.4±7.62ns 220.3±3.84ns +16.31%
deku_write_byte 25.1±1.08ns 22.8±2.13ns -9.16%
deku_write_enum 22.8±0.18ns 21.2±0.36ns -7.02%
deku_write_vec 406.7±7.32ns 357.8±6.71ns -12.02%
read_all 3.8±0.05µs 4.0±0.10µs +5.26%
read_all_bytes 3.9±0.12µs 3.9±0.11µs 0.00%

@github-actions GitHub Actions
Copy link

Benchmark for e38e559

Click to view benchmark
Test Base PR %
count 5.0±0.02µs 5.0±0.02µs 0.00%
deku_read_bits 1129.4±11.94ns 1161.0±11.19ns +2.80%
deku_read_byte 5.9±0.05ns 4.9±0.06ns -16.95%
deku_read_enum 4.7±0.10ns 5.3±0.03ns +12.77%
deku_read_vec 41.8±0.30ns 42.3±0.30ns +1.20%
deku_write_bits 183.0±2.81ns 219.3±4.54ns +19.84%
deku_write_byte 25.0±0.21ns 22.5±0.26ns -10.00%
deku_write_enum 22.8±0.40ns 21.4±1.04ns -6.14%
deku_write_vec 407.4±13.36ns 354.0±7.89ns -13.11%
read_all 3.8±0.04µs 5.6±0.08µs +47.37%
read_all_bytes 3.9±0.04µs 5.6±0.08µs +43.59%

@github-actions GitHub Actions
Copy link

Benchmark for 7cdecbf

Click to view benchmark
Test Base PR %
count 5.0±0.01µs 5.0±0.01µs 0.00%
deku_read_bits 1147.5±20.80ns 1214.1±24.79ns +5.80%
deku_read_byte 5.9±0.19ns 4.9±0.07ns -16.95%
deku_read_enum 4.7±0.05ns 5.3±0.03ns +12.77%
deku_read_vec 41.8±0.43ns 42.1±0.37ns +0.72%
deku_write_bits 201.3±4.28ns 217.0±5.44ns +7.80%
deku_write_byte 25.0±0.44ns 22.6±0.28ns -9.60%
deku_write_enum 22.8±0.39ns 21.5±0.44ns -5.70%
deku_write_vec 406.9±8.96ns 333.1±8.49ns -18.14%
read_all 3.8±0.06µs 5.6±0.03µs +47.37%
read_all_bytes 3.9±0.06µs 5.6±0.04µs +43.59%

@github-actions GitHub Actions
Copy link

Benchmark for f40bbec

Click to view benchmark
Test Base PR %
count 5.0±0.02µs 5.0±0.02µs 0.00%
deku_read_bits 1155.5±19.46ns 1230.4±17.47ns +6.48%
deku_read_byte 5.8±0.08ns 4.6±0.06ns -20.69%
deku_read_enum 5.3±0.03ns 4.1±0.04ns -22.64%
deku_read_vec 42.3±0.39ns 41.2±0.51ns -2.60%
deku_write_bits 186.9±4.09ns 218.2±2.65ns +16.75%
deku_write_byte 25.0±0.34ns 22.5±0.41ns -10.00%
deku_write_enum 22.8±0.43ns 21.2±0.20ns -7.02%
deku_write_vec 421.3±5.12ns 336.3±10.84ns -20.18%
read_all 5.5±0.05µs 5.6±0.04µs +1.82%
read_all_bytes 5.6±0.02µs 5.5±0.04µs -1.79%

@github-actions GitHub Actions
Copy link

Benchmark for ac4ba04

Click to view benchmark
Test Base PR %
count 5.0±0.02µs 5.0±0.01µs 0.00%
deku_read_bits 1323.8±22.60ns 1204.8±17.14ns -8.99%
deku_read_byte 5.9±0.05ns 4.9±0.18ns -16.95%
deku_read_enum 4.7±0.04ns 5.3±0.03ns +12.77%
deku_read_vec 42.2±0.19ns 42.4±0.40ns +0.47%
deku_write_bits 189.9±3.28ns 214.1±4.45ns +12.74%
deku_write_byte 25.1±0.64ns 22.5±0.37ns -10.36%
deku_write_enum 22.8±0.23ns 21.3±0.32ns -6.58%
deku_write_vec 407.2±10.16ns 347.7±6.83ns -14.61%
read_all 3.8±0.05µs 5.6±0.04µs +47.37%
read_all_bytes 3.9±0.04µs 5.6±0.05µs +43.59%

@wcampbell0x2a wcampbell0x2a mentioned this pull request Oct 1, 2024
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024

Benchmark for eecec5e

Click to view benchmark
Test Base PR %
count 5.0±0.02µs 5.0±0.02µs 0.00%
deku_read_bits 1153.1±18.50ns 1162.9±8.75ns +0.85%
deku_read_byte 5.8±0.07ns 5.3±0.03ns -8.62%
deku_read_enum 5.3±0.05ns 3.9±0.06ns -26.42%
deku_read_vec 42.3±0.28ns 41.3±0.26ns -2.36%
deku_write_bits 185.9±3.73ns 219.1±4.41ns +17.86%
deku_write_byte 25.0±0.42ns 22.8±0.59ns -8.80%
deku_write_enum 22.8±0.38ns 21.5±0.29ns -5.70%
deku_write_vec 418.0±4.70ns 360.5±6.31ns -13.76%
read_all 5.6±0.04µs 3.8±0.07µs -32.14%
read_all_bytes 5.7±0.02µs 3.8±0.05µs -33.33%

@sharksforarms
Copy link
Owner

wow this looks great, I owe you a review. To set expectations I'm busy for a while w/ work so it may need to wait a bit, but I'm very excited about this

@wcampbell0x2a
Copy link
Collaborator Author

wow this looks great, I owe you a review. To set expectations I'm busy for a while w/ work so it may need to wait a bit, but I'm very excited about this

No problem!

@wcampbell0x2a
Add bit_order attribute
3d9a70a
@wcampbell0x2a
Add test for issue 485
954d1e3 
* This was fixed as part of this MR, not exactly sure what fixed it and
  don't have time to fully investigate
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 1, 2024

Benchmark for 182af0e

Click to view benchmark
Test Base PR %
count_from_u8_specialize 96.2±11.20ns 96.1±1.24ns -0.10%
count_no_specialize 2.6±0.02µs 2.4±0.02µs -7.69%
count_specialize 95.9±0.87ns 94.4±2.18ns -1.56%
deku_read_bits 1219.1±12.51ns 1209.0±15.61ns -0.83%
deku_read_byte 5.0±0.21ns 4.5±0.07ns -10.00%
deku_read_enum 4.1±0.23ns 3.8±0.07ns -7.32%
deku_read_vec 31.1±1.26ns 30.7±0.47ns -1.29%
deku_write_bits 195.6±7.90ns 239.3±4.62ns +22.34%
deku_write_byte 26.1±0.42ns 23.6±0.39ns -9.58%
deku_write_enum 23.9±0.20ns 22.6±0.35ns -5.44%
deku_write_vec 448.8±4.45ns 514.3±26.85ns +14.59%
read_all 5.1±0.08µs 4.2±0.05µs -17.65%
read_all_bytes 5.1±0.04µs 4.2±0.08µs -17.65%

sharksforarms
sharksforarms approved these changes Dec 3, 2024
View reviewed changes
Copy link
Owner

@sharksforarms sharksforarms left a comment

Choose a reason for hiding this comment

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

Thanks for your efforts here! Let's ship this.

deku-derive/src/macros/deku_read.rs
if (__deku_pad % 8) == 0 {
let bytes_read = __deku_pad / 8;
let mut buf = vec![0; bytes_read];
// TODO: use skip_bytes, or Seek in the future?
Copy link
Owner

@sharksforarms sharksforarms Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

would it be beneficial to address this TODO now or keep it a TODO?

edit: oops old review comment, I'm ok with leaving this as TODO and addressing later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sharksforarms sharksforarms sharksforarms approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Selecting bit order
2 participants
@wcampbell0x2a @sharksforarms