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

Support multiple fields to id #315

Merged
merged 2 commits into from
Feb 28, 2023
Merged

Conversation

wcampbell0x2a
Copy link
Collaborator

@wcampbell0x2a wcampbell0x2a commented Jan 19, 2023

  • Add use of multiple id's from ctx in enums by expanding the match
    statement to tuples

@wcampbell0x2a
Copy link
Collaborator Author

wcampbell0x2a commented Jan 19, 2023

deku::rest is currently not expanded for ctx, not sure how to fix this.

Never-mind, this doesn't work considering DekuWrite.

error[E0425]: cannot find value `__deku_rest` in this scope
  --> examples/enums.rs:20:22
   |
20 |         #[deku(ctx = "*my_id, deku::rest.len()")]
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope

@github-actions
Copy link

Benchmark for 45ad68a

Click to view benchmark
Test Base PR %
deku_read_bits 831.3±10.01ns 828.5±14.65ns -0.34%
deku_read_byte 23.9±0.51ns 24.0±0.43ns +0.42%
deku_read_enum 39.3±0.72ns 40.2±0.51ns +2.29%
deku_read_vec 1946.1±43.63ns 1925.7±48.50ns -1.05%
deku_read_vec_perf 1954.7±35.82ns 1958.0±41.23ns +0.17%
deku_write_bits 152.7±3.12ns 153.5±4.62ns +0.52%
deku_write_byte 82.5±1.61ns 82.9±2.32ns +0.48%
deku_write_enum 139.9±3.53ns 141.3±2.08ns +1.00%
deku_write_vec 6.3±0.14µs 6.3±0.15µs 0.00%
deku_write_vec_perf 6.4±0.19µs 6.3±0.11µs -1.56%

@wcampbell0x2a wcampbell0x2a force-pushed the add-multiple-id-for-ctx branch from 735d484 to 1ec9b25 Compare January 20, 2023 04:01
@wcampbell0x2a wcampbell0x2a changed the title Add multiple fields to id Support multiple fields to id Jan 20, 2023
- Add use of multiple id's from ctx in enums by expanding the match
  statement to tuples
@wcampbell0x2a wcampbell0x2a force-pushed the add-multiple-id-for-ctx branch from 1ec9b25 to d10ce5e Compare January 20, 2023 04:05
@github-actions
Copy link

Benchmark for f7ed986

Click to view benchmark
Test Base PR %
deku_read_bits 625.2±0.54ns 624.4±1.65ns -0.13%
deku_read_byte 18.1±0.13ns 17.9±0.09ns -1.10%
deku_read_enum 31.1±0.55ns 31.6±0.25ns +1.61%
deku_read_vec 1460.7±8.40ns 1434.3±7.27ns -1.81%
deku_read_vec_perf 1441.4±8.92ns 1445.3±6.40ns +0.27%
deku_write_bits 106.6±0.21ns 106.5±0.20ns -0.09%
deku_write_byte 62.8±0.29ns 62.8±0.22ns 0.00%
deku_write_enum 108.2±0.43ns 108.8±0.84ns +0.55%
deku_write_vec 5.1±0.03µs 5.1±0.03µs 0.00%
deku_write_vec_perf 5.2±0.01µs 5.0±0.01µs -3.85%

@github-actions
Copy link

Benchmark for c778eed

Click to view benchmark
Test Base PR %
deku_read_bits 624.2±0.51ns 624.8±0.51ns +0.10%
deku_read_byte 18.0±0.07ns 18.1±0.13ns +0.56%
deku_read_enum 31.1±0.26ns 31.3±0.34ns +0.64%
deku_read_vec 1439.2±7.06ns 1463.3±12.55ns +1.67%
deku_read_vec_perf 1444.4±7.61ns 1441.1±7.08ns -0.23%
deku_write_bits 106.5±0.26ns 107.1±2.41ns +0.56%
deku_write_byte 62.7±0.28ns 62.8±0.27ns +0.16%
deku_write_enum 108.3±0.58ns 108.5±0.42ns +0.18%
deku_write_vec 5.1±0.03µs 5.1±0.01µs 0.00%
deku_write_vec_perf 5.3±0.03µs 5.0±0.01µs -5.66%

@@ -105,36 +105,72 @@ fn test_top_level_ctx_enum_default() {
#[test]
fn test_struct_enum_ctx_id() {
#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
#[deku(ctx = "my_id: u8", id = "my_id")]
#[deku(ctx = "my_id: u8, data: usize", id = "my_id, data")]
Copy link
Owner

Choose a reason for hiding this comment

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

We might want to duplicate the test? (one tuple, one non-tuple)

@github-actions
Copy link

Benchmark for ae7d5c9

Click to view benchmark
Test Base PR %
deku_read_bits 622.2±0.57ns 622.3±3.69ns +0.02%
deku_read_byte 17.8±0.08ns 17.7±0.09ns -0.56%
deku_read_enum 31.9±0.20ns 31.4±0.20ns -1.57%
deku_read_vec 1442.2±7.92ns 1456.8±8.35ns +1.01%
deku_read_vec_perf 1456.0±7.00ns 1450.8±9.36ns -0.36%
deku_write_bits 110.4±0.44ns 110.1±0.46ns -0.27%
deku_write_byte 68.4±0.29ns 67.6±1.07ns -1.17%
deku_write_enum 111.7±0.44ns 113.4±0.80ns +1.52%
deku_write_vec 5.2±0.01µs 5.2±0.01µs 0.00%
deku_write_vec_perf 5.4±0.02µs 5.2±0.01µs -3.70%

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! 🙏

@wcampbell0x2a wcampbell0x2a merged commit dcd0d6b into master Feb 28, 2023
@wcampbell0x2a wcampbell0x2a deleted the add-multiple-id-for-ctx branch February 28, 2023 12:30
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