-
Notifications
You must be signed in to change notification settings - Fork 34
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
Mas d34 ms.i446 plusplus #448
Merged
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
Try to reduce the calls to ++, and ensure that where possible the shorted list is being copied.
So that the list can be accumulated efficiently, without an additional copy to add back the accumulator at the end.
Code review to make sure we prepend to accumulators everywhere, to reduce the copying involved. attempt to further optimise in leveled_sst (where most expensive ++ occurs). This optimises for the case when Acc is [], and enforces a series of '++' to start from the right, prepending in turn. Some shell testing indicated that this was not necessarily the case (although this doesn't seem tobe consistently reproducible). ``` 6> element(1, timer:tc(fun() -> KL1 ++ KL2 ++ KL3 ++ KL4 end)). 28 7> element(1, timer:tc(fun() -> KL1 ++ KL2 ++ KL3 ++ KL4 end)). 174 8> element(1, timer:tc(fun() -> KL1 ++ KL2 ++ KL3 ++ KL4 end)). 96 9> element(1, timer:tc(fun() -> KL1 ++ KL2 ++ KL3 ++ KL4 end)). 106 10> element(1, timer:tc(fun() -> KL1 ++ KL2 ++ KL3 ++ KL4 end)). 112 17> element(1, timer:tc(fun() -> lists:foldr(fun(KL0, KLAcc) -> KL0 ++ KLAcc end, [], [KL1, KL2, KL3, KL4]) end)). 21 18> element(1, timer:tc(fun() -> lists:foldr(fun(KL0, KLAcc) -> KL0 ++ KLAcc end, [], [KL1, KL2, KL3, KL4]) end)). 17 19> element(1, timer:tc(fun() -> lists:foldr(fun(KL0, KLAcc) -> KL0 ++ KLAcc end, [], [KL1, KL2, KL3, KL4]) end)). 12 20> element(1, timer:tc(fun() -> lists:foldr(fun(KL0, KLAcc) -> KL0 ++ KLAcc end, [], [KL1, KL2, KL3, KL4]) end)). 11 ``` running eprof indicates that '++' and lists:reverse have been reduced (however impact had only previously been 1-2%)
Profile following changes (comparison to #445 (comment))
|
No difference in unit test with/without inline compilation, so this has been removed
ThomasArts
reviewed
Sep 3, 2024
ThomasArts
reviewed
Sep 3, 2024
ThomasArts
reviewed
Sep 3, 2024
ThomasArts
reviewed
Sep 3, 2024
ThomasArts
reviewed
Sep 3, 2024
ThomasArts
reviewed
Sep 3, 2024
ThomasArts
approved these changes
Sep 3, 2024
These functions had previously used inline compilation - but this didn't appear to improve performance Co-authored-by: Thomas Arts <thomas.arts@quviq.com>
Co-authored-by: Thomas Arts <thomas.arts@quviq.com>
Co-authored-by: Thomas Arts <thomas.arts@quviq.com>
Also fix code coverage issues
Co-authored-by: Thomas Arts <thomas.arts@quviq.com>
…umner/leveled into mas-d34-ms.i446-plusplus
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.
Within the
leveled_sst
, a previous attempt to avoid the inefficiency of using[relatively large list] ++ [relatively small list]
led to lost of reversing an re-reversing of lists.This refactoring does a better job of enforcing the efficient appending of lists (as measured by perf_SUITE), and is potentially simpler to follow.