Refactor Communication Classes #338
Conversation
Updated ALLLoadBalancer constructor parameters to use `const` references for efficiency and alignment with expected input types for the ALL library. This change improves code clarity and ensures proper handling of input arguments by maintaining consistency in data types.
- const - constexpr - rename variables - move declarations to usages - limit scope - reserve before push_back
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/parallel/NeighborAcquirer.cpp
Outdated
| // Before every set of push_backs make sure there is enough space for this set + all remaining. | ||
| // Work with the assumption that the others are of the same size as the current ones. | ||
| // This is potentially an overestimate but avoids a large number of resizes. |
There was a problem hiding this comment.
Is this assumption because the sizes of the regions decrease, so you can guarantee this is an overestimate? Or is this simply a guess? In which case, what is the reasoning behind the guess? Ultimately, I don't really mind if the reasoning is something as simple as "better than doing nothing but otherwise arbitrary," but the reasoning behind the choice should be somewhat more documented.
There was a problem hiding this comment.
The idea was to call reserve for every region and reserve the vector as if all following regions are of this region's size. Thus it would always be an overestimate since in the end the vector is reserved according to the biggest region. Calls to reserve with smaller sizes don't do anything.
Looking at the code now I realize that it is not doing what I intended... Let me create a fix...
There was a problem hiding this comment.
have a look at the updated doc -> c317648
Description
Some cleanups around communication helper classes.
Meant as preparation work for #321.
Related Pull Requests
Resolved Issues
Load Balancing is slower #321How Has This Been Tested?