-
Notifications
You must be signed in to change notification settings - Fork 364
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.
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
Allow Merge Queue To Handle Larger Inputs #2400
Conversation
Signed-off-by: Matthew Eldridge <[email protected]>
Signed-off-by: Matthew Eldridge <[email protected]>
Signed-off-by: Matthew Eldridge <[email protected]>
It would be interesting to see how this method now performs.
Old runs are in |
Before
After
|
|
||
class MergeQueue(initialSize: Int = 1) { | ||
private val treeSet = new java.util.TreeSet(new RangeComparator()) // Sorted data structure | ||
private val fudge = 1 // Set this to one to preserve original behavior |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this mean? What is the original behavior vs the non-original behavior, and why did it change?
Consider a more clear name for the variable.
Ah yes, I realized that I used the phrase "preservation of previous behavior" in two unrelated ways. The first refers to the fact that in the original code, not just overlapping intervals but intervals that have a gap between them of up to one unit are merged. The second usage refers to the fact that one can request the merged intervals more than once, so a the data structure needs to be preserved. I will make an update in a few moments. |
These changes simplify
MergeQueue
and improve its efficiency. Thank you to @mteldridge for noticing this issue.Connects #2358