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

compiler: Limit depth of sharing state tree #8567

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

frej
Copy link
Contributor

@frej frej commented Jun 12, 2024

With OTP 27.0 the xmerl tests showed a markedly increased run-time. Profiling pointed to beam_ssa_ss:prune/2 as the culprit. It turns out that if there are long value derivation chains which are extended one element at a time (such as in the ss_depth_limit.erl test-case in this patch), the effective time complexity of prune will be O(n^2) where n is the number of elements in the chain.

By limiting the depth of the sharing state tree, we can avoid the drastically increased run-times for the alias analysis. This is a stop-gap fix until the alias analysis can be improved to avoid this pit-fall.

Copy link
Contributor

github-actions bot commented Jun 12, 2024

CT Test Results

    2 files    324 suites   11m 2s ⏱️
  816 tests   814 ✅ 2 💤 0 ❌
5 419 runs  5 417 ✅ 2 💤 0 ❌

Results for commit 28d0a97.

♻️ 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

@frej frej force-pushed the frej/limit-ss-depth branch from c63d259 to 5d73742 Compare June 13, 2024 07:45
@bjorng bjorng self-assigned this Jun 17, 2024
@bjorng bjorng added the team:VM Assigned to OTP team VM label Jun 17, 2024
@bjorng
Copy link
Contributor

bjorng commented Jun 17, 2024

Thanks! Looks good to me.

Can you rebase onto the OTP-27.0 label so that we can include this branch in the next emergency patch? (Only rebase; keep the target for the pull request as maint.)

With OTP 27.0 the xmerl tests showed a markedly increased run-time.
Profiling pointed to beam_ssa_ss:prune/2 as the culprit. It turns out
that if there are long value derivation chains which are extended one
element at a time (such as in the ss_depth_limit.erl test-case in this
patch), the effective time complexity of prune will be O(n^2) where n
is the number of elements in the chain.

By limiting the depth of the sharing state tree, we can avoid the
drastically increased run-times for the alias analysis. This is a
stop-gap fix until the alias analysis can be improved to avoid this
pit-fall.
@frej frej force-pushed the frej/limit-ss-depth branch from 5d73742 to 28d0a97 Compare June 17, 2024 08:24
@frej
Copy link
Contributor Author

frej commented Jun 17, 2024

Rebased onto OTP-27.0 as requested.

@bjorng
Copy link
Contributor

bjorng commented Jun 17, 2024

Thanks! Added to our daily builds.

@bjorng bjorng merged commit d11ac82 into erlang:maint Jun 18, 2024
16 checks passed
@frej frej deleted the frej/limit-ss-depth branch June 27, 2024 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants