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

Large perf regressions in coreclr.dll native code #93837

Closed
jkotas opened this issue Oct 23, 2023 · 4 comments · Fixed by #93838
Closed

Large perf regressions in coreclr.dll native code #93837

jkotas opened this issue Oct 23, 2023 · 4 comments · Fixed by #93838
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@jkotas
Copy link
Member

jkotas commented Oct 23, 2023

Repro

Observe coreclr.dll size is 4,579,328

Observe coreclr.dll size is 6,098,432 (33% regression)

The x86 large size difference is just one manifestation of the regression. There are regressions on other architectures and there are regressions in throughput too. (I have run into them when trying to validate #93766 for servicing backport.)

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 23, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 23, 2023
@ghost
Copy link

ghost commented Oct 23, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Repro

Observe coreclr.dll size is 4,579,328

Observe coreclr.dll size is 6,098,432 (33% regression)

The x86 large size difference is just one manifestation of the regression. There are regressions on other architectures and there are regressions in throughput too. (I have run into them when trying to validate #93766 for servicing backport.)

Author: jkotas
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@jkotas
Copy link
Member Author

jkotas commented Oct 23, 2023

@jtschuster This was introduced by #92844.

cc @jkoritzinsky

I am considering reverting the change. The longer we run with it, the more problems it is going to cause in our performance tracking.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Oct 23, 2023
@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Oct 23, 2023
@jakobbotsch
Copy link
Member

jakobbotsch commented Oct 30, 2023

FWIW, I noticed that CET was enabled in the native EXEs before #93838 (windbg was exiting with an error when trying to record TTD trace). Maybe it could explain the regression?

@jkotas
Copy link
Member Author

jkotas commented Oct 30, 2023

CET should not be a thing on 32-bit x86. I have briefly looked into it before reverting and the top contributor to the binary size regression seemed to be broken LTCG. The linker command line had number of changes that did not look intentional.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants