Fixed unordered map clear #118
Merged
Conversation
This file contains 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
Bobobalink
force-pushed
the
fixed_unordered_map_clear
branch
from
720b11a
to
da264eb
Compare
Bobobalink
force-pushed
the
fixed_unordered_map_clear
branch
4 times, most recently
from
58b37d0
to
a4ad920
Compare
Bobobalink
force-pushed
the
fixed_unordered_map_clear
branch
from
a4ad920
to
ff96ac9
Compare
This was the source of a bug in FixedUnorderedMap for non-TriviallyCopyably types caused by retaining indices into the LinkedList's underlying storage, but the old copy/move logic would change the locations of the stored values. The new operations instead place the values at the same indices of the destination storage.
alexkaratarakis
force-pushed
the
fixed_unordered_map_clear
branch
from
ff96ac9
to
dfe4df0
Compare
This exercises edge cases in the copy and move constructors and assignment operators for value types that are nontrivial.
alexkaratarakis
force-pushed
the
fixed_unordered_map_clear
branch
from
dfe4df0
to
41f5073
Compare
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.
I ran some benchmarks on different clear options:
try_emplaceinto the new mapThe above graph was created with a "fresh" map, starting empty and then pushing each element once. This means the linked list is in perfect order so we don't see the locality issues of using linked lists. Instead, I generated a "used" linked list by repeatedly deleting and replacing subsets of the array, yielding the following results:
