[wip] TaskHost IBuildEngine Callback Support spec for discussion #12868
Conversation
rainersigwald
left a comment
rainersigwald
left a comment
There was a problem hiding this comment.
looks pretty good.
Should we add any ETW events as part of this? What about telemetry?
| ## 1. Problem Statement | ||
|
|
||
| The MSBuild TaskHost (`OutOfProcTaskHostNode`) implements `IBuildEngine10` but lacks support for several callbacks. TaskHost is used when: | ||
| 1. **`MSBUILDFORCEALLTASKSOUTOFPROC=1`** with `-mt` mode - forces non-thread-safe tasks out-of-proc |
There was a problem hiding this comment.
Aren't these separate cases?
| 1. **`MSBUILDFORCEALLTASKSOUTOFPROC=1`** with `-mt` mode - forces non-thread-safe tasks out-of-proc | |
| 1. **`MSBUILDFORCEALLTASKSOUTOFPROC=1`** | |
| 2. `-mt` mode - forces non-thread-safe tasks out-of-proc |
| - A) Forward to parent and actually yield (allows scheduler to run other work) | ||
| - B) Keep as no-op (current behavior, safest) | ||
|
|
||
| **Recommendation:** (B) initially - Yield/Reacquire are rarely used by tasks, and current no-op behavior has shipped. Revisit if real-world need arises. |
There was a problem hiding this comment.
I agree, though I think the most expensive tasks do tend to yield. Definitely something that can wait.
|
|
||
| | Phase | Scope | Risk | Effort | | ||
| |-------|-------|------|--------| | ||
| | 1 | `IsRunningMultipleNodes` | Low | 2 days | |
There was a problem hiding this comment.
Still unsure if this is worth forwarding the request along vs hardcoding "yeah if you're in this mode assume parallelism".
There was a problem hiding this comment.
Is -mt /m:1 (or just /mt for msbuild.exe) the only way to hit this condition? What is the practical impact on the Tasks we have that use IsRunningMultipleNodes? Do we know if any 1P Tasks care about this condition?
There was a problem hiding this comment.
it's /m:1 + a taskhost for any reason (mt + unenlightened task, env var that forces everything to a task host, explicit taskhost request in usingtask).
I can't imagine that it's widely used. I see one use in our repo in the task version of the MSBuild task that changes how StopOnFirstFailure works, which is itself a silly thing to do . . .
| | 1 | `IsRunningMultipleNodes` | Low | 2 days | | ||
| | 2 | `RequestCores`/`ReleaseCores` | Medium | 3 days | | ||
| | 3 | `Yield`/`Reacquire` | Medium | 3 days | | ||
| | 4 | `BuildProjectFile*` | High | 5-7 days | |
There was a problem hiding this comment.
BuildProjectFile is the most critical one, for XAML, right?
3 participants
#12863
Changes Made
spec only