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
Optimize Sieve Of Eratosthenes #50
base: main
Are you sure you want to change the base?
Conversation
purpl3F0x
commented
Apr 13, 2024
- Change return type to std::vector<uint8_t> to avoid performance regression of vector.
- Change input & indexes type to size_t
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your interest in AlgoPlus, a maintainer will see your PR soon!
- Change return type to std::vector<uint8_t> to avoid performance regression of vector<bool>. - Change input & indexes type to size_t
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
- Coverage 97.32% 89.92% -7.40%
==========================================
Files 95 95
Lines 3433 3247 -186
Branches 574 574
==========================================
- Hits 3341 2920 -421
Misses 92 92
- Partials 0 235 +235 |
Eh, i get it but i dont agree with the size_t conversion at all. I prefer int64_t over this. I dont think it's much of a change. |
Vector bool is bitset, each value is 1 bit not 1 byte. (Alternatively, valarray could also be used but I don't anyone likes valarray 🥲)
I don't think there is a reason the type should be signed, it only limits the range of the results. The function basically gets as an argument the max index of the vector which will return(which is a size_t) and then it does index maths,.all of these are size_t operations. I think the question is, if there is a reason not to be rather than being an int.(Ofc I would like to see one trying to allocate 1.8exabytes of RAM but I think my point is still valid) Hard coding the size to 64b will, Slow down architectures with words smaller than 64bits (eg ARM). Also possibly produce unwanted behavior, since I can pass 2^42 as an argument but the function will not return a vector of that size. |