Skip to content

Make .retain() in-order and preserve-order #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jan 3, 2018
Merged

Make .retain() in-order and preserve-order #56

merged 4 commits into from
Jan 3, 2018

Conversation

bluss
Copy link
Member

@bluss bluss commented Jan 2, 2018 •
edited
Loading

  • Having a retain that keeps the order is very useful
  • This is slower across the board but especially if we retain most of the map; the swap_remove version (unordered version) just moves much less values around, even less than a regular Vec::retain.
  • This PR uses a trick (tombstones in self.indices!) for the duration of the retain method only

@bluss
FEAT: Make .retain() in-order and preserve-order
929d459 
We want to visit the elements in order, and keep them in order after the
retain; the complicated part is removing them and backward shifting them
correctly.

The new version is a bit slower than HashMap::retain and than the old
unordered retain, so we keep around the unordered version too for those
that accept jumbling up the order.
@bluss bluss force-pushed the retain-in-place-4 branch 2 times, most recently from 8b8660f to ba42e9e Compare January 2, 2018 21:18
@bluss bluss mentioned this pull request Jan 2, 2018
Closed
8 tasks
bluss added 2 commits January 3, 2018 20:45
@bluss
TEST: Add test for order-preserving retain
5c5cd22
@bluss
FEAT: Simplify find_existing_entry_at
ff0757e 
This simplifies the loop in find_existing_entry_at a little bit
@bluss bluss force-pushed the retain-in-place-4 branch from 3f6727f to ff0757e Compare January 3, 2018 19:45
@bluss bluss merged commit 13ca3e2 into master Jan 3, 2018
@bluss bluss deleted the retain-in-place-4 branch January 3, 2018 22:51
@bluss bluss mentioned this pull request Jan 25, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@bluss