Don't disable the MSBuild server for /mt builds when node reuse is off#14161
Open
AR-May wants to merge 2 commits into
Open
Don't disable the MSBuild server for /mt builds when node reuse is off#14161AR-May wants to merge 2 commits into
AR-May wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts MSBuild’s server-engagement logic so that multithreaded (/mt) builds can still run through MSBuild Server (and therefore benefit from Server GC) even when node reuse is disabled (-nr:false / MSBUILDDISABLENODEREUSE=1). It introduces a “shut down after build” behavior to honor the no-reuse intent while still using the server for /mt.
Changes:
- Decouples “can run on server” from “may keep server resident”:
/mtcan use server even when node reuse is disabled, with a newshutdownServerAfterBuildflag. - Plumbs
shutdownServerAfterBuildthroughXMake→MSBuildClientAppand requests server shutdown after the build completes. - Adds a unit test verifying
/mt -nr:falseuses the server but does not reuse it across builds.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/MSBuild/XMake.cs | Computes shutdownServerAfterBuild and allows /mt to engage the server even when node reuse is disabled. |
| src/MSBuild/MSBuildClientApp.cs | Adds a shutdown-after-build option and triggers server shutdown post-build when requested. |
| src/MSBuild.UnitTests/MSBuildServer_Tests.cs | Adds test coverage for /mt server usage + post-build shutdown behavior when node reuse is disabled. |
Comment on lines
+33
to
35
| /// <param name="shutdownServerAfterBuild">Whether to shut the server down once the build completes | ||
| /// instead of leaving it resident for reuse. | ||
| /// <param name="cancellationToken">Cancellation token.</param> |
Member
Author
There was a problem hiding this comment.
Is somehting wrong with the git diff? there is </param> at the end of the line in the file. The build is successful as well.
Comment on lines
+103
to
+106
| if (shutdownServerAfterBuild) | ||
| { | ||
| MSBuildClient.ShutdownServer(CancellationToken.None); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14157
Context
When node reuse is disabled (
-nr:false), MSBuild refused to use the server node. For a multithreaded ( /mt ) build that's a problem: the server is the only way to get Server GC, which /mt builds depend on for performance reasons.Changes Made
Decouple "may we use the server for this build?" from "may the server stay resident afterward?":
shutdownServerAfterBuildflag tears the server down immediately after the build completes, so it doesn't persist and each build gets a fresh process.Testing
Added a unit test.