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.

Sign up for GitHub

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

Jump to bottom

perf regression from adt_significant_drop_fix #90504

Closed
pnkfelix opened this issue Nov 2, 2021 · 1 comment
Closed

perf regression from adt_significant_drop_fix #90504

pnkfelix opened this issue Nov 2, 2021 · 1 comment
Labels
C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Nov 2, 2021

Spawned off of: #90218 (comment)

  • regression-31157 check regressed by 4.7% on incr and 5.3 on full.
  • issue-46449 check regressed by 1.89% on incr-full.
  • wg-grammar regressed by 1.3-1.4% in a bunch of scenarios.

Its possible that this is inherent in the change injected by PR #90218. But its also possible that we could fix things to remove these regressions.
@pnkfelix pnkfelix added I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Nov 2, 2021
@pnkfelix pnkfelix mentioned this issue Nov 2, 2021
Merged
@JakobDegen JakobDegen mentioned this issue Nov 12, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 16, 2021
@bors
Auto merge of rust-lang#90845 - JakobDegen:adt-drop-perf, r=Mark-Simu…
a2a7683 
…lacrum

Address performance regression introduced by rust-lang#90218

As part of the changes in rust-lang#90218 , the `adt_drop_tys` and friends code stopped recursing through the query system, meaning that intermediate computations did not get cached. This change adds the recursions back in without re-introducing any of the old issues.

On local benchmarks this fixes the 5% regressions in rust-lang#90504 ; the wg-grammar regressions didn't seem to move too much. I may take some time later to look into those.

Not sure who to request for review here, so will leave it up to whoever gets it.
@Mark-Simulacrum
Copy link
Member

Closing as fixed by #90845, though we may end up reverting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pnkfelix @Mark-Simulacrum