Skip to content

Commit 2af5993

Browse files
committed
Avoid action delegate + async state machine allocations on packet header read
1 parent 0072bc6 commit 2af5993

File tree

4 files changed

+26
-12
lines changed

4 files changed

+26
-12
lines changed

src/Build/BackEnd/Client/MSBuildClientPacketPump.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu
203203

204204
// Use a separate reuseable wait handle to avoid allocating on Task.AsyncWaitHandle.
205205
using AutoResetEvent readTaskEvent = new(false);
206-
ValueTask<int> readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length, readTaskEvent);
206+
ValueTask<int> readTask = CommunicationsUtilities.ReadExactlyAsync(localStream, headerByte, headerByte.Length, readTaskEvent);
207207

208208
// Ordering of the wait handles is important. The first signalled wait handle in the array
209209
// will be returned by WaitAny if multiple wait handles are signalled. We prefer to have the
@@ -295,7 +295,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu
295295
else
296296
{
297297
// Start reading the next package header.
298-
readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length, readTaskEvent);
298+
readTask = CommunicationsUtilities.ReadExactlyAsync(localStream, headerByte, headerByte.Length, readTaskEvent);
299299
}
300300
}
301301
break;

src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ public async Task RunPacketReadLoopAsync()
646646
{
647647
try
648648
{
649-
int bytesRead = await CommunicationsUtilities.ReadAsync(_clientToServerStream, _headerByte, _headerByte.Length);
649+
int bytesRead = await CommunicationsUtilities.ReadExactlyAsync(_clientToServerStream, _headerByte, _headerByte.Length);
650650
if (!ProcessHeaderBytesRead(bytesRead))
651651
{
652652
return;

src/Shared/CommunicationsUtilities.cs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -588,16 +588,30 @@ internal static int ReadIntForHandshake(this PipeStream stream, byte? byteToAcce
588588

589589
#if !TASKHOST
590590
/// <summary>
591-
/// Allow interop with EAP / Event-based wait handles without additional allocations.
592-
/// By signalling an external reset event, this allows use of WaitHandle.WaitAny() in non-async/await contexts.
591+
/// This is intended to get around state-machine and wait handle allocations on .NET Framework for async reads.
592+
/// Prefer ReadAsync() when the read is expected to complete synchronously, or if the bytes to read are greater
593+
/// than the stream's buffer and will require multiple reads (e.g. the packet body).
594+
/// By signalling an external reset event, this also allows use of WaitHandle.WaitAny() in non-async/await contexts.
593595
/// </summary>
594-
internal static async ValueTask<int> ReadAsync(Stream stream, byte[] buffer, int bytesToRead, AutoResetEvent autoResetEvent)
596+
internal static ValueTask<int> ReadExactlyAsync(Stream stream, byte[] buffer, int bytesToRead, AutoResetEvent autoResetEvent = null)
595597
{
596-
// Signal to the caller only after the read is complete.
597-
int result = await ReadAsync(stream, buffer, bytesToRead).ConfigureAwait(false);
598-
_ = autoResetEvent.Set();
598+
Task<int> readTask = stream.ReadAsync(buffer, 0, bytesToRead);
599599

600-
return result;
600+
// If the task completed synchronously, directly return the result.
601+
if (readTask.IsCompleted)
602+
{
603+
_ = autoResetEvent?.Set();
604+
return new ValueTask<int>(readTask.Result);
605+
}
606+
607+
// Otherwise, a Task has been allocated and we'll need to set a callback.
608+
readTask = readTask.ContinueWith(static (completedTask, state) =>
609+
{
610+
_ = ((AutoResetEvent)state)?.Set();
611+
return completedTask.Result;
612+
}, autoResetEvent, TaskContinuationOptions.ExecuteSynchronously);
613+
614+
return new ValueTask<int>(readTask);
601615
}
602616

603617
internal static async ValueTask<int> ReadAsync(Stream stream, byte[] buffer, int bytesToRead)

src/Shared/NodeEndpointOutOfProcBase.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ private void RunReadLoop(BufferedReadStream localReadPipe, NamedPipeServerStream
524524
#if !TASKHOST
525525
// Use a separate reuseable wait handle to avoid allocating on Task.AsyncWaitHandle.
526526
using AutoResetEvent readTaskEvent = new(false);
527-
ValueTask<int> readTask = CommunicationsUtilities.ReadAsync(localReadPipe, headerByte, headerByte.Length, readTaskEvent);
527+
ValueTask<int> readTask = CommunicationsUtilities.ReadExactlyAsync(localReadPipe, headerByte, headerByte.Length, readTaskEvent);
528528
#else
529529
IAsyncResult result = localReadPipe.BeginRead(headerByte, 0, headerByte.Length, null, null);
530530
#endif
@@ -617,7 +617,7 @@ private void RunReadLoop(BufferedReadStream localReadPipe, NamedPipeServerStream
617617
}
618618

619619
#if !TASKHOST
620-
readTask = CommunicationsUtilities.ReadAsync(localReadPipe, headerByte, headerByte.Length, readTaskEvent);
620+
readTask = CommunicationsUtilities.ReadExactlyAsync(localReadPipe, headerByte, headerByte.Length, readTaskEvent);
621621
#else
622622
result = localReadPipe.BeginRead(headerByte, 0, headerByte.Length, null, null);
623623
handles[0] = result.AsyncWaitHandle;

0 commit comments

Comments
 (0)