ssbc / bipf

Binary json codec optimized for in-place access
MIT License
48 stars 9 forks source link

Upgrade varint library from 5 to 6 after PR is merged #21

Closed Barbarrosa closed 2 years ago

Barbarrosa commented 2 years ago

The new version includes bounds checking, which is nice. It would be best to wait for the upgrade until https://github.com/chrisdickinson/varint/pull/24 is merged so we can benefit from the decode performance optimization.

arj03 commented 2 years ago

nice performance optimization! :) Do you know what the breaking change is in 6?

Barbarrosa commented 2 years ago

@arj03 There's some bounds checking to prevent NaN and Infinity values from being returned or used by accident, so an error will be thrown in those cases.

Barbarrosa commented 2 years ago

I'm wondering if varint is abandoned now b/c the maintainer hasn't commented on the PR for over a month. The maintainer's still working on other Github projects, though.

arj03 commented 2 years ago

Latest release is a while ago. Maybe you can try and contact him on mail and see if he's willing to let you commit and do releases. I can help review.

Barbarrosa commented 2 years ago

I didn't get an email, but I sent a private message to an account where the maintainer appears to be active. At this point I doubt that I'm going to get a response. Do you have any thoughts about creating an alternate package or embedding the code here?

Barbarrosa commented 2 years ago

@arj03 Glad to see that you got an improvement in #37 by unrolling the loop. Did you have a chance to test with the Math.pow calls pre-calculated, or do you think that gets optimized?

arj03 commented 2 years ago

It is actually faster. Thanks :)