Stop TerminalLogger from opting into task input logging

[Copilot]

Context

TerminalLogger.Initialize and ForwardingTerminalLogger.Initialize opt into IEventSource3.IncludeTaskInputs(), but neither class ever consumes TaskParameterEventArgs. The opt-in flips LogTaskInputs = true in the engine (ProjectInstance.cs:2822), forcing serialization of every task's parameters — including large ItemGroups with metadata — into events on every task invocation.

On builds with hundreds of MSBuild task invocations (e.g. dotnet/source-build-assets), this is what produces the reported ~100× slowdown vs. the console logger, which never opts in.

Changes Made

• Removed the unused eventSource3.IncludeTaskInputs() call from TerminalLogger.Initialize.
• Removed the same unused call from ForwardingTerminalLogger.Initialize.
• Kept the IncludeEvaluationPropertiesAndItems() opt-in — it's actually consumed via ProjectEvaluationFinishedEventArgs.EnumerateProperties().

Testing

• Full repo build is clean (0 warnings).
• All 71 TerminalLogger_Tests pass.

Notes

A grep over src/Build/Logging/TerminalLogger/ confirms no reference to TaskParameterEventArgs or task parameter consumption — the opt-in was dead overhead. TaskCommandLineEventArgs (used by _showCommandLine) is unrelated and not gated by IncludeTaskInputs.

[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.

[baronfel]

I'd be hesitant to take this - several of my draft PRs build on Task input detection in order to enable nicer end user UX.

If we do disable this because of perf we'd just reenable it on those PRs. Is the cost all of the IPC? If so, then MT should solve that problem? Or keep this code and instead make the 'forwarding logger' part of the TL today drop all Task input events at the individual worker node to prevent them being sent to the central logger node.

[ViktorHofer]

I see two options:

1. Keep task input logging enabled and live with the extremely poor performance (and opt-out at least source-build-assets from using /tl).
2. Disable task input logging as it isn't used today, establish perf tests and only re-enable later when performance expectations match (i.e. with MT mode).

[ViktorHofer]

@copilot do you have more information to share in response to @baronfel's statement above?

[Copilot]

@copilot do you have more information to share in response to @baronfel's statement above?

Yes — the cost is not just IPC, and that's the key reason filtering in the forwarding logger isn't a complete fix.

The hot gate is in TaskExecutionHost.InitializeTaskScalarParameter / InitializeTaskItemParameter (and the output-gathering paths) at src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs:1402:

if (LogTaskInputs && !_taskLoggingContext.LoggingService.OnlyLogCriticalEvents)
{
    ...
    ItemGroupLoggingHelper.LogTaskParameter(...);
}

When LogTaskInputs=true, for every task input and output the engine:

1. Allocates a TaskParameterEventArgs,
2. Copies every item into a TaskItemData[] along with all of its metadata (ItemGroupLoggingHelper.CreateTaskParameterEventArgs → metadata cloning),
3. Dispatches the event through the logging service to every sink in-process, regardless of whether anyone consumes it.

Steps 1–3 happen on the worker even before any IPC. With hundreds of MSBuild task invocations passing big item lists (the source-build-assets targetPacks loop), this allocation/copy work dominates — and it shows up at /m:1 too, where there is no IPC. So:

• "Is the cost all of the IPC?" No. IPC adds to it on multi-node, but the engine-side construction is the bulk.
• MT mode removes the IPC tier but not the per-task TaskParameterEventArgs construction (the LogTaskInputs gate is upstream of the dispatch).
• Filter in ForwardingTerminalLogger would help IPC and central-node deserialization, but the worker still pays the per-task allocation/copy. It also doesn't help /m:1.

The reason flipping IncludeTaskInputs off is so impactful is precisely that it short-circuits step (1) at the source: LoggingService.IncludeTaskInputs is the OR over sinks (LoggingService.cs:573), and BuildManager uses it to set parameters.LogTaskInputs (ProjectInstance.cs:2822), which is the gate above.

Re: your draft PRs that do consume task-input events — agreed, and I don't want to block that. Two options that compose with this PR:

1. Land this for now (TL doesn't consume the events today, so it's pure overhead) and re-enable IncludeTaskInputs() in your PR when there's an actual consumer. The opt-in is one line.
2. Keep the opt-in but make it conditional on a TL feature flag / verbosity, so it's only paid when the UX feature that needs it is active.

I'm happy to switch to (2) if you'd prefer — just say the word and tell me which trigger you want (verbosity ≥ Detailed? a tlp: parameter? always-on once your PRs land?). Otherwise I'd lean toward (1) since it's the smallest change and the perf delta is large.

[baronfel]

OK, that's a fine analysis. We can land this for now.

Dismissing based on copilot feedback.

[baronfel]

One thing to note for future work - when in MT mode it seems highly likely that message creation could be closer to zero-copy, so we should look at making message creation more like a 'view' over data that is 'hydrated' only in the out-of-proc IPC scenario.

[ViktorHofer]

/review

[github-actions]

✅ Expert Code Review (command) completed successfully!

Expert review of PR #13686 completed by subagent. The PR is a correct, clean fix removing unused IncludeTaskInputs() opt-ins from TerminalLogger and ForwardingTerminalLogger that caused ~100× build slowdowns. The subagent verified the claims, investigated the codebase, and submitted a COMMENT review directly on the PR. No additional action needed from the orchestrator.

[Copilot]

Pull request overview

This PR removes the IEventSource3.IncludeTaskInputs() opt-in from both TerminalLogger and ForwardingTerminalLogger, preventing MSBuild from globally enabling task-parameter event emission (TaskParameterEventArgs) when the terminal logger is present. This aligns logger opt-ins with actual event consumption and avoids significant overhead from serializing large task input payloads.

Changes:

• Removed IncludeTaskInputs() from TerminalLogger.Initialize since TaskParameterEventArgs are not consumed.
• Removed IncludeTaskInputs() from ForwardingTerminalLogger.Initialize for the same reason.
• Kept IEventSource4.IncludeEvaluationPropertiesAndItems() since evaluation properties/items are consumed via ProjectEvaluationFinishedEventArgs.

Show a summary per file

File	Description
src/Build/Logging/TerminalLogger/TerminalLogger.cs	Stops terminal logger from opting into task-input events it never consumes, reducing task-parameter logging overhead.
src/Build/Logging/TerminalLogger/ForwardingTerminalLogger.cs	Stops forwarding terminal logger from opting into task-input events it never consumes, reducing overhead on node builds as well.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0