Add helper methods to IApiWebRequest for serializing JSON directly to a Stream#8019
Add helper methods to IApiWebRequest for serializing JSON directly to a Stream#8019andrewlock merged 1 commit intomasterfrom
IApiWebRequest for serializing JSON directly to a Stream#8019Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8019) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (69ms) : 67, 70
master - mean (68ms) : 67, 70
section Bailout
This PR (8019) - mean (72ms) : 71, 73
master - mean (72ms) : 71, 74
section CallTarget+Inlining+NGEN
This PR (8019) - mean (1,016ms) : 945, 1087
master - mean (1,013ms) : 948, 1078
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (106ms) : 104, 108
master - mean (105ms) : 103, 108
section Bailout
This PR (8019) - mean (107ms) : 106, 108
master - mean (107ms) : 105, 108
section CallTarget+Inlining+NGEN
This PR (8019) - mean (749ms) : 690, 809
master - mean (735ms) : 667, 804
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (94ms) : 92, 96
master - mean (93ms) : 91, 95
section Bailout
This PR (8019) - mean (94ms) : 93, 96
master - mean (94ms) : 93, 95
section CallTarget+Inlining+NGEN
This PR (8019) - mean (714ms) : 691, 738
master - mean (708ms) : 671, 745
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (92ms) : 90, 94
master - mean (92ms) : 90, 94
section Bailout
This PR (8019) - mean (93ms) : 92, 94
master - mean (93ms) : 92, 94
section CallTarget+Inlining+NGEN
This PR (8019) - mean (634ms) : 621, 648
master - mean (634ms) : 617, 650
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (193ms) : 190, 196
master - mean (194ms) : 190, 198
section Bailout
This PR (8019) - mean (197ms) : 195, 200
master - mean (197ms) : 193, 201
section CallTarget+Inlining+NGEN
This PR (8019) - mean (1,116ms) : 1061, 1171
master - mean (1,112ms) : 1054, 1170
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (279ms) : 273, 286
master - mean (276ms) : 272, 281
section Bailout
This PR (8019) - mean (278ms) : 273, 283
master - mean (277ms) : 274, 280
section CallTarget+Inlining+NGEN
This PR (8019) - mean (935ms) : 885, 984
master - mean (925ms) : 883, 967
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (271ms) : 263, 278
master - mean (270ms) : 266, 274
section Bailout
This PR (8019) - mean (270ms) : 267, 274
master - mean (270ms) : 267, 273
section CallTarget+Inlining+NGEN
This PR (8019) - mean (930ms) : 896, 965
master - mean (931ms) : 898, 964
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8019) - mean (269ms) : 265, 274
master - mean (269ms) : 264, 275
section Bailout
This PR (8019) - mean (269ms) : 266, 272
master - mean (270ms) : 266, 274
section CallTarget+Inlining+NGEN
This PR (8019) - mean (824ms) : 803, 845
master - mean (822ms) : 807, 837
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
d45ddc7 to
cdfa351
Compare
|
BenchmarksBenchmark execution time: 2025-12-30 12:08:12 Comparing candidate commit cdfa351 in PR branch Found 9 performance improvements and 6 performance regressions! Performance is the same for 161 metrics, 10 unstable metrics. scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild net6.0
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody net6.0
scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest net6.0
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync netcoreapp3.1
scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark netcoreapp3.1
scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope net6.0
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes netcoreapp3.1
|
| } | ||
|
|
||
| private TelemetryData GetData() => | ||
| new TelemetryData( |
There was a problem hiding this comment.
Where did this telemetry data come from? Just wondering
There was a problem hiding this comment.
I just copy-pasted it from some other tests that use the same thing as a "somewhat representative example"
## Summary of changes Update telemetry and remote config to use the `PostAsJson<T>` method introduced in #8019 ## Reason for change As shown in #8019, using the `PostAsJson<T>` has some performance benefits (for telemetry, when using gzip, _significant_ benefits). It may also make refactoring to other JSON libraries later somewhat easier. ## Implementation details Update Telemetry and Remote Config to use the new streaming `PostAsJson<T>` model, and remove the old implementation ## Test coverage - Update `MockTracerAgent` to handle chunked encoding in the telemetry and remote config responses (previously these were requiring a `Content-Length`, but chunked responses don't have one) - Add additional unit test to verify above testing fix - Update other tests that require Content-Length to also allow chunked encoding ## Other details Part of a small stack - #8019 - #8017 👈
Summary of changes
Adds
PostAsJson<T>method toIApiWebRequestReason for change
Currently, if we want to send JSON, we serialize it to a string locally, convert the string to utf-8 bytes, potentially compress those bytes, and then copy that to the stream. Doing that as efficiently as we can is somewhat tricky, and we haven't always got it right. By creating a central method, and writing directly to the underlying stream where we can, we can potentially see efficiency gains, and can also potentially make it easier to move to a more modern serializer later.
Implementation details
IApiRequest.PostAsJsonAsync<T>(T payload, MultipartCompression compression)(and an overload that accepts json settings)ApiRequest,HttpClient,HttpStream)Test coverage
Added unit tests by specifically serializing telemetry data, and confirming we get the correct results when we deserialize the other end (telemetry is one of the candidates for using this approach).
When running benchmarks, it became apparent that we had a serious regression in our allocations when we added GZIP-ing of our telemetry 😅 I didn't investigate the root cause, because switching to the new approach (in #8017) will resolve the issue anyway.
Overall conclusion:
ApiWebRequest(.NET FX)HttpClientRequest(.NET Core 3.1, .NET 6) - had to re-enable keep-alive to avoid connection exhaustion!HttpStreamRequestover UDS (.NET Core 3.1, .NET 6)SocketHandlerRequestover UDS (..NET 6)Benchmark used (approx)
Other details
Part of a small stack
IApiWebRequestfor serializing JSON directly to aStream#8019 👈IApiRequest.PostAsJson<T>where possible #8017