Optimize BTreeMap::append() using CursorMut#153107
Open
asder8215 wants to merge 1 commit intorust-lang:mainfrom
Open
Optimize BTreeMap::append() using CursorMut#153107asder8215 wants to merge 1 commit intorust-lang:mainfrom
asder8215 wants to merge 1 commit intorust-lang:mainfrom
Conversation
Contributor
Author
|
I'd be curious to see if this shows any noticeable performance difference with the current implementation of |
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
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.
Since
BTreeMap::mergeusesCursorMutto avoid reconstructing the map from scratch and instead inserting otherBTreeMapat the right places or overwriting the value in selfBTreeMapon conflict, we might as well do the same forBTreeMap::append. This also means that some of the code inappend.rscan be removed;bulk_push()however is used bybulk_build_from_sorted_iterator(), which is used by theFrom/FromIteratortrait impl onBTreeMap. Feels like we should rename the file or place thebulk_push()in an existing file.The same additional optimization consideration that
BTreeMap::mergehas is also applied toBTreeMap::append.r? @Mark-Simulacrum since Mark has seen the
BTreeMap::mergecode already (only diff is theOrdering::Equalcase and now one of the test assertions on a panic case has the correct value now).