Skip to content
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

Make length/1 yielding #2053

Merged
merged 1 commit into from
Dec 18, 2018

Conversation

bjorng
Copy link
Contributor

@bjorng bjorng commented Dec 13, 2018

The guard BIF length/1 would calculate the length of the list in one
go without yielding, even if the list was were long. To make it even
worse, the call to length/1 would only cost a single reduction.

This commit reimplements length/1 so that it eats a number of
reductions proportional to the length of the list, and yields if the
available reductions run out.

@bjorng bjorng added team:VM Assigned to OTP team VM enhancement testing currently being tested, tag is used by OTP internal CI labels Dec 13, 2018
@bjorng bjorng self-assigned this Dec 13, 2018
gc_bif1 Fail=j Live u$bif:erlang:length/1 Src Dst => \
i_length_setup Live Src | i_length Fail Live Dst

i_length_setup t x
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using i_length_setup t xyc work in here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, given i_length_setup is so tiny - would it make sense to also define it for r? It shouldn't grow the main loop significantly, but might save some space in the bytecode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using i_length_setup t xyc work in here?

Yes, it would. Fixed.

Additionally, given i_length_setup is so tiny - would it make sense to also define it for r? It shouldn't grow the main loop significantly, but might save some space in the bytecode.

That would not save any BEAM code space, as the t and x operands are packed into the same 32-bit word. It could possibly save a tiny, tiny amount of execution time, but not enough to make it worthwhile.

While looking at the generated code, I realized that I had forgotten to implement a minor optimization that I had thought of earlier.

@bjorng bjorng added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Dec 13, 2018
erts/emulator/beam/erl_bif_guard.c Outdated Show resolved Hide resolved
erts/emulator/beam/bif_instrs.tab Show resolved Hide resolved
The guard BIF `length/1` would calculate the length of the list in one
go without yielding, even if the list was were long. To make it even
worse, the call to `length/1` would only cost a single reduction.

This commit reimplements `length/1` so that it eats a number of
reductions proportional to the length of the list, and yields if the
available reductions run out.
@bjorng bjorng force-pushed the bjorn/erts/trapping-length/OTP-15439 branch from f1e74d3 to 1ea57e5 Compare December 18, 2018 10:43
@bjorng bjorng merged commit 641cc2d into erlang:master Dec 18, 2018
@bjorng bjorng deleted the bjorn/erts/trapping-length/OTP-15439 branch December 18, 2018 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants