[efficiency-improver] perf: eliminate GetRawText() string allocation in STJ deserializer converters

[nohwnd]

Goal and Rationale

Eliminate redundant string allocation and re-parse on the IPC deserialization path by replacing StjSafe.Deserialize<T>(element.GetRawText(), options) with StjSafe.Deserialize<T>(element, options) across 9 STJ converter classes.

Focus area: Network & I/O Efficiency (IPC deserialization path)

Problem

JsonElement.GetRawText() allocates a new heap string containing the raw JSON fragment. Passing this string to JsonSerializer.Deserialize<T>(string, ...) then causes a second parse — the serializer must re-tokenize the string from scratch.

JsonSerializer.Deserialize<T>(JsonElement, ...) reads directly from the already-parsed JsonElement, skipping both the string allocation and the re-parse entirely. StjSafe already exposes this overload.

Approach

Single-line substitution in 9 converters (all inside #if NETCOREAPP):

Converter	Variable
AttachmentConverters.cs	attachment
TestObjectBaseConverter.cs	keyElement
ExceptionConverter.cs	innerProp
TestRunChangedEventArgsConverter.cs	prop
TestCaseConverter.cs	keyElement
TestRunCompleteEventArgsConverter.cs	prop
TestExecutionContextConverter.cs	filterOptions
AfterTestRunEndResultConverter.cs	prop
DiscoveryCriteriaConverter.cs	prop

The shared DeserializeProperty<T> helper pattern (used by 4 converters) makes each replacement a one-line change.

Energy Efficiency Evidence

Proxy metric: Memory allocation (GC pressure on the IPC deserialization path)

Each GetRawText() call allocates a heap string proportional to the JSON fragment size. Typical fragment sizes for the affected types:

• TestProperty key object: ~60–150 bytes of JSON → ~120–300 bytes UTF-16 string
• UriDataAttachment: ~80–200 bytes → ~160–400 bytes
• FilterOptions / TestExecutionContext sub-objects: ~20–100 bytes

These allocations occur on every TestCase and TestResult deserialization, and on every DiscoveryCriteria / execution context message receive. Eliminating all 9 sites reduces Gen0 heap pressure on the IPC receive thread.

Green Software Foundation context:

• Hardware Efficiency: fewer GC pauses → more CPU cycles on useful work instead of heap management
• SCI (Software Carbon Intensity): less CPU energy per dotnet test invocation, proportional to test count and run frequency

Trade-offs

None. Deserialize<T>(JsonElement, ...) produces identical output for every case — it reads the same token tree that GetRawText() serialises back to a string. Change is fully covered by existing round-trip tests.

Reproducibility

# Verify no Deserialize+GetRawText calls remain:
grep -rn "Deserialize.*GetRawText" --include="*.cs" src/Microsoft.TestPlatform.CommunicationUtilities/

# Run affected tests:
./test.sh -p CommunicationUtilities -c Release

Test Status

• ✅ Build: 0 errors (4 pre-existing IL-trimming warnings, unrelated)
• ✅ Microsoft.TestPlatform.CommunicationUtilities.UnitTests: 632/632 passed, 0 failures

Generated by Efficiency Improver · 747 AIC · ⌖ 15.9 AIC · ⊞ 45.6K · ◷

[Copilot]

Pull request overview

This PR improves IPC deserialization efficiency in Microsoft.TestPlatform.CommunicationUtilities by removing JsonElement.GetRawText() string allocations and avoiding the string-based deserialize path in multiple System.Text.Json converter implementations.

Changes:

• Replaced StjSafe.Deserialize<T>(element.GetRawText(), options) with StjSafe.Deserialize<T>(element, options) across 9 NETCOREAPP-only converters.
• Centralized the improvement via the existing DeserializeProperty<T> helper pattern where present, keeping behavior consistent while reducing allocations.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestRunCompleteEventArgsConverter.cs	Uses JsonElement overload to avoid GetRawText() allocation when deserializing optional properties.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestRunChangedEventArgsConverter.cs	Same substitution in shared property-deserialization helper to reduce allocations.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectBaseConverter.cs	Deserializes TestProperty directly from JsonElement for property bag keys.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestExecutionContextConverter.cs	Deserializes FilterOptions from JsonElement without intermediate string creation.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestCaseConverter.cs	Deserializes TestProperty keys directly from JsonElement to reduce per-test allocations.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/ExceptionConverter.cs	Deserializes nested InnerException directly from JsonElement.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/DiscoveryCriteriaConverter.cs	Updates helper to deserialize properties from JsonElement instead of raw text.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/AttachmentConverters.cs	Deserializes UriDataAttachment directly from JsonElement when enumerating attachments.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/AfterTestRunEndResultConverter.cs	Updates helper to use JsonElement deserialize overload and avoid GetRawText() allocation.

[nohwnd]

🧠 Expert Review — PR #16210

Dimensions activated: IPC Transport & Protocol Stability · Cross-TFM & Framework Resolution · Performance & Allocations · Resource & IDisposable Management · Algorithmic Correctness

---

Summary

No issues found. This is a correct and clean performance optimization.

What was verified:

• StjSafe overload exists — StjSafe.Deserialize<T>(JsonElement, JsonSerializerOptions) is already present in StjSafe.cs (lines 41–47) with the same trim/AOT suppression as the string overload.
• JsonElement lifetime is safe in all 9 cases — Every changed JsonElement (prop, attachment, keyElement, innerProp, filterOptions) is used while its parent JsonDocument is still inside its using block. No use-after-dispose risk.
• Behavioral equivalence — JsonSerializer.Deserialize<T>(JsonElement, options) and Deserialize<T>(element.GetRawText(), options) produce identical deserialized objects. The JsonElement path reads the same already-parsed token tree that GetRawText() would serialize back to UTF-16 and re-parse.
• No wire format change — This only affects the deserialization read path; serialization is untouched.
• Cross-TFM safety — All 9 changed call sites are inside #if NETCOREAPP blocks. JsonSerializer.Deserialize<T>(JsonElement, ...) has been available since .NET Core 3.0.
• Scope completeness — The one remaining GetRawText() call in TestCaseConverter.cs (line 68: token.GetRawText().Trim('"')) is intentionally untouched — it is a string-coercion for non-string JSON values, not a Deserialize+GetRawText pattern.
• PR description accuracy — Title, description, and the 9-converter table match the diff exactly.

---

🧠 Reviewed by expert-reviewing workflow · PR #16210

🧠 Reviewed by Expert Code Reviewer 🧠