Skip to content

Commit abd77c4

Browse files
OvesNCopilot
andcommitted
Skip redundant per-task environment apply/restore in the task host
When a task host runs consecutive tasks whose environment is unchanged and the previous task did not mutate it, skip re-applying and restoring the process environment. Reset the cache at build boundaries and whenever a task blocks on a callback, so nested activity always forces a fresh apply. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7679bb1 commit abd77c4

2 files changed

Lines changed: 100 additions & 7 deletions

File tree

src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,65 @@ public void TransientAndSidecarNodeCanCoexist()
257257
}
258258
}
259259

260+
/// <summary>
261+
/// Regression test for the out-of-proc task host environment-reuse optimization, which lets the host skip
262+
/// re-applying the (unchanged) build process environment between consecutive tasks. The load-bearing
263+
/// assertion is that all three tasks run in the SAME reused task host process (a shared task-host process
264+
/// id, distinct from this build process), proving the skip path was actually exercised. Each invocation --
265+
/// including ones whose apply was skipped -- must also still observe a consistent build environment.
266+
/// (Note: the variable is also inherited by the child process, so the read alone does not isolate the
267+
/// apply; the dedicated coverage for apply correctness is the serialization and -mt change-propagation
268+
/// tests.)
269+
/// </summary>
270+
[Fact]
271+
public void TaskHostObservesEnvironmentAcrossConsecutiveTasks()
272+
{
273+
using TestEnvironment env = TestEnvironment.Create(_output);
274+
275+
string variableName = "MSBUILD_ENV_REUSE_TEST_" + Guid.NewGuid().ToString("N");
276+
const string variableValue = "reuse_value_123";
277+
env.SetEnvironmentVariable(variableName, variableValue);
278+
env.SetEnvironmentVariable("MSBUILDFORCEALLTASKSOUTOFPROC", "1");
279+
280+
string projectContents = $@"
281+
<Project>
282+
<UsingTask TaskName=""{nameof(ReadEnvironmentVariableTask)}"" AssemblyFile=""{typeof(ReadEnvironmentVariableTask).Assembly.Location}"" TaskFactory=""TaskHostFactory"" />
283+
<Target Name='Build'>
284+
<{nameof(ReadEnvironmentVariableTask)} VariableName=""{variableName}"">
285+
<Output PropertyName=""Value1"" TaskParameter=""Value"" />
286+
<Output PropertyName=""Pid1"" TaskParameter=""Pid"" />
287+
</{nameof(ReadEnvironmentVariableTask)}>
288+
<{nameof(ReadEnvironmentVariableTask)} VariableName=""{variableName}"">
289+
<Output PropertyName=""Value2"" TaskParameter=""Value"" />
290+
<Output PropertyName=""Pid2"" TaskParameter=""Pid"" />
291+
</{nameof(ReadEnvironmentVariableTask)}>
292+
<{nameof(ReadEnvironmentVariableTask)} VariableName=""{variableName}"">
293+
<Output PropertyName=""Value3"" TaskParameter=""Value"" />
294+
<Output PropertyName=""Pid3"" TaskParameter=""Pid"" />
295+
</{nameof(ReadEnvironmentVariableTask)}>
296+
</Target>
297+
</Project>";
298+
299+
TransientTestFile project = env.CreateFile("envReuseProject.csproj", projectContents);
300+
ProjectInstance projectInstance = new(project.Path);
301+
302+
projectInstance.Build().ShouldBeTrue();
303+
304+
// Every task invocation -- including ones whose environment apply was skipped because the environment
305+
// was unchanged from the previous task -- must still observe the build-set variable.
306+
projectInstance.GetPropertyValue("Value1").ShouldBe(variableValue);
307+
projectInstance.GetPropertyValue("Value2").ShouldBe(variableValue);
308+
projectInstance.GetPropertyValue("Value3").ShouldBe(variableValue);
309+
310+
// All three invocations should have run in the same task host process (so the reuse path was actually
311+
// exercised) and not in the current build process.
312+
string pid1 = projectInstance.GetPropertyValue("Pid1");
313+
pid1.ShouldNotBeNullOrEmpty();
314+
pid1.ShouldBe(projectInstance.GetPropertyValue("Pid2"));
315+
pid1.ShouldBe(projectInstance.GetPropertyValue("Pid3"));
316+
pid1.ShouldNotBe(Process.GetCurrentProcess().Id.ToString(CultureInfo.InvariantCulture));
317+
}
318+
260319
/// <summary>
261320
/// Regression test for the out-of-proc task host forward environment-delta optimization (#14097). To avoid
262321
/// re-transmitting the (invariant) build process environment in every <see cref="TaskHostConfiguration"/>,

src/MSBuild/OutOfProcTaskHostNode.cs

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,15 @@ internal class OutOfProcTaskHostNode :
104104
/// </summary>
105105
private string _forwardSolutionConfigBaseline;
106106

107+
/// <summary>
108+
/// The build process environment whose values are currently reflected in this task host process. Used to
109+
/// skip the redundant per-task environment apply + restore when the next task's environment is identical
110+
/// and the previous task did not mutate it. Set to <see langword="null"/> whenever a task blocks on a
111+
/// callback (<see cref="SaveOperatingEnvironment"/>) or the node is reused for a new build, so nested
112+
/// activity and build boundaries always force a fresh apply.
113+
/// </summary>
114+
private IDictionary<string, string> _lastAppliedConfigEnvironment;
115+
107116
/// <summary>
108117
/// The event which is set when we should shut down.
109118
/// </summary>
@@ -1137,6 +1146,9 @@ private void SaveOperatingEnvironment(TaskExecutionContext context)
11371146
{
11381147
ArgumentNullException.ThrowIfNull(context);
11391148

1149+
// A task is about to block and let a nested task run, which may change the process environment.
1150+
_lastAppliedConfigEnvironment = null;
1151+
11401152
context.SavedCurrentDirectory = Environment.CurrentDirectory;
11411153
context.SavedEnvironment = new Dictionary<string, string>(
11421154
CommunicationsUtilities.GetEnvironmentVariables(),
@@ -1312,6 +1324,10 @@ private void HandleNodeBuildComplete(NodeBuildComplete buildComplete)
13121324
{
13131325
Assumed.Zero(_activeTaskCount, "We should never have a task in the process of executing when we receive NodeBuildComplete.");
13141326

1327+
// When this node is reused for a later build, reset the environment-reuse cache so the first task of
1328+
// the next build performs a fresh apply rather than trusting state left over from this build.
1329+
_lastAppliedConfigEnvironment = null;
1330+
13151331
// Sidecar TaskHost will persist after the build is done.
13161332
if (_nodeReuse)
13171333
{
@@ -1476,14 +1492,24 @@ private void RunTask(object state)
14761492
// Change to the startup directory
14771493
NativeMethodsShared.SetCurrentDirectory(taskConfiguration.StartupDirectory);
14781494

1479-
if (_updateEnvironment)
1495+
bool canSkipEnvironmentApply = _lastAppliedConfigEnvironment is not null
1496+
&& _blockedTaskCount == 0
1497+
&& _activeTaskCount == 1
1498+
&& CommunicationsUtilities.AreEnvironmentsEquivalent(taskConfiguration.BuildProcessEnvironment, _lastAppliedConfigEnvironment);
1499+
1500+
if (!canSkipEnvironmentApply)
14801501
{
1481-
InitializeMismatchedEnvironmentTable(taskConfiguration.BuildProcessEnvironment);
1502+
if (_updateEnvironment)
1503+
{
1504+
InitializeMismatchedEnvironmentTable(taskConfiguration.BuildProcessEnvironment);
1505+
}
1506+
1507+
// Now set the new environment
1508+
SetTaskHostEnvironment(taskConfiguration.BuildProcessEnvironment);
1509+
DotnetHostEnvironmentHelper.ClearBootstrapDotnetRootEnvironment(taskConfiguration.BuildProcessEnvironment);
14821510
}
14831511

1484-
// Now set the new environment
1485-
SetTaskHostEnvironment(taskConfiguration.BuildProcessEnvironment);
1486-
DotnetHostEnvironmentHelper.ClearBootstrapDotnetRootEnvironment(taskConfiguration.BuildProcessEnvironment);
1512+
_lastAppliedConfigEnvironment = taskConfiguration.BuildProcessEnvironment;
14871513

14881514
// Set culture
14891515
Thread.CurrentThread.CurrentCulture = taskConfiguration.Culture;
@@ -1567,8 +1593,16 @@ private void RunTask(object state)
15671593
}
15681594
#endif
15691595

1570-
// Restore the original clean environment
1571-
CommunicationsUtilities.SetEnvironment(_savedEnvironment);
1596+
bool canSkipEnvironmentRestore = environmentUnchangedByTask
1597+
&& _blockedTaskCount == 0
1598+
&& ReferenceEquals(_lastAppliedConfigEnvironment, taskConfiguration.BuildProcessEnvironment);
1599+
1600+
if (!canSkipEnvironmentRestore)
1601+
{
1602+
// Restore the original clean environment
1603+
CommunicationsUtilities.SetEnvironment(_savedEnvironment);
1604+
_lastAppliedConfigEnvironment = null;
1605+
}
15721606
}
15731607
catch (Exception e)
15741608
{

0 commit comments

Comments
 (0)