-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Optimize list subtraction (A -- B) and make it yield on large inputs #1998
Optimize list subtraction (A -- B) and make it yield on large inputs #1998
Conversation
5dd398f
to
655ef4d
Compare
product of the length of its operands, which was extremely slow on long | ||
lists.</p> | ||
|
||
<p>These days the run-time complexity is "n log n" and the operation will |
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.
"These days". I suggest making it more specific, e.g. from "OTP 22".
lists.</p> | ||
|
||
<p>These days the run-time complexity is "n log n" and the operation will | ||
complete quickly even on very long lists. In fact, it's faster and uses |
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.
"it's": OTP's documentation guidelines says that contraction should be avoided.
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.
Thanks, fixed!
4f4b5da
to
2643f64
Compare
The first stage wasn't bounded by reductions, and it bumped far more reductions than it should have due to a logic bug.
2643f64
to
b719424
Compare
This greatly increases the performance of '--'/2 which does a lot of term comparisons.
44e6530
to
127e78f
Compare
The removal set now uses a red-black tree instead of an array on large inputs, decreasing runtime complexity from `n*n` to `n*log(n)`. It will also exit early when there are no more items left in the removal set, drastically improving performance and memory use when the items to be removed are present near the head of the list. This got a lot more complicated than before as the overhead of always using a red-black tree was unacceptable when either of the inputs were small, but this compromise has okay-to-decent performance regardless of input size. Co-authored-by: Dmytro Lytovchenko <dmytro.lytovchenko@erlang-solutions.com>
127e78f
to
eb9ee88
Compare
@kvakvs had to pause his work on #1993 so this PR takes over where he left off. I've refactored it to make state handling easier to follow, and started using a tree instead of an array for the removal set, decreasing run-time complexity from
n*n
ton*log(n)
.Other than that it's pretty much the same, and it should still be relatively easy to backport.