Faster LinkedHashMap tail() #2725
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
Change: LinkedHashMap tail() is now constant-time, from being O(size) time and space. Motivation: We have a use case which looks somewhat like the following. 1. It's effectively a Queue use case (fifo). 2. Iteration order needs to be stable. 3. Elements in the queue frequently need to be verified as present in the queue. 4. Occasionally, removals must occur, which are usually from the head of the queue although occasionally randomly. We started with a Queue and eventually found performance issues from too much linear scanning. This was not obviously going to be a bottleneck since our queues are usually of very small size, but ended up so. The next type we considered was a LinkedHashSet, using the `head` and `tail` methods to implement a dequeue-like operation and the set operations being sufficient for the other operations. Unfortunately, it seems that `tail` at present copies the entire queue despite not seemingly needing to. We moved to our own hand-crafted HashSet+Queue structure, but this PR seems like it'd generally be beneficial for users of Vavr!
danieldietrich
approved these changes
Nov 28, 2022
|
Thanks, Daniel! I appreciate the fast review + merge! When should I expect this code to be in a release? |
|
You're welcome. I can prepare a patch release this week! |
pivovarit
pushed a commit
that referenced
this pull request
Aug 19, 2024
Change: LinkedHashMap tail() is now constant-time, from being O(size) time and space. Motivation: We have a use case which looks somewhat like the following. 1. It's effectively a Queue use case (fifo). 2. Iteration order needs to be stable. 3. Elements in the queue frequently need to be verified as present in the queue. 4. Occasionally, removals must occur, which are usually from the head of the queue although occasionally randomly. We started with a Queue and eventually found performance issues from too much linear scanning. This was not obviously going to be a bottleneck since our queues are usually of very small size, but ended up so. The next type we considered was a LinkedHashSet, using the `head` and `tail` methods to implement a dequeue-like operation and the set operations being sufficient for the other operations. Unfortunately, it seems that `tail` at present copies the entire queue despite not seemingly needing to. We moved to our own hand-crafted HashSet+Queue structure, but this PR seems like it'd generally be beneficial for users of Vavr!
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Change: LinkedHashMap tail() no longer copies the entire datastructure.
Motivation: We have a use case which looks somewhat like the following.
We started with a Queue and eventually found performance issues from too much linear scanning. This was not obviously going to be a bottleneck since our queues are usually of very small size, but ended up so. The next type we considered was a LinkedHashSet, using the
headandtailmethods to implement a dequeue-like operation and the set operations being sufficient for the other operations. Unfortunately, it seems thattailat present copies the entire queue despite not seemingly needing to.We moved to our own hand-crafted HashSet+Queue structure, but this PR seems like it'd generally be beneficial for users of Vavr! It affects LinkedHashMap, and because LinkedHashSet contains a LinkedHashMap, LinkedHashSet, too.