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

Revert "[release/8.0.2xx] NGEN Microsoft.DotNet.MSBuildSdkResolver.dll and its dependencies (#17750)" #19112

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

ladipro
Copy link
Member

@ladipro ladipro commented Mar 20, 2024

This reverts commit f0c4e4e.

Fixes AB#1994786

The MSBuild change which took advantage of this was reverted in 17.9 because it introduced issues in installations that don't have the .NET SDK component installed. We are fixing the bug in 9.0 by making changes to the dependencies of Microsoft.DotNet.MSBuildSdkResolver (see dotnet/sdk#39573) so this should stay in main. I am reverting it only in 8.0.3xx / 17.10 to fix the Build_Ngen_InvalidAssemblyCount counter which was flagged as a regression by PerfDDRITs.

@ladipro ladipro requested a review from joeloff March 20, 2024 13:39
@ladipro ladipro self-assigned this Mar 20, 2024
@ladipro ladipro merged commit 4c8fb55 into dotnet:release/8.0.3xx Mar 20, 2024
16 checks passed
@ladipro ladipro deleted the revert-ngen-sdkresolver branch March 20, 2024 15:36
ladipro added a commit that referenced this pull request Apr 12, 2024
…es" for devenv only (#19399)

Fixes: [AB#2014670](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2014670)

### Description

A change was made in 8.0.2xx to register MSBuildSdkResolver for NGEN (#17732), against both devenv.exe and MSBuild.exe. Later a bug was found in the way MSBuild.exe loads the resolver so the change was reverted in 8.0.3xx (#19112). However, because the change had a measurable positive perf effect, the revert was effectively a regression for devenv.exe and got flagged so by PerfDDRITs.

This PR is a re-do of the original change, only this time with MSBuild.exe omitted, i.e. we're NGENing the resolver only for the default architecture of devenv.exe.

### Customer Impact

Startup perf regression, about 5% more methods JITted in scenarios measured by Visual Studio PerfDDRITs.

### Regression

Yes, perf regression in VS 17.10.

### Risk 

Low
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants