Update TruncateUTF8 to never split a code point and improve perf#8836
Conversation
The old per-char loop mis-counted surrogate pairs and could Substring between a high/low surrogate, producing invalid UTF-8. This rewrites to an allocation-free (on .NET Core, .NET FX uses array pool) slow path: binary-search the byte boundary, then walk back partial surrogate pairs. Adds additional fast-path cases too.
TruncateUTF8 to never split a code point and improve perfTruncateUTF8 to never split a code point and improve perf
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8836) 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 (8836) - mean (70ms) : 69, 72
master - mean (70ms) : 69, 72
section Bailout
This PR (8836) - mean (74ms) : 73, 76
master - mean (74ms) : 72, 76
section CallTarget+Inlining+NGEN
This PR (8836) - mean (1,084ms) : 1032, 1135
master - mean (1,083ms) : 1035, 1131
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 (8836) - mean (110ms) : 107, 113
master - mean (109ms) : 106, 112
section Bailout
This PR (8836) - mean (110ms) : 108, 113
master - mean (110ms) : 108, 112
section CallTarget+Inlining+NGEN
This PR (8836) - mean (782ms) : 757, 807
master - mean (774ms) : 753, 795
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8836) - mean (97ms) : 95, 99
master - mean (96ms) : 93, 99
section Bailout
This PR (8836) - mean (98ms) : 96, 99
master - mean (101ms) : 95, 106
section CallTarget+Inlining+NGEN
This PR (8836) - mean (945ms) : 909, 981
master - mean (935ms) : 889, 980
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8836) - mean (100ms) : 94, 105
master - mean (97ms) : 91, 104
section Bailout
This PR (8836) - mean (98ms) : 92, 103
master - mean (96ms) : 94, 97
section CallTarget+Inlining+NGEN
This PR (8836) - mean (813ms) : 777, 848
master - mean (812ms) : 765, 859
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 (8836) - mean (203ms) : 199, 208
master - mean (204ms) : 197, 211
section Bailout
This PR (8836) - mean (206ms) : 204, 209
master - mean (207ms) : 202, 212
section CallTarget+Inlining+NGEN
This PR (8836) - mean (1,208ms) : 1165, 1250
master - mean (1,213ms) : 1168, 1258
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 (8836) - mean (291ms) : 285, 296
master - mean (294ms) : 288, 301
section Bailout
This PR (8836) - mean (293ms) : 286, 299
master - mean (295ms) : 289, 300
section CallTarget+Inlining+NGEN
This PR (8836) - mean (980ms) : 952, 1007
master - mean (983ms) : 962, 1003
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8836) - mean (283ms) : 278, 288
master - mean (289ms) : 283, 294
section Bailout
This PR (8836) - mean (282ms) : 277, 288
master - mean (288ms) : 283, 292
section CallTarget+Inlining+NGEN
This PR (8836) - mean (1,172ms) : 1127, 1216
master - mean (1,171ms) : 1132, 1211
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8836) - mean (279ms) : 273, 286
master - mean (282ms) : 276, 287
section Bailout
This PR (8836) - mean (282ms) : 275, 288
master - mean (282ms) : 276, 289
section CallTarget+Inlining+NGEN
This PR (8836) - mean (1,045ms) : 1000, 1090
master - mean (1,047ms) : 1001, 1094
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmark execution time: 2026-06-26 16:43:55 Comparing candidate commit e694922 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 58 known flaky benchmarks, 68 flaky benchmarks without significant changes.
|
| /// <returns>true if the value was truncated. false otherwise</returns> | ||
| // https://github.com/DataDog/datadog-agent/blob/eac2327c5574da7f225f9ef0f89eaeb05ed10382/pkg/trace/traceutil/truncate.go#L36-L51 | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static bool TruncateUTF8(ref string value, int limit) |
There was a problem hiding this comment.
naming nit-pick: TruncateUtf8
There was a problem hiding this comment.
💯, but this name predates these updates by a lot 😄 I'm happy to do the rename, but I'll do that outside of this PR if that's ok 😉
Summary of changes
Improves the
TraceUtil.TruncateUTF8()method performance and correctnessReason for change
The previous version would iterate over every character in a string to work out where to chop a UTF-8 string to keep the UTF-8 size the same. This is relatively slow, misses some fast-path cases, and also fails on a "correctness" level in that it could generate incorrect utf8 bytes if it split a surrogate pair.
Implementation details
Encoding.GetByteCount(value)on the whole string if there's a chance it might be in rangeSpan<T>overloads to remove allocationlimit) so that we can check the size of sub segmentsNote that we don't handle split combining-mark / grapheme-cluster, because the agent doesn't handle those either. It makes our life simpler, and is a rare edge case, for an already oversized string
Test coverage
Tweaked existing unit test to demonstrate the split surrogate issue, and added some extra unit tests.
Ran various benchmarks comparing the old to new - the new version is faster in all cases, and in some cases allocates much less 🎉
Benchmark Details
Other details
I explored some other approaches too, e.g. using
fixedto avoid the need for the array pool, but I preferred this approach for simplicity.