Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

RAR node: Buffer log events to client - #12558

#12558
Merged
YuliiaKovalova merged 2 commits into
dotnet:maindotnet/msbuild:mainfrom
ccastanedaucf:dev/chcasta/rar-buffered-loggingccastanedaucf/msbuild:dev/chcasta/rar-buffered-loggingCopy head branch name to clipboard
Nov 20, 2025
Merged

RAR node: Buffer log events to client#12558
YuliiaKovalova merged 2 commits into
dotnet:maindotnet/msbuild:mainfrom
ccastanedaucf:dev/chcasta/rar-buffered-loggingccastanedaucf/msbuild:dev/chcasta/rar-buffered-loggingCopy head branch name to clipboard

Conversation

@ccastanedaucf

@ccastanedaucf ccastanedaucf commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

Implements logging for out-of-proc RAR node.

Context

Since RAR is often the largest source of events in MSBuild, some complexity is needed to avoid introducing
overhead from excessive allocations and task context-switching. The main goals here are:

  • Logging an event should not block RAR, as IPC can be performed async.
  • Events should be batched to reduce the number of IPC calls. However, too large of a batch will add latency to
    the final response packet.
  • Pending events should be flushed when RAR completes, but before sending the result packet.
  • Buffers should be resusable between packets, as batches are a fixed size.
  • The processing task should only wake when a full packet is ready, so that each read of the event channel
    will immediately return a struct ValueTask.

As such, two channels are used: one for events, and one for the batch size.

When an event is queued, a pending count is incremented. When the count reaches the batch threshold, or the queue
is explicitly flushed, the count is written to the count channel and reset.

Once RAR completes, events are flushed with the remaining count.

Copilot AI review requested due to automatic review settings September 22, 2025 17:40

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 implements buffered logging for the out-of-process RAR (ResolveAssemblyReference) node to improve performance by reducing IPC overhead. The implementation uses channels to batch log events asynchronously before sending them to the client.

  • Adds a buffering mechanism using channels to batch log events before IPC transmission
  • Refactors LogMessagePacketBase from abstract to concrete to enable reuse across different assemblies
  • Updates project references to consolidate logging packet handling

Reviewed Changes

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

Show a summary per file
File Description
src/Tasks/Microsoft.Build.Tasks.csproj Adds new log event buffering class and moves Threading.Channels dependency to common section
src/Tasks/AssemblyDependency/Node/RarNodeBuildEngine.cs Implements channel-based event buffering with async processing
src/Tasks/AssemblyDependency/Node/RarNodeBufferedLogEvents.cs New packet type for batched log events
src/Tasks/AssemblyDependency/Node/OutOfProcRarNodeEndpoint.cs Integrates async log processing into the RAR endpoint lifecycle
src/Tasks/AssemblyDependency/Node/OutOfProcRarClient.cs Handles replay of buffered log events to the real build engine
src/Tasks.UnitTests/AssemblyDependency/Node/RarNodeExecuteRequest_Tests.cs Updates tests to accommodate new constructor requirements
src/Shared/LogMessagePacketBase.cs Refactors from abstract to concrete class with virtual methods for customization
src/MSBuildTaskHost/MSBuildTaskHost.csproj Removes redundant LogMessagePacket reference
src/MSBuild/OutOfProcTaskHostNode.cs Updates to use concrete LogMessagePacketBase
src/MSBuild/MSBuild.csproj Removes LogMessagePacket.cs from project
src/MSBuild/LogMessagePacket.cs File deleted as functionality moved to shared base
src/Build/BackEnd/Components/Communications/LogMessagePacket.cs Moves specialized serialization logic from base class to concrete implementation

Comment thread src/Tasks/AssemblyDependency/Node/RarNodeBuildEngine.cs Outdated
Comment thread src/Tasks/AssemblyDependency/Node/OutOfProcRarClient.cs
@ccastanedaucf
ccastanedaucf marked this pull request as draft September 22, 2025 17:47
@ccastanedaucf
ccastanedaucf force-pushed the dev/chcasta/rar-buffered-logging branch from 62d635a to 74bb593 Compare November 18, 2025 23:40
@ccastanedaucf
ccastanedaucf marked this pull request as ready for review November 18, 2025 23:40
@ccastanedaucf

Copy link
Copy Markdown
Contributor Author

fyi this is identical to the drafted version, just minus the duplicated commits that are now merged to main

Comment thread src/Tasks/AssemblyDependency/Node/RarNodeBuildEngine.cs
@YuliiaKovalova
YuliiaKovalova merged commit f62465c into dotnet:main Nov 20, 2025
10 checks passed
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.