-
Notifications
You must be signed in to change notification settings - Fork 21
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
Feature/improved bitstream efficiency #66
base: main
Are you sure you want to change the base?
Feature/improved bitstream efficiency #66
Conversation
155e547
to
766773c
Compare
Hi @SirTates, thanks for this contribution. At first, we had doubts regarding performance. While it's obviously more efficient to mask complete (parts of) bytes into and out of a stream, instead of bit-by-bit, the most common operations in EXI are single-bit based. Nevertheless, a performance evaluation over our test base indeed confirmed your measurements: A two to fourfold improvement when writing streams, but no improvement (also no degradation) when reading. (Or was it vice versa, @chausGit?) The are two big BUTs:
|
|
Signed-off-by: Siebren Weertman <[email protected]> improve bitstream read efficiency Signed-off-by: Siebren Weertman <[email protected]> rework
766773c
to
27aa0cc
Compare
I rewrote the functions mostly from scratch (those bit manipulation parts have only few other ways to do the same, so I kept those, because that's what I spent most time on last time) I rebased all in one commit to remove reference of the potentially offending code. And I'm not sure whether I got the bit counting right this time. All my test streams go well and the length seems to not have changed from before and after my changes, but I couldn't replicate the same issue with the bit counting. If I knew what combination of wite_bits caused it I could add that test. |
What is the status here? @barsnick I think it's also okay if we don't accept PRs. |
I still think it's worth having. It significantly improves encode/decode speed. Though apparently there still are some issues:
If the latter is no longer an issue and you still want the change, then I can fix the former issue too. |
In this PR I'm improving the efficiency of the bitstream read/write by writing more than one bit at a time, if possible.
With a typical stream this makes the reading/writing ~2x faster (mostly due to the single bit writes being pretty much the same, strings, numbers and byte streams are typically 4x faster by my limited tests)
To test it was still working I employed this snippet: https://gist.github.com/SirTates/2527df0648e3dda3ae3cc27f0215ab85