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

Review/Fix MSBuild ngen registrations #10815

Open
JanKrivanek opened this issue Oct 15, 2024 · 3 comments
Open

Review/Fix MSBuild ngen registrations #10815

JanKrivanek opened this issue Oct 15, 2024 · 3 comments
Assignees
Labels
Cost:S Work that requires one engineer up to 1 week performance Priority:2 Work that is important, but not critical for the release triaged

Comments

@JanKrivanek
Copy link
Member

Context

Reported by @davkean:

Its super unclear why all MSBuild binaries in general are being registered for all architectures too (we only should register 32-bit for 32-bit and 64-bit for 64-bit), those 32-bit vsn.exe are definitely wrong as we don’t ship a 32-bit version of VS, so we’re wasting end-user machine resources.

Goal

Only required registrations are issued

@maridematte maridematte added Cost:S Work that requires one engineer up to 1 week Priority:2 Work that is important, but not critical for the release labels Oct 15, 2024
@YuliiaKovalova
Copy link
Member

YuliiaKovalova commented Oct 18, 2024

Sub-issue
#10847

@rainersigwald
Copy link
Member

Looking at this as part of #11145, and I think we need Setup features or a pretty big package refactor to do it right.

Specifically, we want our core assemblies and their transitive closure to be ngened in several contexts:

Context controlling config arch file
VS in-proc vsn.exe amd64 or arm64 bin\Microsoft.Build*.dll
closure
Mainline CLI bin\amd64\MSBuild.exe amd64 bin\amd64\MSBuild.exe
bin\Microsoft.Build*.dll
closure
Old CLI and 32-bit taskhost bin\MSBuild.exe x86 bin\MSBuild.exe
bin\Microsoft.Build*.dll
closure
arm64 CLI bin\arm64\MSBuild.exe arm64 bin\arm64\MSBuild.exe
bin\Microsoft.Build*.dll
closure

But I don't see a way to express in the .swr or the VSIX manifest things like "ngen bin\Microsoft.Build.dll for amd64 with bin\amd64\MSBuild.exe.config and vsn.config and also for x86 with bin\MSBuild.exe.config".

Today, we run:

[framework]
install "C:\VisualStudio\MSBuild\Current\Bin\Microsoft.Build.dll" /NoDependencies /ExeConfig:"C:\VisualStudio\MSBuild\Current\Bin\MSBuild.exe" /queue:1
install "C:\VisualStudio\MSBuild\Current\Bin\Microsoft.Build.dll" /NoDependencies /ExeConfig:"C:\VisualStudio\Common7\IDE\vsn.exe" /queue:1

[framework64]
install "C:\VisualStudio\MSBuild\Current\Bin\Microsoft.Build.dll" /NoDependencies /ExeConfig:"C:\VisualStudio\Common7\IDE\vsn.exe" /queue:1
install "C:\VisualStudio\MSBuild\Current\Bin\Microsoft.Build.dll" /NoDependencies /ExeConfig:"C:\VisualStudio\MSBuild\Current\Bin\MSBuild.exe" /queue:1
  • the 32-bit vsn.exe one is unnecessary
  • 32-bit msbuild.exe is important for CI
  • 64-bit vsn.exe is critical
  • 64-bit msbuild.exe is critical (but is it getting the right context here? At runtime it'll get bin\amd64\MSBuild.exe.config but it's running with just bin\MSBuild.exe.config)

@JanProvaznik
Copy link
Member

reverted #11182 which contributed to this:

Regresses VM_AdjustedImagesInMemory_Total_devenv e.g. here:
https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/606301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cost:S Work that requires one engineer up to 1 week performance Priority:2 Work that is important, but not critical for the release triaged
Projects
None yet
Development

No branches or pull requests

5 participants