Skip to content

Add an IArrayPool<char> implementation for vendored Newtonsoft.JSON#8228

Merged
andrewlock merged 3 commits into
masterfrom
andrew/use-json-array-pool
Mar 9, 2026
Merged

Add an IArrayPool<char> implementation for vendored Newtonsoft.JSON#8228
andrewlock merged 3 commits into
masterfrom
andrew/use-json-array-pool

Conversation

@andrewlock

Copy link
Copy Markdown
Member

Summary of changes

Adds a simple IArrayPool<char> for use by Newtonsoft.JSON, and uses it everywhere we can

Reason for change

Newtonsoft.JSON fundamentally works with .NET's char type (UTF-16), (as opposed to System.Text.Json which works with UTF-8 where it can). To do so, it needs to create a bunch of char[] instances to use as buffers.

The JsonTextReader and JsonTextWriter abstractions allow plugging in an IArrayPool<char> implementation, and luckily this matches (pretty much exactly) the API exposed by the ArrayPool in corelib (+ vendored), so it's easy to implement.

This should help alleviate some GC pressure, as we currently do a fair amount of serializing and deserializing.

Implementation details

Pretty simple:

  • Implement IArrayPool<char> using ArrayPool<char>.Shared
  • Get 🤖 to find everywhere that we could use it (JsonTextReader and JsonTextWriter) and initialize
  • Fix some cases where these weren't being disposed.

Warning

It's important that we do dispose these, so that the arrays are correctly returned to the pool, so that we don't leak memory

There are actually other places we can update too, as this PR doesn't cover the common JsonConvert.Serialize() etc, but I'll follow up with those in a separate PR.

Test coverage

All the existing tests should pass. I worked on this as part of general perf work on remote config, so the results are a bit fuzzy (as I can't remember if it includes the savings from #8226 as well), but the results are pretty conclusive, especially for big payloads 😅

Method size Mean Error Allocated
DeserializeResponse_Original Small 22.71 ns 2,076.771 ns 23.83 KB
DeserializeResponse_Updated Small 13.70 us 0.186 us 17.23 KB
DeserializeResponse_Original Big 1,953.04 us 58,665.219 ns 2,343.26 KB
DeserializeResponse_Updated Big 614.46 us 11.988 us 252.37 KB

Other details

https://datadoghq.atlassian.net/browse/LANGPLAT-940

Discovered this while exploring remote config optimizations

@andrewlock
andrewlock requested a review from a team as a code owner February 20, 2026 17:52
@andrewlock andrewlock added the type:performance Performance, speed, latency, resource usage (CPU, memory) label Feb 20, 2026
@andrewlock
andrewlock requested review from a team as code owners February 20, 2026 17:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad20bb98cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@andrewlock
andrewlock force-pushed the andrew/use-json-array-pool branch from ad20bb9 to 7c8bf45 Compare February 20, 2026 18:00
Comment on lines +135 to +136
// Default values for StreamReader, but with leaveOpen:true
using var streamReader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, bufferSize: 1024, leaveOpen: true);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously this wasn't disposing the StreamReader, presumably because that closes the MemoryStream by default. I think technically that was ok, but seemed better to be explicit here, especially as we now have to dispose the JsonTextReader, and don't want there to be any ambiguity around it

scopeMembers.Add(result);

var reader = new JsonTextReader(new StringReader(expressionJson));
using var reader = new JsonTextReader(new StringReader(expressionJson)) { ArrayPool = JsonArrayPool.Shared };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this was just an accidental miss of the using

@@ -276,11 +277,11 @@ internal static void WriteValue(JsonWriter writer, object? value)
}

internal static JsonTextWriter GetJsonWriter(StringBuilder builder)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callers "own" and handle disposing the returned object

@andrewlock
andrewlock force-pushed the andrew/use-json-array-pool branch from 7c8bf45 to 947d1d6 Compare February 20, 2026 18:04
@pr-commenter

pr-commenter Bot commented Feb 20, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-03-06 14:56:53

Comparing candidate commit a33aae2 in PR branch andrew/use-json-array-pool with baseline commit 76f3785 in branch master.

Found 7 performance improvements and 13 performance regressions! Performance is the same for 153 metrics, 19 unstable metrics.

scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces net6.0

  • 🟩 execution_time [-79.310ms; -78.941ms] or [-39.195%; -39.012%]

scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0

  • 🟩 execution_time [-22.637ms; -16.293ms] or [-10.437%; -7.512%]

scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody netcoreapp3.1

  • 🟩 execution_time [-23.460ms; -17.730ms] or [-10.770%; -8.139%]

scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net472

  • 🟥 throughput [-230.509op/s; -227.025op/s] or [-6.063%; -5.972%]

scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs netcoreapp3.1

  • 🟥 execution_time [+20.953ms; +22.059ms] or [+11.640%; +12.254%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0

  • 🟩 execution_time [-52.305ms; -47.283ms] or [-25.244%; -22.820%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1

  • 🟥 execution_time [+19.885ms; +21.320ms] or [+11.488%; +12.318%]

scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice netcoreapp3.1

  • 🟥 throughput [-192.908op/s; -143.679op/s] or [-37.300%; -27.781%]

scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool net6.0

  • 🟥 execution_time [+158.573µs; +161.814µs] or [+15.254%; +15.566%]
  • 🟥 throughput [-129.641op/s; -127.235op/s] or [-13.477%; -13.227%]

scenario:Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery net472

  • 🟥 throughput [-20082.166op/s; -18534.683op/s] or [-5.546%; -5.118%]

scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync netcoreapp3.1

  • 🟥 throughput [-38361.895op/s; -30531.317op/s] or [-8.131%; -6.471%]

scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync net472

  • 🟩 throughput [+20639.919op/s; +23547.003op/s] or [+5.460%; +6.229%]

scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync netcoreapp3.1

  • 🟥 throughput [-34905.918op/s; -24116.408op/s] or [-8.093%; -5.592%]

scenario:Benchmarks.Trace.HttpClientBenchmark.SendAsync net472

  • 🟥 throughput [-5326.592op/s; -5036.246op/s] or [-6.053%; -5.723%]

scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark netcoreapp3.1

  • 🟥 throughput [-431.355op/s; -255.872op/s] or [-20.503%; -12.162%]

scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1

  • 🟥 execution_time [+41.168ms; +42.352ms] or [+25.822%; +26.564%]

scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan net6.0

  • 🟩 execution_time [-14.578ms; -10.667ms] or [-6.879%; -5.034%]

scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes net6.0

  • 🟥 execution_time [+12.436ms; +18.268ms] or [+6.292%; +9.243%]

scenario:Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin net6.0

  • 🟩 execution_time [-27.795ms; -23.899ms] or [-12.918%; -11.107%]

@dd-trace-dotnet-ci-bot

dd-trace-dotnet-ci-bot Bot commented Feb 20, 2026

Copy link
Copy Markdown

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (8228) and master.

✅ No regressions detected - check the details below

Full Metrics Comparison

FakeDbCommand

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration75.09 ± (75.07 - 75.36) ms77.00 ± (76.95 - 77.24) ms+2.5%✅⬆️
.NET Framework 4.8 - Bailout
duration79.17 ± (79.06 - 79.39) ms81.39 ± (81.21 - 81.62) ms+2.8%✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1084.74 ± (1085.98 - 1091.80) ms1100.52 ± (1102.30 - 1108.95) ms+1.5%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms22.84 ± (22.80 - 22.88) ms23.14 ± (23.09 - 23.18) ms+1.3%✅⬆️
process.time_to_main_ms86.51 ± (86.33 - 86.69) ms88.60 ± (88.40 - 88.81) ms+2.4%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.92 ± (10.92 - 10.92) MB10.93 ± (10.93 - 10.94) MB+0.1%✅⬆️
runtime.dotnet.threads.count12 ± (12 - 12)12 ± (12 - 12)+0.0%
.NET Core 3.1 - Bailout
process.internal_duration_ms22.80 ± (22.75 - 22.84) ms23.32 ± (23.25 - 23.39) ms+2.3%✅⬆️
process.time_to_main_ms88.18 ± (87.97 - 88.40) ms91.00 ± (90.80 - 91.21) ms+3.2%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.95 ± (10.95 - 10.96) MB10.96 ± (10.95 - 10.96) MB+0.0%✅⬆️
runtime.dotnet.threads.count13 ± (13 - 13)13 ± (13 - 13)+0.0%
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms239.09 ± (235.14 - 243.05) ms238.94 ± (234.86 - 243.03) ms-0.1%
process.time_to_main_ms492.10 ± (491.42 - 492.79) ms503.83 ± (503.12 - 504.54) ms+2.4%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed47.59 ± (47.56 - 47.61) MB47.65 ± (47.63 - 47.68) MB+0.1%✅⬆️
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.0%✅⬆️
.NET 6 - Baseline
process.internal_duration_ms21.73 ± (21.69 - 21.77) ms22.24 ± (22.20 - 22.28) ms+2.3%✅⬆️
process.time_to_main_ms75.69 ± (75.51 - 75.87) ms78.70 ± (78.52 - 78.88) ms+4.0%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.64 ± (10.64 - 10.64) MB10.64 ± (10.64 - 10.64) MB+0.0%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 6 - Bailout
process.internal_duration_ms21.60 ± (21.55 - 21.65) ms22.08 ± (22.03 - 22.14) ms+2.3%✅⬆️
process.time_to_main_ms76.71 ± (76.52 - 76.90) ms79.70 ± (79.51 - 79.89) ms+3.9%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.67 ± (10.67 - 10.68) MB10.75 ± (10.75 - 10.76) MB+0.7%✅⬆️
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms257.69 ± (254.48 - 260.90) ms249.21 ± (244.90 - 253.53) ms-3.3%
process.time_to_main_ms476.91 ± (476.21 - 477.61) ms488.70 ± (487.86 - 489.55) ms+2.5%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed48.31 ± (48.29 - 48.33) MB48.33 ± (48.31 - 48.35) MB+0.0%✅⬆️
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.0%
.NET 8 - Baseline
process.internal_duration_ms19.74 ± (19.71 - 19.78) ms20.33 ± (20.28 - 20.37) ms+3.0%✅⬆️
process.time_to_main_ms74.80 ± (74.64 - 74.95) ms77.14 ± (76.96 - 77.32) ms+3.1%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.68 ± (7.67 - 7.68) MB7.69 ± (7.68 - 7.70) MB+0.2%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 8 - Bailout
process.internal_duration_ms19.67 ± (19.62 - 19.71) ms20.40 ± (20.36 - 20.45) ms+3.7%✅⬆️
process.time_to_main_ms75.29 ± (75.15 - 75.43) ms78.91 ± (78.71 - 79.11) ms+4.8%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.76 ± (7.75 - 7.77) MB7.73 ± (7.73 - 7.74) MB-0.3%
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms190.96 ± (190.26 - 191.66) ms197.18 ± (196.49 - 197.86) ms+3.3%✅⬆️
process.time_to_main_ms456.81 ± (456.13 - 457.48) ms470.24 ± (469.61 - 470.87) ms+2.9%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed36.12 ± (36.09 - 36.16) MB36.22 ± (36.18 - 36.26) MB+0.3%✅⬆️
runtime.dotnet.threads.count27 ± (27 - 27)27 ± (27 - 27)-0.1%

HttpMessageHandler

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration194.26 ± (194.27 - 195.06) ms195.87 ± (196.03 - 197.06) ms+0.8%✅⬆️
.NET Framework 4.8 - Bailout
duration198.90 ± (198.73 - 199.42) ms198.96 ± (198.78 - 199.57) ms+0.0%✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1148.23 ± (1150.99 - 1159.20) ms1157.36 ± (1161.11 - 1170.41) ms+0.8%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms188.69 ± (188.26 - 189.13) ms189.39 ± (188.98 - 189.79) ms+0.4%✅⬆️
process.time_to_main_ms81.92 ± (81.67 - 82.17) ms82.60 ± (82.37 - 82.83) ms+0.8%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.12 ± (16.10 - 16.15) MB16.06 ± (16.03 - 16.09) MB-0.4%
runtime.dotnet.threads.count20 ± (20 - 20)20 ± (19 - 20)-0.2%
.NET Core 3.1 - Bailout
process.internal_duration_ms188.22 ± (187.89 - 188.55) ms189.11 ± (188.77 - 189.45) ms+0.5%✅⬆️
process.time_to_main_ms82.78 ± (82.63 - 82.93) ms83.88 ± (83.70 - 84.06) ms+1.3%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.19 ± (16.16 - 16.22) MB16.16 ± (16.13 - 16.19) MB-0.1%
runtime.dotnet.threads.count21 ± (21 - 21)21 ± (21 - 21)+0.1%✅⬆️
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms442.28 ± (439.99 - 444.56) ms437.34 ± (434.70 - 439.99) ms-1.1%
process.time_to_main_ms477.87 ± (477.18 - 478.57) ms479.98 ± (479.38 - 480.57) ms+0.4%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed57.99 ± (57.87 - 58.12) MB57.98 ± (57.85 - 58.10) MB-0.0%
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)+0.0%✅⬆️
.NET 6 - Baseline
process.internal_duration_ms193.51 ± (193.06 - 193.96) ms193.09 ± (192.76 - 193.42) ms-0.2%
process.time_to_main_ms71.37 ± (71.13 - 71.60) ms70.95 ± (70.80 - 71.10) ms-0.6%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.18 ± (16.05 - 16.30) MB16.39 ± (16.36 - 16.42) MB+1.3%✅⬆️
runtime.dotnet.threads.count19 ± (18 - 19)19 ± (19 - 19)+2.4%✅⬆️
.NET 6 - Bailout
process.internal_duration_ms192.04 ± (191.72 - 192.36) ms192.96 ± (192.61 - 193.31) ms+0.5%✅⬆️
process.time_to_main_ms72.03 ± (71.90 - 72.16) ms72.09 ± (71.98 - 72.21) ms+0.1%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.31 ± (16.22 - 16.41) MB16.19 ± (16.07 - 16.31) MB-0.7%
runtime.dotnet.threads.count20 ± (20 - 20)20 ± (20 - 20)-0.1%
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms452.30 ± (450.16 - 454.43) ms456.16 ± (454.39 - 457.92) ms+0.9%✅⬆️
process.time_to_main_ms452.33 ± (451.79 - 452.88) ms455.58 ± (454.99 - 456.18) ms+0.7%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed58.11 ± (58.01 - 58.22) MB57.79 ± (57.68 - 57.90) MB-0.6%
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)+0.1%✅⬆️
.NET 8 - Baseline
process.internal_duration_ms190.22 ± (189.91 - 190.53) ms191.57 ± (191.22 - 191.92) ms+0.7%✅⬆️
process.time_to_main_ms70.24 ± (70.04 - 70.43) ms70.56 ± (70.34 - 70.77) ms+0.5%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.70 ± (11.68 - 11.72) MB11.76 ± (11.74 - 11.78) MB+0.5%✅⬆️
runtime.dotnet.threads.count18 ± (18 - 18)18 ± (18 - 18)-0.1%
.NET 8 - Bailout
process.internal_duration_ms189.65 ± (189.35 - 189.95) ms190.30 ± (189.93 - 190.68) ms+0.3%✅⬆️
process.time_to_main_ms71.23 ± (71.09 - 71.37) ms71.49 ± (71.32 - 71.65) ms+0.4%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.78 ± (11.75 - 11.81) MB11.77 ± (11.73 - 11.80) MB-0.1%
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 19)+0.4%✅⬆️
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms370.27 ± (368.82 - 371.71) ms371.87 ± (370.40 - 373.35) ms+0.4%✅⬆️
process.time_to_main_ms435.29 ± (434.68 - 435.89) ms444.76 ± (443.76 - 445.76) ms+2.2%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed47.78 ± (47.74 - 47.81) MB47.80 ± (47.77 - 47.84) MB+0.1%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)+0.3%✅⬆️
Comparison explanation

Execution-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:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

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 charts
FakeDbCommand (.NET Framework 4.8)
gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8228) - mean (77ms)  : 75, 79
    master - mean (75ms)  : 73, 77

    section Bailout
    This PR (8228) - mean (81ms)  : 79, 83
    master - mean (79ms)  : 78, 81

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (1,106ms)  : 1058, 1154
    master - mean (1,089ms)  : 1047, 1130

Loading
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 (8228) - mean (119ms)  : 115, 123
    master - mean (117ms)  : 113, 120

    section Bailout
    This PR (8228) - mean (122ms)  : 119, 124
    master - mean (118ms)  : 115, 121

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (780ms)  : 715, 846
    master - mean (766ms)  : 708, 823

Loading
FakeDbCommand (.NET 6)
gantt
    title Execution time (ms) FakeDbCommand (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8228) - mean (108ms)  : 104, 112
    master - mean (104ms)  : 101, 107

    section Bailout
    This PR (8228) - mean (108ms)  : 106, 111
    master - mean (105ms)  : 103, 107

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (770ms)  : 691, 848
    master - mean (768ms)  : 707, 828

Loading
FakeDbCommand (.NET 8)
gantt
    title Execution time (ms) FakeDbCommand (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8228) - mean (106ms)  : 103, 108
    master - mean (103ms)  : 100, 105

    section Bailout
    This PR (8228) - mean (107ms)  : 105, 110
    master - mean (103ms)  : 101, 105

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (703ms)  : 672, 733
    master - mean (681ms)  : 651, 712

Loading
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 (8228) - mean (197ms)  : 190, 203
    master - mean (195ms)  : 191, 199

    section Bailout
    This PR (8228) - mean (199ms)  : 195, 203
    master - mean (199ms)  : 196, 202

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (1,166ms)  : 1097, 1235
    master - mean (1,155ms)  : 1095, 1215

Loading
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 (8228) - mean (281ms)  : 274, 287
    master - mean (279ms)  : 272, 286

    section Bailout
    This PR (8228) - mean (282ms)  : 277, 286
    master - mean (279ms)  : 275, 284

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (949ms)  : 907, 992
    master - mean (950ms)  : 913, 986

Loading
HttpMessageHandler (.NET 6)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8228) - mean (273ms)  : 266, 279
    master - mean (274ms)  : 268, 279

    section Bailout
    This PR (8228) - mean (274ms)  : 270, 278
    master - mean (272ms)  : 269, 276

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (942ms)  : 915, 969
    master - mean (934ms)  : 903, 965

Loading
HttpMessageHandler (.NET 8)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8228) - mean (272ms)  : 265, 279
    master - mean (270ms)  : 265, 275

    section Bailout
    This PR (8228) - mean (272ms)  : 266, 277
    master - mean (270ms)  : 266, 275

    section CallTarget+Inlining+NGEN
    This PR (8228) - mean (848ms)  : 810, 885
    master - mean (836ms)  : 813, 859

Loading

@andrewlock
andrewlock force-pushed the andrew/use-json-array-pool branch from 947d1d6 to dd07383 Compare February 23, 2026 12:39
@andrewlock
andrewlock requested a review from a team as a code owner February 23, 2026 12:39
@andrewlock
andrewlock force-pushed the andrew/add-streaming-base64-decoder branch from 33c73ce to 3d24011 Compare February 23, 2026 12:39

@link04 link04 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lucaspimentel lucaspimentel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice

@dromanol dromanol left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot

Base automatically changed from andrew/add-streaming-base64-decoder to master March 6, 2026 14:10
@andrewlock
andrewlock force-pushed the andrew/use-json-array-pool branch from dd07383 to a33aae2 Compare March 6, 2026 14:13
@andrewlock
andrewlock enabled auto-merge (squash) March 6, 2026 14:28
@andrewlock
andrewlock disabled auto-merge March 6, 2026 14:28
@andrewlock
andrewlock enabled auto-merge (squash) March 9, 2026 16:55
@andrewlock
andrewlock merged commit d17ace6 into master Mar 9, 2026
142 checks passed
@andrewlock
andrewlock deleted the andrew/use-json-array-pool branch March 9, 2026 17:06
@github-actions github-actions Bot added this to the vNext-v3 milestone Mar 9, 2026
andrewlock added a commit that referenced this pull request Mar 9, 2026
## Summary of changes

Adds JSON helper APIs to ensure we use the array pool where possible

## Reason for change

There are various "helper" APIs, which are wrappers around
Newtonsoft.JSON's `JsonSerializer` and `JsonReader`/`JsonWriter` APIs.
In #8228 we updated the explicit usages to use an array pool
implementation for `JsonReader`/`JsonWriter` calls, but they're used
internally without a pool in the helper cases.

This PR creates alternative implementations which _do_ use the pool,
updates existing code to use them, and adds the existing APIs to the
"banned API" list.

> There's another possible approach, in which we update the vendored
code to _always_ use the array pool. I was torn as to which is the
better option, and went for this approach in the end, but I'm not wedded
to it, so happy to take the alternative approach if people think it's
preferable?

## Implementation details

- Use 🤖 to find all the potential places that we need to convert.
- Create "array pool" versions of the APIs
- Update the call sites to use the new APIs
- Add the original APIs to the list of "banned" APIs to avoid using them
accidentally in the future

## Test coverage

Covered by all our existing tests, added some unit tests to confirm the
new tests behave as expected

## Other details

https://datadoghq.atlassian.net/browse/LANGPLAT-940

Discovered this while exploring remote config optimizations, but should
help lots of areas.
andrewlock added a commit that referenced this pull request Mar 10, 2026
## Summary of changes

Stop sending the fixed `RootVersion = 1` with every remote config
request

## Reason for change

Currently we're sending a fixed value of `RootVersion = 1` for all our
remote config requests, but doing so causes the agent to repeatedly send
us all the root certificates, significantly increasing the payload size,
because it thinks we haven't seen them. Sending the "final" version,
acknowledges it, and stops all the extra data, saving ~35kB per call.

## Implementation details

- Deserialize the roots in `GetRcmResponse`, leaving them as base64
encoded `string`s (which is how they are sent in the payload)
- When processing the response, de-encode the _last_ root object, and
deserialize it
- Using the `Base64DecodingStream` introduced in #8226 to avoid extra
allocations
  - Using the `IArrayPool` from #8228
- Introduces a `MinimalTufRoot` (in contrast to `TufRoot`) so that we
only materialize what we need (the `roots.signed.version` key)

This implementation avoids ~35kB per call for subsequent remote config
requests.

## Test coverage

Added unit test, and did manual test with the real agent, to confirm the
expected behaviour (reduction in data sent)

## Other details

https://datadoghq.atlassian.net/browse/LANGPLAT-940

All part of the Remote Config perf stack
andrewlock added a commit that referenced this pull request Mar 11, 2026
…#8228)

## Summary of changes

Adds a simple `IArrayPool<char>` for use by Newtonsoft.JSON, and uses it
everywhere we can

## Reason for change

Newtonsoft.JSON fundamentally works with .NET's `char` type (UTF-16),
(as opposed to System.Text.Json which works with UTF-8 where it can). To
do so, it needs to create a bunch of `char[]` instances to use as
buffers.

The `JsonTextReader` and `JsonTextWriter` abstractions allow plugging in
an `IArrayPool<char>` implementation, and luckily this matches (pretty
much exactly) the API exposed by the `ArrayPool` in corelib (+
vendored), so it's easy to implement.

This should help alleviate some GC pressure, as we currently do a fair
amount of serializing and deserializing.

## Implementation details

Pretty simple:
- Implement `IArrayPool<char>` using `ArrayPool<char>.Shared`
- Get 🤖 to find everywhere that we could use it (`JsonTextReader` and
`JsonTextWriter`) and initialize
- Fix some cases where these weren't being disposed.

> [!WARNING]
> It's important that we _do_ dispose these, so that the arrays are
correctly returned to the pool, so that we don't leak memory

There are actually other places we can update too, as this PR doesn't
cover the common `JsonConvert.Serialize()` etc, but I'll follow up with
those in a separate PR.

## Test coverage

All the existing tests should pass. I worked on this as part of general
perf work on remote config, so the results are a bit fuzzy (as I can't
remember if it includes the savings from #8226 as well), but the results
are pretty conclusive, especially for big payloads 😅


| Method | size | Mean | Error | Allocated |
| ---------------------------- | ----- | ----------: | ------------: |
---------: |
| DeserializeResponse_Original | Small | 22.71 ns | 2,076.771 ns | 23.83
KB |
| DeserializeResponse_Updated | Small | 13.70 us | 0.186 us | 17.23 KB |
| | | | | |
| DeserializeResponse_Original | Big | 1,953.04 us | 58,665.219 ns
|2,343.26 KB |
| DeserializeResponse_Updated | Big | 614.46 us | 11.988 us | 252.37 KB
|


## Other details


https://datadoghq.atlassian.net/browse/LANGPLAT-940

Discovered this while exploring remote config optimizations
andrewlock added a commit that referenced this pull request Mar 11, 2026
## Summary of changes

Adds JSON helper APIs to ensure we use the array pool where possible

## Reason for change

There are various "helper" APIs, which are wrappers around
Newtonsoft.JSON's `JsonSerializer` and `JsonReader`/`JsonWriter` APIs.
In #8228 we updated the explicit usages to use an array pool
implementation for `JsonReader`/`JsonWriter` calls, but they're used
internally without a pool in the helper cases.

This PR creates alternative implementations which _do_ use the pool,
updates existing code to use them, and adds the existing APIs to the
"banned API" list.

> There's another possible approach, in which we update the vendored
code to _always_ use the array pool. I was torn as to which is the
better option, and went for this approach in the end, but I'm not wedded
to it, so happy to take the alternative approach if people think it's
preferable?

## Implementation details

- Use 🤖 to find all the potential places that we need to convert.
- Create "array pool" versions of the APIs
- Update the call sites to use the new APIs
- Add the original APIs to the list of "banned" APIs to avoid using them
accidentally in the future

## Test coverage

Covered by all our existing tests, added some unit tests to confirm the
new tests behave as expected

## Other details

https://datadoghq.atlassian.net/browse/LANGPLAT-940

Discovered this while exploring remote config optimizations, but should
help lots of areas.
andrewlock added a commit that referenced this pull request Mar 11, 2026
## Summary of changes

A variety of minor performance improvements to remote config

## Reason for change

I did some initial benchmarking of remote config, as well as running a
test app with ASM/Debugger enabled (which use RCM), and the results
weren't great. Given we make RCM requests every 5s, smallish changes
here should add up, though the real-world effect will be tricky to
gauge.

## Implementation details

Most of the individual changes are small. In summary:
- Making work lazy where possible (delaying creating collections)
  - Don't re-create dictionaries inside loops if not required
- Avoid creating empty dictionaries and collections for no-op RCM
responses
- Avoid creating empty collections in JSON objects when they're not in
the JSON
- Cache things that change rarely
- The `ExtraServicesProvider` will rarely see new service names, so
cache the array (as the collection is append-only)
- Don't create a new request each time, just update any values that may
have changed
  - Cache the capabilities array which will rarely change
- There's actually a potential threading bug around the use of
`BigInteger`. We probably could/should use `ulong` instead but that I'll
look at that in a separate PR
- Use abstractions introduced earlier in the stack (e.g. #8226, #8228)

Additionally, I did a little bit of cleanup:
- Remove unused members from the `IRcmSubscriptionManager` interface
- Add `#nullable enable` to the RCM types (and fix nullability where
required)

## Test coverage

Mostly covered by existing unit tests, also did some manual testing.
Finally, ran a few benchmarks, but it's a bit tricky to check reliably.
This benchmark is benchmarking `_manager.SendRequest(_rcmTracer, _ =>
_steadyStateResponseTask)` and passing the same response every time. In
practice, the response changes every time, so this isn't strictly
representative, but with the changes to request caching, this should
actually mean our improvements are _better_ than the original would be.
The regression in the .NET duration is curious, but I'm not massively
concerned, and the allocations are obviously down a lot at least


| Method | Runtime | Mean | Error | Allocated |
| ------------------------ | ------------------ | ----------: |
-----------: | --------: |
| PollSteadyState_Original | .NET 10.0 | 14,235.8 ns | 275.09 ns | 12.04
KB |
| PollSteadyState_Updated | .NET 10.0 | 20,472.0 ns | 1,352.567 ns |
3.90 KB |
| | | | | |
| PollSteadyState_Original | .NET 6.0 | 33,915.7 ns | 3,339.42 ns |
12.27 KB |
| PollSteadyState_Updated | .NET 6.0 | 14,979.7 ns | 447.784 ns | 3.99
KB |
| | | | | |
| PollSteadyState_Original | .NET Core 2.1 | 21,488.2 ns | 365.39 ns |
12.85 KB |
| PollSteadyState_Updated | .NET Core 2.1 | 17,847.2 ns | 693.510 ns |
4.04 KB |
| | | | | |
| PollSteadyState_Original | .NET Core 3.1 | 18,925.2 ns | 304.16 ns |
12.23 KB |
| PollSteadyState_Updated | .NET Core 3.1 | 15,407.7 ns | 313.26 ns |
4.01 KB |
| | | | | |
| PollSteadyState_Original | .NET Framework 4.8 | 20,946.8 ns | 320.06
ns | 14.21 KB |
| PollSteadyState_Updated | .NET Framework 4.8 | 16,635.2 ns | 266.526
ns | 4.40 KB |


## Other details
https://datadoghq.atlassian.net/browse/LANGPLAT-940

All part of the Remote Config perf stack. I think this is probably about
the end of it for now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:performance Performance, speed, latency, resource usage (CPU, memory)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants