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

Usage of bit masks and some code improvements #13

Open
helderduartemop opened this issue Feb 11, 2021 · 6 comments
Open

Usage of bit masks and some code improvements #13

helderduartemop opened this issue Feb 11, 2021 · 6 comments

Comments

@helderduartemop
Copy link

I've been looking at your code and noticed some improvements that can be made:
Replacing all divisions of factors of 2 by Shift Right operations.
Replacing all modulus operators of factors of 2 by bitmasks.
Remove the swap_two function, using it on the spot, as it only currently appears in a total of 4 instances throughout the code.
Around line 300, if you use pts[0] instead pts[cnt++] and initialize cnt = 5 you save some operations, reducing the computational intensity. Usage of constants whenever possible is always preferable for speedup!
I will fork and submit a version later.

@scandum
Copy link
Owner

scandum commented Feb 11, 2021

Thanks for checking it out. :)

As for bit operations, I decided not to use them because the performance gain was minimal or nonexistent while readability suffered.

Good call about using constants in that section, I'll go ahead and change that upstream since there've been a bunch of other changes since my last commit. I'll push out some updates in a few.

@helderduartemop
Copy link
Author

If you want, I can help you with reviewing code and ideas for the project as well. I feel like there's still some room for improvement :)

@scandum
Copy link
Owner

scandum commented Feb 12, 2021

There's probably not much meat left on the bone, but I'm open to suggestions. I'll try to run some benchmarks in the next few days to check if a similar use of constants in the quad_swap routine is worth it.

@helderduartemop
Copy link
Author

helderduartemop commented Feb 12, 2021

I have uploaded an updated version of the code with your latest changes.
At the quad_swap function, instructions like if (cmp(&pta[0], &pta[2]) <= 0) became if (pta[2] > pta[0]), effectively reducing the amount of comparisons the code has to make.

I've tried other changes too but since they happened on code that only happened once, they seemed kind of pointless to me.

@scandum
Copy link
Owner

scandum commented Feb 12, 2021

The cmp() call is needed however for string comparison support and qsort() compatibility.

@scandum
Copy link
Owner

scandum commented May 2, 2022

Quadsort changed quite a bit since this discussion. Anyone interested in micro optimizations might want to have a look at:

https://github.com/scandum/tinysort

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

No branches or pull requests

2 participants