Refactor Node IDs for taskhost to use record struct

[Copilot]

Fixes #12676

Context

Follow-up to #12577. The taskhost node identification used bit-packing to combine scheduledNodeId (8 bits) and HandshakeOptions (9 bits) into a single int, limiting multithreaded builds to 256 nodes. This approach was fragile and hard to maintain.

Changes Made

• New TaskHostNodeKey record struct in CommunicationsUtilities.cs combining HandshakeOptions and NodeId as a clean, type-safe key
• Updated NodeProviderOutOfProcTaskHost with optimized dictionary structure:
  • _nodeContexts keyed by TaskHostNodeKey for checking if a node with given (HandshakeOptions, scheduledNodeId) exists (acquisition logic)
  • _nodeIdToNodeKey for O(1) reverse lookup during node termination
  • _nodeIdToPacketFactory and _nodeIdToPacketHandler keyed by int for O(1) packet routing performance
  • _activeNodes tracks node IDs (int) for shutdown handling
  • Added atomic _nextNodeId counter for unique communication node ID generation
• Simplified TaskHostTask - removed GenerateTaskHostNodeId method and associated bit-packing constants
• Improved error handling - Added VerifyThrow assertions in DisconnectFromHost and CreateNode to verify context existence and prevent duplicate nodes

// Before: bit-packed int with 256-node limit
_taskHostNodeId = GenerateTaskHostNodeId(_scheduledNodeId, _requiredContext);
// (scheduledNodeId << 9) | ((int)handshakeOptions & 0x1FF)

// After: clean record struct, no limit
_taskHostNodeKey = new TaskHostNodeKey(_requiredContext, _scheduledNodeId);

This gives the best of both worlds:

• Maintainability: Type-safe Keys for managing node lifecycle and acquisition
• Performance: Direct Integer lookups for the high-frequency packet routing hot path

Testing

• Added 7 unit tests for TaskHostNodeKey (equality, hash codes, dictionary keys, large node IDs)
• All 86 existing TaskHost tests pass

Notes

• Removes the NODE_ID_MAX_VALUE_FOR_MULTITHREADED (255) constraint entirely
• SendData(int, INodePacket) throws NotImplementedException as it's required by INodeProvider interface but not used for task hosts (similar to TaskHostNodeManager)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[multithreaded] Refactor Node IDs for taskhost</issue_title>
<issue_description>followup to #12577
Refactor src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs , HandshakeOptions to use record struct as a key to nodes to increase maintanability and remove limitation to 256 nodes.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [multithreaded] Refactor Node IDs for taskhost #12610

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[dotnet-policy-service]

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• rcxvsblobprodcus328.vsblob.vsassets.io
  • Triggering command: /home/REDACTED/work/msbuild/msbuild/artifacts/bin/bootstrap/core/dotnet dotnet test src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj --filter FullyQualifiedName~TaskHost --no-build -v n (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• rcxvsblobprodcus328.vsblob.vsassets.io
  • Triggering command: /home/REDACTED/work/msbuild/msbuild/artifacts/bin/bootstrap/core/dotnet dotnet test src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj --filter FullyQualifiedName~TaskHost --no-build -v n (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• rcxvsblobprodcus328.vsblob.vsassets.io
  • Triggering command: /home/REDACTED/work/msbuild/msbuild/artifacts/bin/bootstrap/core/dotnet dotnet test src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj --filter FullyQualifiedName~TaskHost --no-build -v n (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[JanProvaznik]

@copilot address comments

[JanProvaznik]

is this method needed for anything now? if no, remove

[Copilot]

The SendData(int nodeId, INodePacket packet) method is required by the INodeProvider interface but not actually used for task hosts (task hosts use SendData(TaskHostNodeKey, ...) via TaskHostTask). Similar to TaskHostNodeManager.SendData, keeping it as NotImplementedException to satisfy the interface contract.

[JanProvaznik]

this is a bit sus, but I suppose it works, flagging this for reviewers

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• rcxvsblobprodcus328.vsblob.vsassets.io
  • Triggering command: /home/REDACTED/work/msbuild/msbuild/artifacts/bin/bootstrap/core/dotnet dotnet test src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj --filter FullyQualifiedName~TaskHost --no-build -v n (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

This PR successfully refactors the TaskHost node identification system from fragile bit-packing (limiting builds to 256 nodes) to a clean, type-safe TaskHostNodeKey record struct. The refactoring removes the artificial node limit while improving code maintainability and preserving O(1) packet routing performance.

Key Changes

• Introduced TaskHostNodeKey record struct combining HandshakeOptions and NodeId for type-safe node identification
• Restructured NodeProviderOutOfProcTaskHost dictionaries to separate concerns: acquisition logic uses TaskHostNodeKey, while packet routing uses integer IDs for performance
• Removed bit-packing constants and the GenerateTaskHostNodeId method from TaskHostTask
• Added comprehensive unit tests validating the new key structure including large node IDs beyond the previous 256-node limit

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/Shared/CommunicationsUtilities.cs	Defined TaskHostNodeKey record struct and removed obsolete documentation about bit-packing constraints
src/Build/Instance/TaskFactories/TaskHostTask.cs	Replaced integer node ID with TaskHostNodeKey, removed bit-packing logic and related constants
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs	Refactored dictionary structure with atomic node ID generation, separate key-based and ID-based lookups for optimal performance
src/Build.UnitTests/BackEnd/TaskHostNodeKey_Tests.cs	Added 7 comprehensive tests for equality, hashing, dictionary usage, and validation of unlimited node IDs

Comments suppressed due to low confidence (1)

src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs:615

• Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

            if (!_nodeContexts.ContainsKey(nodeKey))
            {
                nodeCreationSucceeded = CreateNode(nodeKey, factory, handler, configuration, taskHostParameters);
            }
            else
            {
                // node already exists, so "creation" automatically succeeded
                nodeCreationSucceeded = true;
            }

---

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.