Skip to content
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

CPU pipline parallel reverse bits algorithm #195

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from

Conversation

tryfinally
Copy link
Contributor

@tryfinally tryfinally commented Jun 13, 2020

algorithm is explained in the book Hackers Delight
You can see it also here Bit Twiddling Hacks at graphics.stanford.edu

@JohnSully
Copy link
Collaborator

Same as the other perf fix. Can you provide a scenario to repro the perf improvement?

We of course love perf improvements!

@tryfinally
Copy link
Contributor Author

I have no perf data on this. But this is old stuff, using bitwise ops and utilizing pipelining instead of a loop.

@JohnSully
Copy link
Collaborator

Can you update the comment at the top. If there’s a resource you used to get that algorithm ideally put that link there. Otherwise just delete it.

Once that’s in I should be ready to merge.

@JohnSully
Copy link
Collaborator

By the way if you want to work together on more changes let’s collaborate on https://community.keydb.dev/

@tryfinally
Copy link
Contributor Author

@JohnSully great, meanwhile am getting acquainted with the code, and these are just low hanging fruits that quickly identify base on my previous experience.

@JohnSully
Copy link
Collaborator

This one is still delayed on the comment fix.

@tryfinally
Copy link
Contributor Author

Hi @JohnSully what comment you need me to fix?
I have edited the comment in the commit on top and aslo the comment above the function in dict.c has the url.
another source is https://www.amazon.com/Hackers-Delight-2nd-Henry-Warren/dp/0321842685
chapter 7.
https://www.oreilly.com/library/view/hackers-delight/0201914654/0201914654_ch07lev1sec1.html

@JohnSully
Copy link
Collaborator

Hi @tryfinally The comment highlighted in blue. Since you have changed it perhaps you have not pushed to the source branch?

I'm preparing a new release which includes your earlier changes however this one will have to wait to the next release.

image

@tryfinally
Copy link
Contributor Author

done

@miroslavp
Copy link

done

x = (x & 0x55555555) << 1 | (x & 0xAAAAAAAA) >> 1;

Is this line removed by accident?

@tryfinally
Copy link
Contributor Author

@miroslavp yes it is by mistake. thanks!

@danog
Copy link

danog commented Mar 2, 2023

Clang offers a __builtin_bitreverse intrinsic, which on architectures like ARM directly calls the native rbit instruction; might be worth using it when compiling with clang.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants