-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Implement simplified task routing for multi-threaded execution mode #12384
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.
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
base: main
Are you sure you want to change the base?
Conversation
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. |
… and tests Co-authored-by: JanProvaznik <[email protected]>
…perty Co-authored-by: JanProvaznik <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot keep it simple
src/Build/BackEnd/Components/RequestBuilder/ThreadSafeTaskRegistry.cs
Outdated
Show resolved
Hide resolved
…y tests Co-authored-by: JanProvaznik <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjust the testing strategy @copilot
{ | ||
/// <summary> | ||
/// Registry that maintains an allowlist of tasks that are known to be thread-safe | ||
/// and can be executed within thread nodes in multi-threaded execution mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rewrite the documentation of this file noting it is a temporary solution before we adopt a Thread safe task interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the documentation to clarify this is a temporary solution before adopting a thread-safe task interface. The comments now explain that this is an interim allowlist approach that will be replaced with interface-based detection once a formal thread-safe task interface is established. Commit 8811159
/// Verify that non-thread-safe tasks execute out-of-process when multi-threaded execution is enabled. | ||
/// </summary> | ||
[Fact] | ||
public void NonThreadSafeTasksExecuteOutOfProcessWhenMultiThreadedExecutionEnabled() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see tests that prove a safe task (Message) actually runs in another process. And that an unsafe one (Copy) doesn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhanced the testing strategy with integration tests that actually execute Message and Copy tasks using BuildManager with MultiThreaded=true. The new tests verify:
- Message task executes in-process (thread-safe routing)
- Copy task executes out-of-process (non-thread-safe routing)
- Both tasks complete successfully under multithreaded execution
Also added comprehensive parameterized tests covering all routing scenarios. Commit 8811159
…egy with integration tests Co-authored-by: JanProvaznik <[email protected]>
Adjusted the testing strategy to include:
The new approach provides better coverage of the actual behavior while maintaining focused, maintainable tests. Commit 8811159 |
This PR implements a simplified task routing mechanism for multi-threaded execution mode as described in the multithreaded MSBuild specification.
Overview
The implementation provides a foundation for routing tasks based on thread safety when using the existing
/multithreaded
command-line switch. Thread-safe tasks execute within thread nodes (in-process), while non-thread-safe tasks are routed to sidecar taskhost processes.Key Components
ThreadSafeTaskRegistry
FrozenSet<string>
for optimal performanceMessage
,Warning
,Error
,ItemGroup
,PropertyGroup
,CallTarget
,MSBuild
IsTaskThreadSafe()
methodTask Routing Logic
ShouldTaskExecuteOutOfProc()
method toTaskHost
classMultiThreaded = false
→ use default behavior (no change to existing builds)MultiThreaded = true
→ thread-safe tasks run in-process, others go to sidecar taskhostIntegration with Existing Infrastructure
BuildParameters.MultiThreaded
property/multithreaded
(or/mt
) command-line switchUsage
Enable task routing using the existing experimental multi-threaded switch:
Testing
The implementation includes focused unit tests covering:
Manual testing confirms that:
Future Work
This provides the foundation for the multi-threaded execution mode. As tasks are migrated to implement the new thread-safe interface (per #11828), they can be added to the allowlist to benefit from in-process execution.
Fixes #12196.
💡 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.