From 0e26c0d6eae229c0032d9fd231ee06c691f94250 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Wed, 13 May 2026 19:35:11 +0200 Subject: [PATCH] Server: -mt implies MSBuild Server (MSBUILDUSESERVER=0 opt-out) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When -mt is on the command line and MSBUILDUSESERVER is unset, automatically engage MSBuild Server. Explicit MSBUILDUSESERVER=0 always opts out (takes precedence over the implicit -mt opt-in). Decision tree ------------- | MSBUILDUSESERVER | -mt switch | Server engaged? | Reason | |------------------|------------|-----------------|--------------------| | 1 | (any) | Yes (existing) | EnvVar | | 0 | (any) | No (existing) | (explicit opt-out) | | unset | present | Yes (NEW) | ImpliedByMt | | unset | absent | No (existing) | - | Rationale --------- -mt users already accept process-shared state (in-proc thread workers instead of multi-process worker nodes), so server reuse barely adds risk and recovers the per-invocation JIT/SDK-resolution warm-up cost that -mt would otherwise pay every build (because -mt shares the entry process with the build instead of the workers). Investigation #9379 / Thread F. Implementation -------------- - New ShouldUseMSBuildServer(string[], out string) helper at the routing decision point in XMake.cs Main. - Lightweight CommandLineContainsMultiThreadedSwitch parses only -mt/-multithreaded (and honors :false) without doing full GatherAllSwitches twice. Conservative on parse errors (returns false → declines implicit opt-in; explicit env=1 still works). - New BuildTelemetry.ServerEnableReason field ('EnvVar' or 'ImpliedByMt') so dashboards can measure adoption of the implicit path separately from the explicit env-var path. Test ---- ServerStartsWhenMtPresentEvenWithoutEnvVar: runs two consecutive '-mt' builds with MSBUILDUSESERVER unset and asserts the server PID is the SAME for both (server reuse is the unique signature — a non-server build would always get a fresh worker PID). All 10 MSBuildServer_Tests pass. The opt-out case (env=0 + -mt) is implementation-tested by inspection of ShouldUseMSBuildServer: an end-to-end PID-pattern test would be unreliable because -mt's worker placement is independent of server engagement (workers may run in-proc or out-of-proc regardless of server mode), so 'PIDs match → no server' is not a sound inference under -mt. The current MSBuildServerTest pattern relies on the opposite signature ('PIDs differ across multiple invocations → server is reused') which is the reliable signal used in the new test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Framework/Telemetry/BuildTelemetry.cs | 11 ++ src/MSBuild.UnitTests/MSBuildServer_Tests.cs | 31 ++++++ src/MSBuild/XMake.cs | 109 ++++++++++++++++++- 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/src/Framework/Telemetry/BuildTelemetry.cs b/src/Framework/Telemetry/BuildTelemetry.cs index 34a95a589d7..de3e5c4e014 100644 --- a/src/Framework/Telemetry/BuildTelemetry.cs +++ b/src/Framework/Telemetry/BuildTelemetry.cs @@ -61,6 +61,16 @@ internal class BuildTelemetry : TelemetryBase, IActivityTelemetryDataHolder /// public string? ServerFallbackReason { get; set; } + /// + /// Why MSBuild server was engaged for this invocation. One of: + /// "EnvVar" — MSBUILDUSESERVER=1 was set (explicit opt-in) + /// "ImpliedByMt" — -mt switch was on the command line and MSBUILDUSESERVER was unset + /// null — server was not engaged + /// Lets dashboards measure adoption of the implicit -mt-implies-server path separately + /// from the explicit env-var path. + /// + public string? ServerEnableReason { get; set; } + /// /// Version of MSBuild. /// @@ -200,6 +210,7 @@ public override IDictionary GetProperties() AddIfNotNull(InitialMSBuildServerState); AddIfNotNull(ProjectPath != null ? Path.GetFileName(ProjectPath) : null, nameof(ProjectPath)); AddIfNotNull(ServerFallbackReason); + AddIfNotNull(ServerEnableReason); AddIfNotNull(SanitizeBuildTarget(BuildTarget), nameof(BuildTarget)); AddIfNotNull(BuildEngineVersion?.ToString(), nameof(BuildEngineVersion)); AddIfNotNull(BuildSuccess?.ToString(), nameof(BuildSuccess)); diff --git a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs index 1531c665f54..7942fe423cb 100644 --- a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs +++ b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs @@ -307,6 +307,37 @@ public void ServerShouldStartWhenBuildIsInteractive() serverIsDown.ShouldBeTrue(); } + [Fact] + public void ServerStartsWhenMtPresentEvenWithoutEnvVar() + { + // Regression test for the "-mt implies MSBuild Server" routing decision + // (investigation #9379, ShouldUseMSBuildServer / CommandLineContainsMultiThreadedSwitch). + // When MSBUILDUSESERVER is unset and the user passes -mt, the client should engage + // the server automatically. Verified by running two builds back-to-back and asserting + // the server process PID is the SAME for both — server reuse is the unique signature + // of MSBuild server engagement (a non-server build would always get a fresh worker PID). + TransientTestFile project = _env.CreateFile("testProject.proj", printPidContents); + // Explicitly clear MSBUILDUSESERVER so we test the -mt-implies-server path. + _env.SetEnvironmentVariable("MSBUILDUSESERVER", null); + + // Make sure we start with no server running. + MSBuildClient.ShutdownServer(CancellationToken.None); + + string output1 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, project.Path + " -mt", out bool success1, false, _output); + success1.ShouldBeTrue(); + int serverPid1 = ParseNumber(output1, "Server ID is "); + + string output2 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, project.Path + " -mt", out bool success2, false, _output); + success2.ShouldBeTrue(); + int serverPid2 = ParseNumber(output2, "Server ID is "); + + serverPid1.ShouldBe(serverPid2, "When -mt implies server, two consecutive builds should reuse the same server process. PIDs were " + serverPid1 + " and " + serverPid2 + "."); + + _env.WithTransientProcess(serverPid1); + // Clean up the server we spun up. + MSBuildClient.ShutdownServer(CancellationToken.None); + } + [Fact] public void PropertyMSBuildStartupDirectoryOnServer() { diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 4fdb1aecdb3..1eee140a2f3 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -311,12 +311,16 @@ public static int Main(string[] args) int exitCode; if ( - Environment.GetEnvironmentVariable(Traits.UseMSBuildServerEnvVarName) == "1" && + ShouldUseMSBuildServer(args, out string serverEnableReason) && !Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout && CanRunServerBasedOnCommandLineSwitches(args)) { Console.CancelKeyPress += Console_CancelKeyPress; + if (KnownTelemetry.PartialBuildTelemetry != null) + { + KnownTelemetry.PartialBuildTelemetry.ServerEnableReason = serverEnableReason; + } // Use the client app to execute build in msbuild server. Opt-in feature. exitCode = ((s_initialized && MSBuildClientApp.Execute(args, s_buildCancellationSource.Token) == ExitType.Success) ? 0 : 1); @@ -337,6 +341,109 @@ public static int Main(string[] args) return exitCode; } + /// + /// Returns true if MSBuild Server should be used for this invocation. + /// + /// + /// Decision tree: + /// + /// MSBUILDUSESERVER=1 → use server (existing explicit opt-in). + /// MSBUILDUSESERVER=0 → do NOT use server (explicit opt-out, takes precedence over -mt). + /// MSBUILDUSESERVER unset AND command line contains -mt/-multithreaded → use server. + /// Rationale: -mt users already accept process-shared state (in-proc thread workers + /// instead of multi-process worker nodes), so server reuse barely adds risk and recovers the + /// per-invocation JIT/SDK-resolution warm-up cost that -mt would otherwise pay every + /// build (because -mt shares the entry process with the build instead of the workers). + /// See #9379. + /// Otherwise → no server (existing default). + /// + /// + /// Raw command-line arguments. + /// Telemetry-friendly reason: "EnvVar", "ImpliedByMt", or empty when not enabled. + /// True if server should be used. + private static bool ShouldUseMSBuildServer(string[] args, out string serverEnableReason) + { + serverEnableReason = string.Empty; + string envVar = Environment.GetEnvironmentVariable(Traits.UseMSBuildServerEnvVarName); + + if (envVar == "1") + { + serverEnableReason = "EnvVar"; + return true; + } + + // Explicit opt-out: MSBUILDUSESERVER=0 always wins, even if -mt is on the command line. + if (envVar == "0") + { + return false; + } + + // Implicit opt-in via -mt. Best-effort detection that does not throw — if argument + // parsing fails for any reason we conservatively decline the implicit opt-in (the + // explicit MSBUILDUSESERVER=1 path is unaffected). + if (CommandLineContainsMultiThreadedSwitch(args)) + { + serverEnableReason = "ImpliedByMt"; + return true; + } + + return false; + } + + /// + /// Lightweight check for the presence of -mt/-multithreaded on the command + /// line, suitable for the early routing decision in . + /// + /// + /// Does NOT do full command-line parsing (which is expensive and runs again later). + /// Intentionally conservative: returns false on any parse problem. + /// + private static bool CommandLineContainsMultiThreadedSwitch(string[] args) + { + if (args is null || args.Length == 0) + { + return false; + } + + foreach (string arg in args) + { + if (string.IsNullOrEmpty(arg) || arg.Length < 2) + { + continue; + } + + // Switches start with - or / (and on Unix, - is the only convention). + char prefix = arg[0]; + if (prefix != '-' && prefix != '/') + { + continue; + } + + // Strip leading - or /, then split on : to get the switch name (ignore any value/parameters). + string body = arg.Substring(1); + int colonIndex = body.IndexOf(':'); + string switchName = colonIndex >= 0 ? body.Substring(0, colonIndex) : body; + + if (switchName.Equals("mt", StringComparison.OrdinalIgnoreCase) || + switchName.Equals("multithreaded", StringComparison.OrdinalIgnoreCase)) + { + // Honor an explicit ":false" value if present (matches ProcessBooleanSwitch semantics). + if (colonIndex >= 0) + { + string value = body.Substring(colonIndex + 1); + if (value.Equals("false", StringComparison.OrdinalIgnoreCase) || + value.Equals("0", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + } + return true; + } + } + + return false; + } + /// /// Returns true if arguments allows or make sense to leverage msbuild server. ///