-
Notifications
You must be signed in to change notification settings - Fork 6
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
Boundcheck #13
base: main
Are you sure you want to change the base?
Boundcheck #13
Conversation
Check the speed @ronanh. 🚀 |
I'm running the benchmarks, will check the speed. Take some rest, I cannot keep up ;-) |
@pascaldekloe I'm having mixed feelings with this one. There's quite a lot of efforts into it, and it improves readability. But there's also a lot of stuff and it's difficult to see what's beneficial in terms of performance and what is not. The first commit
for the whole thing:
There's obvious good things: input arrays, changing order of parameters, remove panic in switch. But you also went the append route, that I wanted to avoid in the original design:
I realised that the strategy to recompress small inputs had drawbacks #15 and will likely want to use the place where buffer is grown to make clever things to recompress small blocks. Also, I don't understand your comments about panics, am I missing something? |
I have pushed a branch input-arrays where I have included those changes: input arrays, changing order of parameters, remove panic in switch. Will run benchmarks on it to compare. |
Another approach that might be performant would be to assign the whole array in one go from a big literal in the generate code... |
No worries. I started my own implementation. Let's compare and see next week. Our implementations won't be compatible, as I reversed the delta to optimise the increment into single bit packs. |
Looks promising. The speed will be good. |
Not sure about the optimal balance for the compression limit yet. The main objective was code/instruction reduction. The |
Can also eliminate the zero compression as there is always a way to trim one word when including overflows as an option for deltas. Won't be fast, but maybe that's OK? |
Minimise bounds checks and panics with input arrays and append.