Add fast-lanes for '++'/2 and '--'/2 on nil arguments #8743
Conversation
CT Test Results 3 files 141 suites 49m 20s ⏱️ For more details on these failures, see this check. Results for commit 8eeb95d. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
the-mikedavis
force-pushed
the
md/optimize-plus-plus-nil-rhs
branch
from
77add1f to
8fe9b3e
Compare
the-mikedavis
force-pushed
the
md/optimize-plus-plus-nil-rhs
branch
from
8fe9b3e to
4490c7c
Compare
the-mikedavis
changed the title
List++[]
the-mikedavis
force-pushed
the
md/optimize-plus-plus-nil-rhs
branch
3 times, most recently
from
601905b to
d9db77d
Compare
the-mikedavis
force-pushed
the
md/optimize-plus-plus-nil-rhs
branch
2 times, most recently
from
0d371fc to
1c429ad
Compare
erts/emulator/beam/erl_bif_lists.c
Outdated
| int res; | ||
|
|
||
| /* First check that lhs is proper. */ | ||
| res = append_is_proper_list(p, &context->iterator); | ||
|
|
||
| if (res == 1) { | ||
| #ifdef DEBUG | ||
| context->result_cdr = &context->rhs_original; | ||
| #endif | ||
| context->result = context->lhs_original; | ||
| } | ||
|
|
||
| return res; |
There was a problem hiding this comment.
If we move setting these to the branch returning 1 in append_is_proper_list, we can shorten this a lot. While we're at it we can change its name to append_empty_rhs or the likes.
Otherwise this PR LGTM.
the-mikedavis
force-pushed
the
md/optimize-plus-plus-nil-rhs
branch
from
1c429ad to
e5c1f78
Compare
Currently appending an empty list to another list causes the left-hand side list to be fully copied. We can skip the append operation when the right-hand side is nil since it shouldn't change the left-hand side list. In this fast-lane we scan the list to determine if it is improper so that cases like `[1, 2 | 3] ++ []` behave the same as cases like `[1, 2 | 3] ++ [4]`: return a badarg error. This patch adds similar fast-lanes for '--'/2: we can return the first argument when either the first or second arguments are nil.
the-mikedavis
force-pushed
the
md/optimize-plus-plus-nil-rhs
branch
from
e5c1f78 to
8eeb95d
Compare
|
Merged, thanks for the PR! |
|
Thanks for your help with this @jhogberg! I wasn't familiar with the internals and trapping so this was a cool learning experience :) |
Currently appending an empty list to another list causes the left-hand side list to be fully copied. We can skip the append operation when the right-hand side is nil since it shouldn't change the left-hand side list.
It looks like there are historical artifacts in the
++/2implementation like its behavior on[] ++ anythingso I'm not sure if I'm missing some context, but I wanted to propose this optimization since it seems needless to allocate forList ++ []. What do you think?