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

Switch to full ngen for Roslyn binaries #54995

Merged
merged 3 commits into from
Jul 30, 2021
Merged

Conversation

genlu
Copy link
Member

@genlu genlu commented Jul 20, 2021

…Ngen"

This reverts commit 5536669, reversing
changes made to b6a9f8c.
@genlu
Copy link
Member Author

genlu commented Jul 30, 2021

The insertion shows reduced jitting across the board and some regression caused by increased image size, which is expected.

@genlu
Copy link
Member Author

genlu commented Jul 30, 2021

FYI @allisonchou @davkean

@allisonchou
Copy link
Contributor

@genlu As someone who's not very familiar with ngen, what implications (if any) will this have on the insertion flow?

@genlu
Copy link
Member Author

genlu commented Jul 30, 2021

@allisonchou based on the RPS results in the val insertion, this would reduce jitting but also might cause regression in refset due to increased image size. The regression should be auto-approved for exception though, since it's expected for full ngen.

FYI, we saw similar refset regression last time we tried to switch to full ngen
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_git/VS/pullrequest/329020

@davkean
Copy link
Member

davkean commented Jul 30, 2021

This regressed this counter which should be looked at:

BuildInfoVS64.Test - RegressedBeyondThreshold / 0001.BuildInfo / Build_Ngen_InvalidAssemblyCount / Regressed: 4.00 Count (21.05%)

@genlu
Copy link
Member Author

genlu commented Jul 30, 2021

@davkean That's unrelated. There's a breaking change in LanguageServer.Client (PR), which requires some additional change in VS repo, which my validation PR doesn't have.

@genlu genlu merged commit 14a228a into dotnet:main-vs-deps Jul 30, 2021
@ghost ghost added this to the Next milestone Jul 30, 2021
@genlu genlu deleted the fullNgen branch July 30, 2021 21:57
@dibarbet dibarbet modified the milestones: Next, 17.0.P4 Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants