quininer / cbor4ii

CBOR: Concise Binary Object Representation
MIT License
54 stars 5 forks source link

Release 0.3.0 #23

Closed quininer closed 1 year ago

vmx commented 2 years ago

I see that you're changing the errors a bit. One thing I've on my todo list (which I haven't found the time to do yet :( is trying to add the offset to the error messages. When decoding it's really helpful to see where it happened. But that could also be something that can be added later. I just want to bring it up, in case it makes sense to do it for the 0.3.0 release.

quininer commented 2 years ago

I thought about adding an offset, but most it's not needed and it would increase the size of error type. for this PR, I intentionally optimized the size of error so that it is only 2 word on 64bit platform.

It would be better to have a custom Read record the offset, and get it on error.

like

struct Reader(..);

impl Reader {
    pub fn offset(&self) -> usize;
}

let mut reader = Reader::new();

if let Err(err) = Target::decode(&mut reader) {
    println!("error: {:?} offset:{}", err, reader.offset());
}
quininer commented 2 years ago

The BytesSequence based decode_stack_cow enables short strings called via from_reader to be allocated on stack, but it is slower for from_slice.

serde_cbor-de           time:   [708.60 ns 714.74 ns 722.99 ns]
                        change: [+0.0567% +0.9269% +1.7664%] (p = 0.03 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) high mild
  2 (2.00%) high severe

cbor4ii-de              time:   [794.11 ns 801.12 ns 809.81 ns]
                        change: [-3.0797% -1.7441% -0.3062%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 11 outliers among 100 measurements (11.00%)
  2 (2.00%) high mild
  9 (9.00%) high severe

This is not acceptable and I need more time to optimize it.

quininer commented 1 year ago

Since almost all my use cases are for &[u8], I have lost interest in optimizing BytesSequence for time being. implementing it in future won't break API, so I decided not to block 0.3.