perf: stop copying containers that are only walked - #821
Merged
Conversation
`ConnectedComponents::components` clones each group, in full, in order to iterate it by value and insert the elements into a set. The impl already requires `&It: IntoIterator`, so the group can be borrowed and only the elements that are kept get cloned. About 8% off 20 000 groups of 8. `kuhn_munkres` holds the x nodes of the alternating path in an `FxIndexSet`, but each root adds a given node at most once and the set is only ever cleared, appended to and iterated: nothing ever looks a node up. A `Vec` does the same without hashing. This one is a simplification rather than a speed-up; it does not measure as a change on a 250x250 assignment.
Member
|
That's fine as-is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #818.
Borrows the group in
ConnectedComponents::componentsrather than cloning it whole, and uses aVecinkuhn_munkreswhere anFxIndexSetwas only being appended to and iterated.Checking
290 tests pass, clippy and rustfmt clean.
Measured against this branch's parent, four passes with the order of the two binaries swapped on alternate passes:
components, 20 000 groups of 8: consistently faster in all four passes, between 4% and 13%, so about 10%kuhn_munkres, 250x250: the passes disagree on sign, so I am claiming nothing for it — it is a simplification, not a speed-upseparate_components, untouched by this change, measures unchanged, which is the controlHappy to drop the
kuhn_munkreshalf if you would rather this PR only carried the part with a number behind it.Measured on Windows 11, Intel Core Ultra 7 265K, 64 GB RAM.