Skip to content

Allow NodeProviderOutOfProcTaskHost to manage multiple nodes instead of one per arch #12521

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Allow NodeProviderOutOfProcTaskHost to manage multiple nodes instead of one per arch #12521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3321023
change NodeProviderOutOfProcTaskHost to manage pools of nodes:
surayya-MS Sep 12, 2025
9bae89a
remove unused prop
surayya-MS Sep 12, 2025
46cfde5
Merge branch 'main' into out-of-proc-task-host-pools
surayya-MS Sep 12, 2025
d3b705b
return int.MaxValue for AvailableNodes, fix error message, cleanup co…
surayya-MS Sep 15, 2025
34e5365
remove lock since _odeContexts is now ConcurrentDictionary
surayya-MS Sep 15, 2025
4912cf7
remove lock on _nodeContexts
surayya-MS Sep 15, 2025
8a43dcc
use LockType
surayya-MS Sep 15, 2025
2a2140f
Merge branch 'main' into out-of-proc-task-host-pools
surayya-MS Sep 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 33 additions & 57 deletions src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
Expand All @@ -22,13 +23,6 @@ namespace Microsoft.Build.BackEnd
/// </summary>
internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeProvider, INodePacketFactory, INodePacketHandler
{
/// <summary>
/// The maximum number of nodes that this provider supports. Should
/// always be equivalent to the number of different TaskHostContexts
/// that exist.
/// </summary>
private const int MaxNodeCount = 4;

/// <summary>
/// Store the path for MSBuild / MSBuildTaskHost so that we don't have to keep recalculating it.
/// </summary>
Expand Down Expand Up @@ -87,8 +81,14 @@ internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeP
/// <summary>
/// A mapping of all the nodes managed by this provider.
/// </summary>
private Dictionary<HandshakeOptions, NodeContext> _nodeContexts;
private ConcurrentDictionary<int, NodeContext> _nodeContexts;

/// <summary>
/// The next node id to assign to a node.
/// </summary>
private int _nextNodeId = 1;

private LockType _nextNodeIdLock = new LockType();
/// <summary>
/// A mapping of all of the INodePacketFactories wrapped by this provider.
/// </summary>
Expand Down Expand Up @@ -135,7 +135,7 @@ public int AvailableNodes
{
get
{
return MaxNodeCount - _nodeContexts.Count;
return int.MaxValue;
}
}

Expand Down Expand Up @@ -175,19 +175,9 @@ public IList<NodeInfo> CreateNodes(int nextNodeId, INodePacketFactory packetFact
/// <param name="packet">The packet to send.</param>
public void SendData(int nodeId, INodePacket packet)
{
throw new NotImplementedException("Use the other overload of SendData instead");
}

/// <summary>
/// Sends data to the specified node.
/// </summary>
/// <param name="hostContext">The node to which data shall be sent.</param>
/// <param name="packet">The packet to send.</param>
public void SendData(HandshakeOptions hostContext, INodePacket packet)
{
ErrorUtilities.VerifyThrow(_nodeContexts.ContainsKey(hostContext), "Invalid host context specified: {0}.", hostContext.ToString());
ErrorUtilities.VerifyThrow(_nodeContexts.ContainsKey(nodeId), "Invalid host context specified: {0}.", nodeId);

SendData(_nodeContexts[hostContext], packet);
SendData(_nodeContexts[nodeId], packet);
}

/// <summary>
Expand All @@ -199,10 +189,7 @@ public void ShutdownConnectedNodes(bool enableReuse)
// Send the build completion message to the nodes, causing them to shutdown or reset.
List<NodeContext> contextsToShutDown;

lock (_nodeContexts)
{
contextsToShutDown = new List<NodeContext>(_nodeContexts.Values);
}
contextsToShutDown = new List<NodeContext>(_nodeContexts.Values);

ShutdownConnectedNodes(contextsToShutDown, enableReuse);

Expand All @@ -227,7 +214,7 @@ public void ShutdownAllNodes()
public void InitializeComponent(IBuildComponentHost host)
{
this.ComponentHost = host;
_nodeContexts = new Dictionary<HandshakeOptions, NodeContext>();
_nodeContexts = new ConcurrentDictionary<int, NodeContext>();
_nodeIdToPacketFactory = new Dictionary<int, INodePacketFactory>();
_nodeIdToPacketHandler = new Dictionary<int, INodePacketHandler>();
_activeNodes = new HashSet<int>();
Expand Down Expand Up @@ -592,24 +579,23 @@ internal bool AcquireAndSetUpHost(
INodePacketFactory factory,
INodePacketHandler handler,
TaskHostConfiguration configuration,
Dictionary<string, string> taskHostParameters)
Dictionary<string, string> taskHostParameters,
out int nodeId)
{
bool nodeCreationSucceeded;
if (!_nodeContexts.ContainsKey(hostContext))
{
nodeCreationSucceeded = CreateNode(hostContext, factory, handler, configuration, taskHostParameters);
}
else

lock (_nextNodeIdLock)
{
// node already exists, so "creation" automatically succeeded
nodeCreationSucceeded = true;
nodeId = _nextNodeId++;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider https://learn.microsoft.com/en-us/dotnet/api/system.threading.interlocked.increment?view=net-9.0 instead of the lock

}

nodeCreationSucceeded = CreateNode(hostContext, nodeId, factory, handler, configuration, taskHostParameters);

if (nodeCreationSucceeded)
{
NodeContext context = _nodeContexts[hostContext];
_nodeIdToPacketFactory[(int)hostContext] = factory;
_nodeIdToPacketHandler[(int)hostContext] = handler;
NodeContext context = _nodeContexts[nodeId];
_nodeIdToPacketFactory[nodeId] = factory;
_nodeIdToPacketHandler[nodeId] = handler;

// Configure the node.
context.SendData(configuration);
Expand All @@ -622,33 +608,26 @@ internal bool AcquireAndSetUpHost(
/// <summary>
/// Expected to be called when TaskHostTask is done with host of the given context.
/// </summary>
internal void DisconnectFromHost(HandshakeOptions hostContext)
internal void DisconnectFromHost(int nodeId)
{
ErrorUtilities.VerifyThrow(_nodeIdToPacketFactory.ContainsKey((int)hostContext) && _nodeIdToPacketHandler.ContainsKey((int)hostContext), "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?");
ErrorUtilities.VerifyThrow(_nodeIdToPacketFactory.ContainsKey(nodeId) && _nodeIdToPacketHandler.ContainsKey(nodeId), "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?");

_nodeIdToPacketFactory.Remove((int)hostContext);
_nodeIdToPacketHandler.Remove((int)hostContext);
_nodeIdToPacketFactory.Remove(nodeId);
_nodeIdToPacketHandler.Remove(nodeId);
}

/// <summary>
/// Instantiates a new MSBuild or MSBuildTaskHost process acting as a child node.
/// </summary>
internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration, Dictionary<string, string> taskHostParameters)
internal bool CreateNode(HandshakeOptions hostContext, int nextNodeId, INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration, Dictionary<string, string> taskHostParameters)
{
ErrorUtilities.VerifyThrowArgumentNull(factory);
ErrorUtilities.VerifyThrow(!_nodeIdToPacketFactory.ContainsKey((int)hostContext), "We should not already have a factory for this context! Did we forget to call DisconnectFromHost somewhere?");

if (AvailableNodes <= 0)
{
ErrorUtilities.ThrowInternalError("All allowable nodes already created ({0}).", _nodeContexts.Count);
return false;
}
ErrorUtilities.VerifyThrow(!_nodeIdToPacketFactory.ContainsKey(nextNodeId), "We should not already have a factory for this nodeId! Did we forget to call DisconnectFromHost somewhere?");

// if runtime host path is null it means we don't have MSBuild.dll path resolved and there is no need to include it in the command line arguments.
string commandLineArgsPlaceholder = "{0} /nologo /nodemode:2 /nodereuse:{1} /low:{2} ";

IList<NodeContext> nodeContexts;
int nodeId = (int)hostContext;

// Handle .NET task host context
#if NETFRAMEWORK
Expand All @@ -664,7 +643,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
nodeContexts = GetNodes(
runtimeHostPath,
string.Format(commandLineArgsPlaceholder, Path.Combine(msbuildAssemblyPath, Constants.MSBuildAssemblyName), NodeReuseIsEnabled(hostContext), ComponentHost.BuildParameters.LowPriority),
nodeId,
nextNodeId,
this,
handshake,
NodeContextCreated,
Expand All @@ -688,7 +667,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
nodeContexts = GetNodes(
msbuildLocation,
string.Format(commandLineArgsPlaceholder, string.Empty, NodeReuseIsEnabled(hostContext), ComponentHost.BuildParameters.LowPriority),
nodeId,
nextNodeId,
this,
new Handshake(hostContext),
NodeContextCreated,
Expand All @@ -714,7 +693,7 @@ bool NodeReuseIsEnabled(HandshakeOptions hostContext)
/// </summary>
private void NodeContextCreated(NodeContext context)
{
_nodeContexts[(HandshakeOptions)context.NodeId] = context;
_nodeContexts[context.NodeId] = context;

// Start the asynchronous read.
context.BeginAsyncPacketRead();
Expand All @@ -731,10 +710,7 @@ private void NodeContextCreated(NodeContext context)
/// </summary>
private void NodeContextTerminated(int nodeId)
{
lock (_nodeContexts)
{
_nodeContexts.Remove((HandshakeOptions)nodeId);
}
_nodeContexts.TryRemove(nodeId, out _);

// May also be removed by unnatural termination, so don't assume it's there
lock (_activeNodes)
Expand Down
18 changes: 10 additions & 8 deletions src/Build/Instance/TaskFactories/TaskHostTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ internal class TaskHostTask : IGeneratedTask, ICancelableTask, INodePacketFactor
/// The task host context of the task host we're launching -- used to
/// communicate with the task host.
/// </summary>
private HandshakeOptions _requiredContext = HandshakeOptions.None;
private HandshakeOptions _requiredHandshakeOptions = HandshakeOptions.None;

private int _requiredNodeId = -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this naming is confusing, if you want to express that a specific taskhosttask has a mapping to a nodeId, then I'd just say something like _taskHostNodeId


/// <summary>
/// True if currently connected to the task host; false otherwise.
Expand Down Expand Up @@ -251,7 +253,7 @@ public void Cancel()
{
if (_taskHostProvider != null && _connectedToTaskHost)
{
_taskHostProvider.SendData(_requiredContext, new TaskHostTaskCancelled());
_taskHostProvider.SendData(_requiredNodeId, new TaskHostTaskCancelled());
}
}

Expand Down Expand Up @@ -304,14 +306,14 @@ public bool Execute()
{
lock (_taskHostLock)
{
_requiredContext = CommunicationsUtilities.GetHandshakeOptions(
_requiredHandshakeOptions = CommunicationsUtilities.GetHandshakeOptions(
taskHost: true,

// Determine if we should use node reuse based on build parameters or user preferences (comes from UsingTask element).
// If the user explicitly requested the task host factory, then we always disable node reuse due to the transient nature of task host factory hosts.
nodeReuse: _buildComponentHost.BuildParameters.EnableNodeReuse && !_taskHostFactoryExplicitlyRequested,
taskHostParameters: _taskHostParameters);
_connectedToTaskHost = _taskHostProvider.AcquireAndSetUpHost(_requiredContext, this, this, hostConfiguration, _taskHostParameters);
_connectedToTaskHost = _taskHostProvider.AcquireAndSetUpHost(_requiredHandshakeOptions, this, this, hostConfiguration, _taskHostParameters, out _requiredNodeId);
}

if (_connectedToTaskHost)
Expand Down Expand Up @@ -340,23 +342,23 @@ public bool Execute()
{
lock (_taskHostLock)
{
_taskHostProvider.DisconnectFromHost(_requiredContext);
_taskHostProvider.DisconnectFromHost(_requiredNodeId);
_connectedToTaskHost = false;
}
}
}
else
{
LogErrorUnableToCreateTaskHost(_requiredContext, runtime, architecture, null);
LogErrorUnableToCreateTaskHost(_requiredHandshakeOptions, runtime, architecture, null);
}
}
catch (BuildAbortedException)
{
LogErrorUnableToCreateTaskHost(_requiredContext, runtime, architecture, null);
LogErrorUnableToCreateTaskHost(_requiredHandshakeOptions, runtime, architecture, null);
}
catch (NodeFailedToLaunchException e)
{
LogErrorUnableToCreateTaskHost(_requiredContext, runtime, architecture, e);
LogErrorUnableToCreateTaskHost(_requiredHandshakeOptions, runtime, architecture, e);
}

return _taskExecutionSucceeded;
Expand Down