sharksforarms / deku

Declarative binary reading and writing: bit-level, symmetric, serialization/deserialization
Apache License 2.0
1.15k stars 55 forks source link

Impl reader+writer seek #360

Closed wcampbell0x2a closed 2 months ago

wcampbell0x2a commented 1 year ago

See https://github.com/sharksforarms/deku/issues/105

Currently being used in https://github.com/wcampbell0x2a/cpio-deku

github-actions[bot] commented 1 year ago

Benchmark for c05f1ba

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | 784.1±1.34ns | 784.1±1.36ns | 0.00% | | deku_read_byte | 23.5±0.11ns | 23.5±0.08ns | 0.00% | | deku_read_enum | 10.1±0.08ns | 10.1±0.07ns | 0.00% | | deku_read_vec | **65.4±0.34ns** | 68.2±0.77ns | **+4.28%** | | deku_write_bits | **105.5±0.39ns** | 105.9±1.16ns | **+0.38%** | | deku_write_byte | 164.3±0.91ns | **163.5±0.76ns** | **-0.49%** | | deku_write_enum | 105.8±0.35ns | 105.8±0.31ns | 0.00% | | deku_write_vec | 4.0±0.01µs | 4.0±0.01µs | 0.00% |
github-actions[bot] commented 11 months ago

Benchmark for f9655f8

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1253.9±14.83ns** | 1293.6±23.46ns | **+3.17%** | | deku_read_byte | 21.8±0.59ns | 21.8±0.54ns | 0.00% | | deku_read_enum | 9.5±0.16ns | 9.4±0.15ns | -1.05% | | deku_read_vec | 59.5±0.65ns | **58.6±0.82ns** | **-1.51%** | | deku_write_bits | **142.6±5.32ns** | 225.8±16.05ns | **+58.35%** | | deku_write_byte | 142.1±4.61ns | **20.9±0.36ns** | **-85.29%** | | deku_write_enum | 84.4±2.43ns | **19.6±0.33ns** | **-76.78%** | | deku_write_vec | 3.3±0.07µs | **300.4±4.12ns** | **-90.90%** |
github-actions[bot] commented 11 months ago

Benchmark for 84fd5f3

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1116.3±18.52ns** | 1197.5±16.33ns | **+7.27%** | | deku_read_byte | **21.6±0.49ns** | 22.2±1.48ns | **+2.78%** | | deku_read_enum | 9.3±0.15ns | 9.3±0.13ns | 0.00% | | deku_read_vec | 53.5±0.59ns | 53.9±0.51ns | +0.75% | | deku_write_bits | **180.7±3.25ns** | 188.7±21.84ns | **+4.43%** | | deku_write_byte | 21.2±2.63ns | **20.8±0.23ns** | **-1.89%** | | deku_write_enum | 20.2±0.36ns | 20.1±0.18ns | -0.50% | | deku_write_vec | 298.2±5.20ns | 296.1±3.47ns | -0.70% |
github-actions[bot] commented 6 months ago

Benchmark for e6c5a67

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1229.2±16.47ns** | 1261.1±14.49ns | **+2.60%** | | deku_read_byte | 3.3±0.12ns | 3.3±0.08ns | 0.00% | | deku_read_enum | 2.6±0.07ns | 2.6±0.04ns | 0.00% | | deku_read_vec | 33.7±0.47ns | 33.4±0.33ns | -0.89% | | deku_write_bits | 149.5±4.52ns | 149.0±2.81ns | -0.33% | | deku_write_byte | 21.8±0.51ns | 22.0±0.31ns | +0.92% | | deku_write_enum | **21.1±0.31ns** | 21.4±0.26ns | **+1.42%** | | deku_write_vec | **351.9±3.46ns** | 405.2±5.23ns | **+15.15%** |
github-actions[bot] commented 6 months ago

Benchmark for cee1fd3

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | 1289.2±17.71ns | **1219.4±15.03ns** | **-5.41%** | | deku_read_byte | **3.3±0.07ns** | 3.4±0.29ns | **+3.03%** | | deku_read_enum | 2.6±0.06ns | 2.6±0.03ns | 0.00% | | deku_read_vec | 34.5±0.40ns | 34.3±0.33ns | -0.58% | | deku_write_bits | **148.8±2.97ns** | 155.7±3.30ns | **+4.64%** | | deku_write_byte | 22.0±0.29ns | 21.9±0.40ns | -0.45% | | deku_write_enum | 21.1±0.26ns | 21.2±0.79ns | +0.47% | | deku_write_vec | 408.3±5.52ns | **354.0±3.25ns** | **-13.30%** |
github-actions[bot] commented 6 months ago

Benchmark for 2a61000

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1222.2±15.48ns** | 1250.6±43.96ns | **+2.32%** | | deku_read_byte | 3.5±0.07ns | **3.3±0.03ns** | **-5.71%** | | deku_read_enum | 3.0±0.05ns | **2.6±0.06ns** | **-13.33%** | | deku_read_vec | 34.9±0.67ns | 34.4±0.62ns | -1.43% | | deku_write_bits | **157.5±3.33ns** | 235.5±3.68ns | **+49.52%** | | deku_write_byte | **22.0±0.22ns** | 22.6±0.29ns | **+2.73%** | | deku_write_enum | 21.4±0.37ns | 21.4±0.31ns | 0.00% | | deku_write_vec | 423.2±4.81ns | **417.7±24.23ns** | **-1.30%** |
github-actions[bot] commented 6 months ago

Benchmark for d862393

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | 1283.7±14.74ns | 1276.0±56.88ns | -0.60% | | deku_read_byte | 3.5±0.06ns | **3.3±0.05ns** | **-5.71%** | | deku_read_enum | 3.0±0.05ns | **2.6±0.05ns** | **-13.33%** | | deku_read_vec | 33.7±0.64ns | 33.6±0.62ns | -0.30% | | deku_write_bits | 154.8±4.91ns | **148.6±6.87ns** | **-4.01%** | | deku_write_byte | 21.8±0.28ns | 21.6±0.40ns | -0.92% | | deku_write_enum | 21.1±0.44ns | 21.1±0.31ns | 0.00% | | deku_write_vec | 424.7±17.49ns | **350.7±4.28ns** | **-17.42%** |
github-actions[bot] commented 6 months ago

Benchmark for 26c8d2a

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1234.8±24.41ns** | 1289.0±9.75ns | **+4.39%** | | deku_read_byte | 3.5±0.07ns | **3.3±0.05ns** | **-5.71%** | | deku_read_enum | 3.0±0.05ns | **2.6±0.05ns** | **-13.33%** | | deku_read_vec | 34.4±0.58ns | **33.5±0.31ns** | **-2.62%** | | deku_write_bits | **149.2±3.32ns** | 157.6±3.18ns | **+5.63%** | | deku_write_byte | **21.9±0.20ns** | 24.5±0.37ns | **+11.87%** | | deku_write_enum | **21.1±0.34ns** | 23.0±0.37ns | **+9.00%** | | deku_write_vec | **404.8±5.36ns** | 439.4±5.98ns | **+8.55%** |
vhdirk commented 6 months ago

This would be pretty useful in my project. Anything I can assist with to get this merged?

wcampbell0x2a commented 6 months ago

This would be pretty useful in my project. Anything I can assist with to get this merged?

We need https://github.com/no-std-io/no-std-io/pull/5 and a final review by me and @sharksforarms

wcampbell0x2a commented 2 months ago

This would be pretty useful in my project. Anything I can assist with to get this merged?

We need no-std-io/no-std-io#5 and a final review by me and @sharksforarms

Decided to just fork that repo and use my own no-std-io2 for now, since that wasn't getting merged and i've tried contacting them a couple of times.

github-actions[bot] commented 2 months ago

Benchmark for 830408c

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | **1157.1±16.19ns** | 1197.3±10.14ns | **+3.47%** | | deku_read_byte | 3.3±0.15ns | 3.3±0.02ns | 0.00% | | deku_read_enum | **2.5±0.08ns** | 2.6±0.06ns | **+4.00%** | | deku_read_vec | **33.7±0.21ns** | 34.5±0.25ns | **+2.37%** | | deku_write_bits | 187.9±4.96ns | **181.1±8.26ns** | **-3.62%** | | deku_write_byte | **22.7±0.43ns** | 25.2±0.26ns | **+11.01%** | | deku_write_enum | **21.7±0.30ns** | 23.0±0.19ns | **+5.99%** | | deku_write_vec | **387.4±3.96ns** | 404.9±4.90ns | **+4.52%** |
github-actions[bot] commented 2 months ago

Benchmark for fe7abff

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | deku_read_bits | 1185.2±9.81ns | 1176.5±15.95ns | -0.73% | | deku_read_byte | 3.3±0.04ns | 3.3±0.09ns | 0.00% | | deku_read_enum | **2.5±0.05ns** | 2.6±0.05ns | **+4.00%** | | deku_read_vec | 33.7±0.39ns | 33.9±0.49ns | +0.59% | | deku_write_bits | 181.5±7.18ns | 181.2±3.77ns | -0.17% | | deku_write_byte | **22.7±0.33ns** | 25.2±0.58ns | **+11.01%** | | deku_write_enum | **21.7±0.39ns** | 22.7±0.36ns | **+4.61%** | | deku_write_vec | **390.7±3.67ns** | 431.1±6.61ns | **+10.34%** |
wcampbell0x2a commented 2 months ago

Thanks! This is great. This got me thinking, it would be pretty cool if there was a way to add attributes like this without adding bounds to the generic, such that "seek_" could only be used if T is Seek, else it would be a compile error. This way, we don't impose more bounds than necessary?

The downside of this PR is that now types are now required to be Seek (maybe less of a concern with the reader interface... but I can for-see some folks not liking this, maybe?)

I'm not sure about any solutions that includes generic bounds, I think you'll always need to have Seek as a bounds on Reader, unless i'm wrong.

the friends at binrw 'solve' this by having a NoSeek

sharksforarms commented 2 months ago

the friends at binrw 'solve' this by having a NoSeek

Ah! Indeed. It was likely enough of a pain point to include something in the library.