Optimize IdList by Using an Index and a Proper Iterator#1002
Optimize IdList by Using an Index and a Proper Iterator#1002phkahler merged 4 commits intosolvespace:masterfrom
Conversation
dc60401 to
3c6e2be
Compare
|
I am really happy that this is being worked on, and I tested it out and it really improved speed a lot for complex things. I did get some warnings when compiling this (this is one of many but they all look like the same thing in different places): I cannot make any other comments to the code since it is way above my head. In short, it works over here! =) |
Thanks for taking a look!
Hmm yes this is an obscure one: https://stackoverflow.com/questions/1828037/whats-the-point-of-g-wreorder |
|
Yup that fixed those warnings! I think it should probably have shown up in the macOS logs (I got those with clang on macOS at least) |
- Use `std::vector<T> elemstore` to store elements. Avoids manual memory management. - Add and index (`std::vector<int> elemidx`) that avoids moving large objects when adding elements. - Add a free element list (`std::vector<int> freelist`) that speeds up element removal by avoiding rearranging the element storage. It also avoids reallocations when adding elements later. - Add a proper iterator. It will be used to remove NextAfter - which is a performance bottleneck.
Based on commit '3b395bb5a7' by pjanx Resolves a performance regression of iteration being O(n * log n) rather than O(n).
|
So is this "better" than #992? Are either of them complete? I haven't been following this since it wasn't going in 3.0. |
|
I think it is a bit cleaner than #992 since it uses std:vector. It is also ever so slightly faster (see here #992 (comment)) probably because I have a It is complete. The tests pass. It would be nice if a few people do a code review and run the address sanitizers on Linux. When I get to my other system I will try on Windows with clang and sanitizers as well. I did try adding an extra std::unordered_map to map between handles and indexes to speed up search from O(log(n)) but it did not make an overall speed difference in the tests I tried. It just shifted which functions are on the the critical path. But this can always be added later. |
|
Is this a win over #992 then? I'd like to settle on one of these soon. |
By definition I'm biased ;-) Others should review both. |
|
Merging this version. Let's get some real usage to test it. I'm a bit concerned about bugs, but there's not much reason for people to be running edge builds right now. |
This drastically speeds up IdList operations. Closes #932 and #939 and even the remainder of #841.
It is similar to #992 and Evil-Spirit@44eb1a4
It optimizes IdList by Using an Index and a Proper Iterator:
std::vector<T> elemstoreto store elements. Avoids manual memory management.std::vector<int> elemidx) that avoids moving large objects when adding elements.std::vector<int> freelist) that speeds up element removal by avoiding rearranging the element storage. It also avoids reallocations when adding elements later.O(n*log n)rather thanO(n).