[efficiency-improver] perf: encode JSON body once in TcpMessageHandler.WriteRequestAsync#8720
Draft
Evangelink wants to merge 1 commit into
Draft
Conversation
Previously WriteRequestAsync called Encoding.UTF8.GetByteCount(messageStr) to get the Content-Length, then StreamWriter.WriteAsync(messageStr) which internally re-encoded the string to UTF-8 — scanning the string twice. On NETCOREAPP: encode to a pooled ArrayPool<byte> buffer once, use .Length for the header, flush the StreamWriter (headers), then write the raw bytes directly to the underlying stream — 1 encoding instead of 2. On netstandard2.0: same approach but with a heap-allocated byte[], avoiding GetByteCount + re-encode. Still goes from 2 encodings to 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the server-mode JSON-RPC write path by encoding serialized request bodies to UTF-8 bytes explicitly before writing them to the TCP stream.
Changes:
- Replaces
StreamWriter.WriteAsync(messageStr)for the JSON body with direct writes to the underlying stream. - Uses
ArrayPool<byte>on NETCOREAPP targets and a singleGetBytesallocation on non-NETCOREAPP targets. - Preserves existing header formatting and flushing behavior while reducing body-write overhead.
Show a summary per file
| File | Description |
|---|---|
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/TcpMessageHandler.cs |
Updates WriteRequestAsync to pre-encode the JSON body and write bytes directly to the TCP stream. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
Comment on lines
+124
to
+125
| // Encode the message body once to avoid scanning the string twice: | ||
| // once in GetByteCount (for the Content-Length header) and once via StreamWriter encoding. |
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.
🤖 This PR was created by Efficiency Improver, an automated AI assistant focused on reducing energy consumption and computational footprint.
Goal and Rationale
In
TcpMessageHandler.WriteRequestAsync, the serialized JSON message string was processed in two separate steps:Encoding.UTF8.GetByteCount(messageStr)— scans the string to compute the byte count for theContent-LengthheaderStreamWriter.WriteAsync(messageStr)— internally re-encodes the string to UTF-8 bytes for writingThis PR replaces the two-step approach: on netstandard2.0 we call
Encoding.UTF8.GetBytes(messageStr)once (getting both bytes and length), and on NETCOREAPP we callGetByteCount+GetBytesinto a pooledArrayPool<byte>buffer, then write directly to the underlying stream in both cases.Focus Area
Code-Level Efficiency — redundant CPU work (double string encoding) and unnecessary StreamWriter buffering in the server-mode JSON-RPC message write path.
Approach
NETCOREAPP path (before → after):
GetByteCount(messageStr)(1 scan) +StreamWriter.WriteAsync(messageStr)(1 internal encode + buffer alloc) = 2 scans, StreamWriter heap bufferGetByteCount+GetBytesintoArrayPool<byte>buffer (2 scans, 0 heap allocs) + directBaseStream.WriteAsync= same 2 scans, 0 heap allocationsnetstandard2.0 path (before → after):
GetByteCount(messageStr)(1 scan) +StreamWriter.WriteAsync(messageStr)(1 internal encode + alloc) = 2 scans, 2 allocationsEncoding.UTF8.GetBytes(messageStr)(1 scan, 1 byte[] allocation) + directBaseStream.WriteAsync= 1 scan, 1 allocationThe key improvement on netstandard2.0 is eliminating one full string scan. On NETCOREAPP, we eliminate the StreamWriter's internal char→byte transcoding buffer heap allocation.
Energy Efficiency Evidence
Proxy metric: CPU instruction count (fewer string traversals) and heap allocation (fewer GC-triggering byte buffer allocations = less DRAM energy from GC pressure).
Call frequency:
WriteRequestAsyncis called once per JSON-RPC message in server mode (IDE-driven test runs via--server). Every test result, discovery result, and progress heartbeat goes through this path. In a large test suite, this fires thousands of times per session.Green Software Foundation — Hardware Efficiency: removing a full string re-traversal per RPC message reduces CPU work per functional unit (one message delivered). Fewer heap allocations reduce GC-induced stop-the-world pauses that waste proportional CPU energy.
Trade-offs
The code is slightly more complex due to the
ArrayPoolpattern on NETCOREAPP. The inline comment explains the rationale. The wire format is identical — the same UTF-8-encoded JSON body is written to the TCP stream.Test Status
✅ Build succeeded
✅ All unit tests passed (
./build.sh -test)Add this agentic workflows to your repo
To install this agentic workflow, run