Hash_map: Implement erase method.#6493
Closed
GilesBathgate wants to merge 10 commits intoCGAL:mainfrom
Closed
Conversation
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.
Summary of Changes
When looking at SNC_structure from the Nef_3 package, I noticed an unusual design that wraps
Object_iteratorin aboost::optionalI have come to believe that the reason for the design is to work around the fact that Unique_hash_map/chained_map implementation does not support an erase function. Instead of removing the Object_iterator from the hashmap, its optional wrapper is set toboost::none. In theory, this design could have a memory leak, as adding and removing boundary items would only add optional items. I am not sure if this is the case in practice.Either way, I decided to attempt to implement erase for the hashmap. One of the problems that I ran into was managing the so-called
freepointer since the existing code needs to be able to increment it to move to the next available memory, but the erased element could exist below the free pointer's location. One approach would be to move the erased element to the location of the free pointer and decrement the free pointer, but this was found to be slow, as it required unlinking anything that pointed to thefreenode.Instead, I had the idea of a "free chain" which builds on the single linked lists of the existing code. I think this idea could be novel, but I am not very well versed in hashmap literature. Essentially the idea is to build a chain of the overflow memory, and instead of incrementing free, an iterator function
free = free->succis used. The advantage of this approach is that when memory needs to be returned to the overflow memory viaerase, it is simply a case of inserting it back into the free chain.The pull request also adds a
containsmethod (wrapper for is_defined) and uses the methods within SNC_structure/Sphere_map. Usage of std::move was added to erase and rehash.Testing
Release Management