Add colony data structure #31338
Merged
Add colony data structure #31338
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
ifreund
added
[C++]
ifreund
commented
Jun 11, 2019
| #define COLONY_DESTROY(the_allocator, allocator_instance, location) std::allocator_traits<the_allocator>::destroy(allocator_instance, location) | ||
| #define COLONY_ALLOCATE(the_allocator, allocator_instance, size, hint) std::allocator_traits<the_allocator>::allocate(allocator_instance, size, hint) | ||
| #define COLONY_ALLOCATE_INITIALIZATION(the_allocator, size, hint) std::allocator_traits<the_allocator>::allocate(*this, size, hint) | ||
| #define COLONY_DEALLOCATE(the_allocator, allocator_instance, location, size) std::allocator_traits<the_allocator>::deallocate(allocator_instance, location, size) |
There was a problem hiding this comment.
Could get rid of these now too, trying to decide if its worth the effort...
|
I'm in the process of porting the test suite from plf over to our catch2 testing environment. Once that is finished, this should be ready to go. |
ifreund
force-pushed
the
colony
branch
3 times, most recently
from
a28ab96 to
863317e
Compare
|
Alright, we now have 1000 lines of test cases making sure this works as expected. I'm calling this done, barring additional code cleanup suggestions. |
- colony basics (simple insertion/erasure/iterator testing) - insert and erase - range erase - sort - insertion methods - perfect forwarding - emplace - group size and capacity - splice Delete `group_size_sum()` from colony as it is only needed for debugging
Correction to reserve() semantics. Reserve() also will no longer assert that reserve amount is within bounds of maximum and minimum group sizes, and will silently round the value down or up accordingly. Please consult the documentation to understand the current limitations of reserve() for colony.
|
plflib just released a small update, so I applied that too. |
This was referenced Jun 14, 2019
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.
Summary
SUMMARY: Infrastructure "Add colony data structure."
Purpose of change
The intent is to used this data structure to replace how we store items on the map.
There are several issues with using our current data structure of an
std::list. In particular, removal of elements from the "middle" of the list invalidates references and iterators to elements after the one removed. This makes the item management code convoluted, and in some cases severely technically limited.Describe the solution
Introduce a new data structure,
colonythat has the following major advantages overstd::list:You can read about all the details and more about the justification here: https://www.plflib.org/colony.htm
This implementation is taken directly from https://github.com/mattreecebentley/plf_colony and modified to fit into our code better
This pull request does not yet reimplement
item_stackusing a colony, that will come in a subsequent PR. I just wanted to get the data structure/implementation itself up for formal review.Additional context
I'm not quite done with this, I intend to remove the preprocessor directives for supporting older c++ compilers/versions than we support to further clean up the code.
Also, it would be probably be polite to acknowledge that we are using a modified version of the PLF implementation somewhere in the project documentation, but I wasn't sure where best to put this. Will defer to @kevingranade on that.