[efficiency-improver] perf: eliminate Duration.ToString() and Guid.ToString() allocations in IPC serializers#16216
Conversation
…n IPC serializers Replace heap-allocating ToString() calls with zero-allocation alternatives on the IPC serialization hot path (called for every TestResult and TestCase crossing the process boundary): - TestResultConverterV2.Write / TestResultConverter.Write: replace value.Duration.ToString() with TimeSpan.TryFormat + stackalloc char[32] buffer, then write via WriteString/WriteStringValue(ReadOnlySpan<char>). Eliminates one heap string allocation (~26-50 bytes) per TestResult on the IPC write path. - TestCaseConverter.Write: replace writer.WriteStringValue(value.Id.ToString()) with writer.WriteStringValue(value.Id) (the Guid overload). Eliminates one heap string allocation (~72-88 bytes) per TestCase on the IPC write path. The V2 converter already used this pattern; now V1 matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces allocations on the Microsoft.TestPlatform.CommunicationUtilities IPC JSON serialization write path by avoiding per-item TimeSpan.ToString() and Guid.ToString() string allocations when serializing TestResult.Duration and TestCase.Id across the vstest.console ↔ testhost boundary.
Changes:
- Replace
TimeSpan.ToString()forTestResult.DurationwithTimeSpan.TryFormatinto astackallocbuffer, written via span-basedUtf8JsonWriterAPIs (V1 and V2 converters). - Replace
Guid.ToString()forTestCase.IdwithUtf8JsonWriter.WriteStringValue(Guid)(V1 converter), matching the already-optimized V2 path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestResultConverterV2.cs | Eliminates Duration.ToString() allocation by formatting into a stack buffer and writing via span overload. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestResultConverter.cs | Same Duration stack-buffer formatting change for the v1 protocol serializer. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestCaseConverter.cs | Eliminates Guid.ToString() allocation by using Utf8JsonWriter’s Guid overload for v1 protocol serialization. |
nohwnd
left a comment
There was a problem hiding this comment.
Review: perf: eliminate Duration.ToString() and Guid.ToString() allocations in IPC serializers
Dimensions activated: IPC Transport & Protocol Stability · Backward Compatibility & Rollback Safety · Defensive Coding at Boundaries
Wire Compatibility ✅
Both changes produce byte-for-byte identical JSON output — verified against the IPC contract:
| Change | Before | After | Identical? |
|---|---|---|---|
TestCaseConverter — Guid |
Guid.ToString() → "D" format, lowercase |
WriteStringValue(Guid) → "D" format internally |
✅ Yes |
TestResultConverter — Duration |
TimeSpan.ToString() → "c" (constant) format |
TryFormat(..., default, ...) → "c" format (empty format sentinel defaults to "c") |
✅ Yes |
TestResultConverterV2 — Duration |
same | same | ✅ Yes |
The deserialization paths (TimeSpan.Parse(..., CultureInfo.InvariantCulture) and GuidPolyfill.Parse) are unchanged and accept both the old and new serialized forms without modification.
Cross-TFM Compatibility ✅
All three files are entirely guarded by #if NETCOREAPP. Span<T>, stackalloc char[N], TimeSpan.TryFormat, and Utf8JsonWriter.WriteStringValue(ReadOnlySpan<char>) are all available on every NETCOREAPP target. No net462/netstandard2.0 exposure.
Finding: TryFormat return value ignored ⚠️
See inline comments on TestResultConverter.cs:165 and TestResultConverterV2.cs:129.
The 32-char buffer is provably sufficient today — TimeSpan.MinValue serializes to 27 chars, leaving 5 chars of headroom. But TryFormat returning false silently writes an empty Duration string ("") into the IPC stream, causing TimeSpan.Parse to throw FormatException on the receiving side. Adding Debug.Assert would make the invariant explicit and surface any future regression immediately in debug builds. Not a correctness bug with the current code; flagged as a hardening suggestion.
PR Description Alignment ✅
The title and description accurately describe the changes. The claimed format equivalence ("byte-for-byte identical"), buffer size analysis (26 chars for MaxValue, 32 headroom), and protocol version scope (V1 + V2 converters) all match the diff exactly. The note that TestCaseConverterV2 already used the Guid overload is correct.
🧠 Reviewed by expert-reviewer workflow · Dimensions: IPC Transport & Protocol Stability, Backward Compatibility & Rollback Safety, Defensive Coding
🧠 Reviewed by Expert Code Reviewer 🧠
| writer.WriteString("ComputerName", value.ComputerName); | ||
| writer.WriteString("Duration", value.Duration.ToString()); | ||
| Span<char> durationBuf = stackalloc char[32]; | ||
| value.Duration.TryFormat(durationBuf, out int durationChars, default, CultureInfo.InvariantCulture); |
There was a problem hiding this comment.
[Defensive Coding / IPC Transport] Same TryFormat return-value concern as in TestResultConverter.cs: if the call returns false, durationChars is 0, the Duration field is written as "" into the IPC stream, and TimeSpan.Parse on the receiving side throws a FormatException.
Same mitigation applies — capture the return value and add a Debug.Assert:
Span<char> durationBuf = stackalloc char[32];
bool formatted = value.Duration.TryFormat(durationBuf, out int durationChars, default, CultureInfo.InvariantCulture);
Debug.Assert(formatted, "TimeSpan.TryFormat failed — buffer too small");
writer.WriteString("Duration", durationBuf[..durationChars]);| WriteProperty(writer, TestResultProperties.Duration, options); | ||
| writer.WriteStringValue(value.Duration.ToString()); | ||
| Span<char> durationBuf = stackalloc char[32]; | ||
| value.Duration.TryFormat(durationBuf, out int durationChars, default, CultureInfo.InvariantCulture); |
There was a problem hiding this comment.
[Defensive Coding / IPC Transport] The return value of TryFormat is silently discarded.
value.Duration.TryFormat(durationBuf, out int durationChars, default, CultureInfo.InvariantCulture);If TryFormat returns false (buffer too small), durationChars is set to 0 and durationBuf[..0] is an empty span. WriteStringValue(ReadOnlySpan<char>.Empty) then writes "" into the IPC wire stream — and the receiving side's TimeSpan.Parse("", CultureInfo.InvariantCulture) throws a FormatException, resulting in a silent IPC deserialization failure that's extremely hard to diagnose.
The 32-char buffer is provably sufficient today (TimeSpan.MinValue = "-10675199.02:48:05.4775808" = 27 chars), but a future format-specifier change or a code change that reuses this pattern with a different type could trigger the failure path silently.
Suggestion — make the intent explicit and crash-fast on a broken invariant:
Span<char> durationBuf = stackalloc char[32];
bool formatted = value.Duration.TryFormat(durationBuf, out int durationChars, default, CultureInfo.InvariantCulture);
Debug.Assert(formatted, "TimeSpan.TryFormat failed — buffer too small");
writer.WriteStringValue(durationBuf[..durationChars]);
Goal and Rationale
Eliminate two categories of heap allocation on the IPC serialization hot path — called for every
TestResultandTestCasecrossing the vstest.console ↔ testhost process boundary.Focus area: Network & I/O Efficiency (IPC serialization write path)
Problem
1.
Duration.ToString()—TestResultConverterV2andTestResultConverterTimeSpan.ToString()always allocates a heap string (typically 7–26 bytes UTF-16 encoded). This string is immediately consumed byWriteString/WriteStringValueand then eligible for GC. It serves no purpose beyond bridging the absence of aWriteString(string, TimeSpan)overload.2.
Guid.ToString()—TestCaseConverter(v1 protocol)Guid.ToString()allocates a 72–88 byte heap string (36-char UUID).Utf8JsonWriteralready has aWriteStringValue(Guid)overload that writes the UUID directly from the struct, without any heap allocation. The V2 converter (TestCaseConverterV2) already uses this overload (line 79); the V1 converter was overlooked.Approach
Duration fix (both converters): allocate a 32-char stack buffer, format in-place with
TryFormat, write directly viaWriteString(string, ReadOnlySpan<char>)/WriteStringValue(ReadOnlySpan<char>):Guid fix (
TestCaseConverter): use the existing zero-allocation overload:Energy Efficiency Evidence
Proxy metric: Memory allocation (GC pressure on the IPC write thread)
TestResultConverterV2.Write— DurationTestResultConverter.Write— DurationTestCaseConverter.Write— GuidFor a run with 10,000 tests:
Green Software Foundation context:
dotnet testinvocation, proportional to test count and run frequencyTrade-offs
None.
TimeSpan.TryFormatproduces byte-for-byte identical output toTimeSpan.ToString()(same "G" general format).WriteStringValue(Guid)produces byte-for-byte identical JSON UUID string output. All existing round-trip tests pass.The
stackalloc char[32]is safe:TimeSpan.MaxValueformats to 26 chars (-10675199.02:48:05.4775807); 32 gives ample headroom.Reproducibility
Test Status
Microsoft.TestPlatform.CommunicationUtilities.UnitTests: 632/632 passed, 0 failures