Skip to content

Add new multithreaded APIs - #12625

Merged
AR-May merged 5 commits into
dotnet:mainfrom
AR-May:add-mt-apis
Oct 14, 2025
Merged

AR-May merged 5 commits into
dotnet:mainfrom
AR-May:add-mt-apis

Conversation

@AR-May

@AR-May AR-May commented Oct 8, 2025

Copy link
Copy Markdown
Member

Related to #11828

Changes:
This PR introduces the core APIs that enable thread-safe task execution in MSBuild's multithreaded execution model. These APIs provide safe alternatives to global process state operations, allowing IMultithreadable tasks to run concurrently within a single MSBuild process.

Node:
Adding multithreaded APIs in order to unblock #12617
Extracted APIs from #12608

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces core APIs to enable thread-safe task execution in MSBuild's multithreaded execution model. The changes provide safe alternatives to global process state operations, allowing IMultithreadable tasks to run concurrently within a single MSBuild process.

Key changes:

  • New TaskEnvironment class providing thread-safe environment variable and path operations
  • AbsolutePath struct for safe path handling
  • IMultiThreadableTask interface for tasks that can execute in parallel
  • MSBuildMultiThreadableTaskAttribute for marking existing tasks as thread-safe

Reviewed Changes

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

Show a summary per file
File Description
src/Framework/TaskEnvironment.cs Core API providing thread-safe environment operations and path resolution
src/Framework/PathHelpers/AbsolutePath.cs Immutable struct representing absolute file system paths
src/Framework/IMultiThreadableTask.cs Interface for tasks capable of thread-safe execution
src/Framework/MSBuildMultiThreadableTaskAttribute.cs Attribute for marking existing tasks as thread-safe
documentation/specs/multithreading/thread-safe-tasks.md Updates to specification removing virtual modifiers

Comment thread src/Framework/IMultiThreadableTask.cs
Comment thread src/Framework/MSBuildMultiThreadableTaskAttribute.cs Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@AR-May

AR-May commented Oct 9, 2025

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@AR-May

AR-May commented Oct 9, 2025

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@JanProvaznik JanProvaznik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just venting some thoughts about doc comments, the shape of the code is good, approved, but needs another reviewer

Comment thread src/Framework/IMultiThreadableTask.cs Outdated
Comment thread src/Framework/IMultiThreadableTask.cs Outdated
Comment thread src/Framework/IMultiThreadableTask.cs Outdated
Comment thread src/Framework/IMultiThreadableTask.cs Outdated
Comment thread src/Framework/MSBuildMultiThreadableTaskAttribute.cs
@AR-May
AR-May requested a review from baronfel October 13, 2025 14:15
@baronfel

Copy link
Copy Markdown
Member

I added a bit of fit-and-finish to the existing API docs because I couldn't help myself.

@JanProvaznik JanProvaznik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

ViktorHofer added a commit to dotnet/dotnet that referenced this pull request Aug 21, 2026
Under MSBuild's multi-threaded mode (-mt), TaskRouter routes every task that
lacks [MSBuildMultiThreadableTask] to an out-of-proc sidecar TaskHost. In the
VMR runtime inner build this raised TaskHost invocations from 849 to 14,125 and
added ~98s of net task time, cancelling out most of the expected -mt savings.

Analysis
--------
Every concrete Arcade task (136 total) was checked against the rule set from
dotnet/msbuild src/TaskAnalyzer - the MSBuildTask0001/0002/0004 banned-API
tables plus static mutable state - using a Roslyn walker with transitive
call-chain analysis, so violations reached through shared helpers are caught.

MSBuildTask0003 (relative paths) is not treated as a blocker: MSBuild's own
Copy, Exec, Unzip, ZipDirectory and every ToolTask-derived tool task carry the
attribute. The bar applied here is no process-global state (env vars, cwd,
Path.GetFullPath/GetTempPath, Console.*, Process.Start) and no unsynchronized
static mutable state.

44 tasks came back clean and are annotated. The remaining 92 need code changes
and are tracked in dotnet/arcade#17378; most are blocked by a shared helper
(PackageIndex, LocateDotNet, the Helix/AzDO client stack, the publishing stack)
rather than by the task itself.

MSBuild package bump
--------------------
MSBuildMultiThreadableTaskAttribute was added in dotnet/msbuild#12625 and first
shipped in 18.3.x, so the 17.12.50 pin could not compile it. Bumped to 18.4.0,
which ships net472 and net10.0 libs matching Arcade's NetMinimum. This is
runtime-safe on older hosts: AssemblyTaskFactory only calls
TaskRouter.NeedsTaskHostInMultiThreadedMode when BuildParameters.MultiThreaded
is true, so a 17.x host never enumerates the attribute.

TargetFrameworkResolver race
----------------------------
TargetFrameworkResolver.CreateOrGet used an unsynchronized static readonly
Dictionary with TryGetValue/Add. Switched to ConcurrentDictionary.GetOrAdd,
which is both a correctness fix and what unblocks the two hottest tasks -
ChooseBestP2PTargetFrameworkTask (1740 invocations, +16.1s under -mt) and
ChooseBestTargetFrameworksTask (1195 invocations, +12.2s), together roughly 35%
of the measured TaskHost overhead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants