Change IDictionary<string, string> to TaskHostParameters#12620
Merged
YuliiaKovalova merged 15 commits intomainfrom Nov 4, 2025
Merged
Change IDictionary<string, string> to TaskHostParameters#12620YuliiaKovalova merged 15 commits intomainfrom
YuliiaKovalova merged 15 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the use of IDictionary<string, string> with a new TaskHostParameters readonly struct for passing task identity parameters throughout the MSBuild system. The change improves type safety by eliminating string-based dictionary keys that were prone to typos and runtime validation errors, while also providing better performance through reduced allocations and clearer API design.
Key changes:
- Introduces the new
TaskHostParametersstruct with strongly-typed properties - Updates
ITaskFactory2interface with new overloads usingTaskHostParameters - Refactors internal task registry and execution logic to use the new type
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/TaskHostParameters.cs | New readonly struct defining task host parameters with runtime, architecture, and .NET host paths |
| src/Framework/ITaskFactory2.cs | Adds new overloads using TaskHostParameters and marks old dictionary-based methods as obsolete |
| src/Framework/BinaryTranslator.cs | Implements serialization support for TaskHostParameters |
| src/Build/Instance/TaskRegistry.cs | Updates task registration and identity comparison logic to use TaskHostParameters |
| src/Shared/CommunicationsUtilities.cs | Refactors handshake options generation to work with TaskHostParameters |
| src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs | Major refactoring to use TaskHostParameters throughout task instantiation logic |
Comments suppressed due to low confidence (1)
src/Build/Instance/TaskRegistry.cs:539
- Missing closing brace for the VerifyThrowIdentityParametersValid method. The method structure appears incomplete.
}
baronfel
reviewed
Oct 8, 2025
baronfel
reviewed
Oct 8, 2025
Refactor task factory parameter merging to check for task host factory conditionally.
YuliiaKovalova
force-pushed
the
dev/ykovalova/infer_task_host_and_arch
branch
from
cbb2a71 to
2dae9bd
Compare
rainersigwald
approved these changes
Oct 28, 2025
This was referenced Nov 5, 2025
YuliiaKovalova
added a commit
that referenced
this pull request
Nov 6, 2025
)" This reverts commit 25f6112.
YuliiaKovalova
added a commit
that referenced
this pull request
Nov 10, 2025
This change introduces a new read only struct that is used in Dictionary for caching in TaskRegistry #12620 Without a proper equality check it spawns a new process due to mismatch. Experimental insertion without extra allocation reported: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/686770
This was referenced Nov 19, 2025
YuliiaKovalova
pushed a commit
that referenced
this pull request
Nov 25, 2025
### Context Sidecar taskhost was accidentally disabled in #12620 ### Changes Made Re-enabled sidecar taskhost functionality. ### Testing Only unit tests.
ViktorHofer
added a commit
that referenced
this pull request
Jan 14, 2026
If the task host factory is explicitly requested, do not act as a long lived task host. This is important as customers use task host factories for short lived tasks to release potential locks after the build. This goes back to the previous behavior. This regressed with #12620
ViktorHofer
added a commit
that referenced
this pull request
Jan 14, 2026
If the task host factory is explicitly requested, do not act as a long lived task host. This is important as customers use task host factories for short lived tasks to release potential locks after the build. This goes back to the previous behavior. This regressed with #12620
ViktorHofer
added a commit
that referenced
this pull request
Jan 14, 2026
If the task host factory is explicitly requested, do not act as a long lived task host. This is important as customers use task host factories for short lived tasks to release potential locks after the build. This goes back to the previous behavior. This regressed with #12620
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12287 , #12686 and #12683
Context
The previous implementation used IDictionary<string, string> to pass task identity parameters throughout the system. This approach had several drawbacks:
What Changed
New TaskHostParameters readonly struct was added.
Main Changes:
Testing
The functionality is well covered by existing task factory tests