Add default value to StringBuilderCache, and use in more places #6232

vandonr · 2024-11-04T14:58:27Z

Summary of changes

add a default value to the StringBuilderCache.Acquire() method. Since we're going to reuse it if it didn't grow, it's ok to allocate it a bit larger than we need by default.
Looked for uses of new StringBuilder, and replaced with the cache where it made sense.

Reason for change

Implementation details

Test coverage

datadog-ddstaging · 2024-11-04T15:17:59Z

Datadog Report

Branch report: vandonr/clean
Commit report: 0a24d67
Test service: dd-trace-dotnet

✅ 0 Failed, 375637 Passed, 2715 Skipped, 26h 4m 47.12s Total Time

andrewlock · 2024-11-04T16:15:32Z

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

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 shown 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).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6232) - mean (70ms)  : 68, 72
     .   : milestone, 70,
    master - mean (70ms)  : 68, 71
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6232) - mean (1,115ms)  : 1093, 1136
     .   : milestone, 1115,
    master - mean (1,116ms)  : 1099, 1133
     .   : milestone, 1116,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6232) - mean (108ms)  : 105, 110
     .   : milestone, 108,
    master - mean (108ms)  : 106, 110
     .   : milestone, 108,

    section CallTarget+Inlining+NGEN
    This PR (6232) - mean (776ms)  : 762, 789
     .   : milestone, 776,
    master - mean (775ms)  : 762, 787
     .   : milestone, 775,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6232) - mean (91ms)  : 89, 94
     .   : milestone, 91,
    master - mean (91ms)  : 90, 93
     .   : milestone, 91,

    section CallTarget+Inlining+NGEN
    This PR (6232) - mean (733ms)  : 715, 751
     .   : milestone, 733,
    master - mean (731ms)  : 716, 746
     .   : milestone, 731,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6232) - mean (190ms)  : 187, 192
     .   : milestone, 190,
    master - mean (191ms)  : 185, 196
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6232) - mean (1,226ms)  : 1205, 1246
     .   : milestone, 1226,
    master - mean (1,229ms)  : 1208, 1250
     .   : milestone, 1229,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6232) - mean (276ms)  : 272, 280
     .   : milestone, 276,
    master - mean (275ms)  : 270, 280
     .   : milestone, 275,

    section CallTarget+Inlining+NGEN
    This PR (6232) - mean (954ms)  : 935, 972
     .   : milestone, 954,
    master - mean (949ms)  : 934, 964
     .   : milestone, 949,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6232) - mean (264ms)  : 260, 269
     .   : milestone, 264,
    master - mean (264ms)  : 260, 268
     .   : milestone, 264,

    section CallTarget+Inlining+NGEN
    This PR (6232) - mean (943ms)  : 915, 970
     .   : milestone, 943,
    master - mean (935ms)  : 913, 957
     .   : milestone, 935,

andrewlock · 2024-11-04T16:45:41Z

Benchmarks Report for appsec 🐌

Benchmarks for #6232 compared to master:

1 benchmarks are faster, with geometric mean 1.158
2 benchmarks have fewer allocations
2 benchmarks have more allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.Asm.AppSecBodyBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6232

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472	1.158	4,358.01	3,763.55

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	72.3μs	67.8ns	263ns	0.0716	0	6 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	62.5μs	58.7ns	220ns	0.0938	0	6.95 KB
master	`AllCycleSimpleBody`	net472	49.1μs	72ns	279ns	1.31	0	8.33 KB
master	`AllCycleMoreComplexBody`	net6.0	78.4μs	55ns	213ns	0.117	0	9.51 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	69.4μs	56.4ns	211ns	0.139	0	10.36 KB
master	`AllCycleMoreComplexBody`	net472	56.5μs	85.2ns	330ns	1.87	0.0283	11.85 KB
master	`ObjectExtractorSimpleBody`	net6.0	145ns	0.12ns	0.448ns	0.00395	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	209ns	0.137ns	0.494ns	0.00369	0	272 B
master	`ObjectExtractorSimpleBody`	net472	171ns	0.0861ns	0.334ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	3.04μs	2.64ns	9.86ns	0.0533	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.91μs	3.08ns	11.9ns	0.0507	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	4.36μs	1.92ns	7.44ns	0.601	0.00656	3.8 KB
#6232	`AllCycleSimpleBody`	net6.0	73μs	108ns	416ns	0.0739	0	6.01 KB
#6232	`AllCycleSimpleBody`	netcoreapp3.1	62.7μs	52.5ns	203ns	0.0934	0	6.95 KB
#6232	`AllCycleSimpleBody`	net472	48.3μs	38.7ns	150ns	1.31	0	8.33 KB
#6232	`AllCycleMoreComplexBody`	net6.0	79.1μs	55.9ns	216ns	0.119	0	9.51 KB
#6232	`AllCycleMoreComplexBody`	netcoreapp3.1	69.2μs	82.7ns	309ns	0.138	0	10.37 KB
#6232	`AllCycleMoreComplexBody`	net472	54.6μs	54ns	209ns	1.88	0.0273	11.85 KB
#6232	`ObjectExtractorSimpleBody`	net6.0	140ns	0.25ns	0.969ns	0.00398	0	280 B
#6232	`ObjectExtractorSimpleBody`	netcoreapp3.1	207ns	0.633ns	2.45ns	0.00362	0	272 B
#6232	`ObjectExtractorSimpleBody`	net472	169ns	0.145ns	0.56ns	0.0446	0	281 B
#6232	`ObjectExtractorMoreComplexBody`	net6.0	3.17μs	2.03ns	7.59ns	0.0527	0	3.78 KB
#6232	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.91μs	2.15ns	8.04ns	0.049	0	3.69 KB
#6232	`ObjectExtractorMoreComplexBody`	net472	3.76μs	1.97ns	7.64ns	0.602	0.00566	3.8 KB

Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	38.2μs	26.2ns	102ns	0.454	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	54.3μs	36.7ns	142ns	0.432	0	32.4 KB
master	`EncodeArgs`	net472	66.4μs	27.9ns	104ns	5.15	0.0661	32.5 KB
master	`EncodeLegacyArgs`	net6.0	76.2μs	407ns	2.11μs	0	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	109μs	254ns	983ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	154μs	97.1ns	376ns	0.31	0	2.15 KB
#6232	`EncodeArgs`	net6.0	37.1μs	23.8ns	92.1ns	0.448	0	32.4 KB
#6232	`EncodeArgs`	netcoreapp3.1	54.1μs	32.9ns	127ns	0.433	0	32.4 KB
#6232	`EncodeArgs`	net472	67.9μs	38.5ns	149ns	5.16	0.0679	32.5 KB
#6232	`EncodeLegacyArgs`	net6.0	80.4μs	23.2ns	86.8ns	0	0	2.14 KB
#6232	`EncodeLegacyArgs`	netcoreapp3.1	108μs	64.1ns	231ns	0	0	2.14 KB
#6232	`EncodeLegacyArgs`	net472	155μs	98.6ns	382ns	0.31	0	2.15 KB

Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunWafRealisticBenchmark`	net6.0	185μs	60.9ns	236ns	0	2.44 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	198μs	238ns	921ns	0	2.39 KB
master	`RunWafRealisticBenchmark`	net472	210μs	141ns	545ns	0.315	2.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	106ns	381ns	0	1.47 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	130μs	155ns	598ns	0	1.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	138μs	42.5ns	153ns	0.208	1.49 KB
#6232	`RunWafRealisticBenchmark`	net6.0	183μs	91.5ns	342ns	0	2.44 KB
#6232	`RunWafRealisticBenchmark`	netcoreapp3.1	196μs	105ns	380ns	0	2.39 KB
#6232	`RunWafRealisticBenchmark`	net472	210μs	92.5ns	346ns	0.314	2.46 KB
#6232	`RunWafRealisticBenchmarkWithAttack`	net6.0	123μs	70.8ns	274ns	0	1.47 KB
#6232	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	129μs	68.9ns	267ns	0	1.46 KB
#6232	`RunWafRealisticBenchmarkWithAttack`	net472	140μs	49.5ns	192ns	0.209	1.49 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6232

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net472	272.91 KB	286.72 KB	13.81 KB	5.06%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0	253.95 KB	264.78 KB	10.83 KB	4.27%

Fewer allocations 🎉 in #6232

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	255.27 KB	253.29 KB	-1.98 KB	-0.78%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	60.49 KB	59.85 KB	-640 B	-1.06%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	52.9μs	209ns	782ns	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	54.2μs	287ns	1.38μs	42.64 KB
master	`StringConcatBenchmark`	net472	38μs	165ns	616ns	60.49 KB
master	`StringConcatAspectBenchmark`	net6.0	295μs	4.71μs	43.7μs	253.95 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	354μs	1.97μs	12.1μs	255.27 KB
master	`StringConcatAspectBenchmark`	net472	278μs	6.14μs	58.9μs	272.91 KB
#6232	`StringConcatBenchmark`	net6.0	60.6μs	739ns	7.2μs	43.44 KB
#6232	`StringConcatBenchmark`	netcoreapp3.1	55.3μs	312ns	2.1μs	42.64 KB
#6232	`StringConcatBenchmark`	net472	37.1μs	56.8ns	205ns	59.85 KB
#6232	`StringConcatAspectBenchmark`	net6.0	302μs	6.1μs	59.5μs	264.78 KB
#6232	`StringConcatAspectBenchmark`	netcoreapp3.1	338μs	1.61μs	6.23μs	253.29 KB
#6232	`StringConcatAspectBenchmark`	net472	254μs	1.13μs	4.39μs	286.72 KB

tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/NativeLibrary.cs

lucaspimentel · 2024-11-04T19:50:06Z

tracer/src/Datadog.Trace/Util/StringBuilderCache.cs

@@ -23,7 +23,7 @@ internal static class StringBuilderCache
        [ThreadStatic]
        private static StringBuilder? _cachedInstance;

-        public static StringBuilder Acquire(int capacity)
+        public static StringBuilder Acquire(int capacity = MaxBuilderSize)


I wonder why we never had a default capacity here. Our StringBuilderCache type was ~~copied from~~ based on .NET's internal StringBuilderCache, and they had a 16 default capacity.

.NET Framework:
https://referencesource.microsoft.com/#mscorlib/system/text/stringbuildercache.cs,50

Latest:
https://github.com/dotnet/runtime/blob/1c10ceecbf5356c33c67f6325072d753707f854e/src/libraries/Common/src/System/Text/StringBuilderCache.cs#L20

yes, I wrote about this in the slack thread:

I'm not sure it makes a huge difference, since we're going to reuse that StringBuilder until it grows bigger that MaxBuilderSize, we might as well allocate it with that size from the get go

Either we needed less space and it gets reused, or we needed more, and in this case no harm in having a big one at first. It's only bad if we create stringbuilders that are too big in short lived threads where they won't get reused.

Where this creates trouble I think is when we use values like 256 that will get over the max allowed capacity when expanded (because the allocated memory doubles on expand, roughly), so if we request a StringBuilder of capacity 256 and we need to write 260 characters to it, it'll get thrown away on release, which is sad.

Which is why I think in our use case, 320 from the beginning is good.

It can be wasteful to make the StringBuilder default capacity either too small or too large. It's a trade-off between too much resizing (too small) or allocating more capacity than we need (too large). Some of that is mitigated by the fact that we reuse them to some extent, and I think we trend towards "larger is better, as long as it's not too large."

Unfortunately, it's all guesswork since we don't have the numbers to make an informed decision 😅. For example, how often are the StringBuilder instances reused? How large do they get? What's the p50, p90, etc? How often do they grow above the max size? Does any of this have an impact large enough to justify adding telemetry? Am I overthinking this? 😝

Yeah, I was thinking about this as well. We could add telemetry to it, on creation, on reuse and on release, and I think we'd have all the info we need. But I don't think it's something we do much. We do telemetry logs, but not really telemetry metrics ?
Because Microsoft chose 320 based on their experts' advice, but maybe we can choose a value more suited to our use cases. Also, as you say, it may be overthinking, and it may not matter that much in the end :p

We do telemetry logs, but not really telemetry metrics ?

We have telemetry metrics. Usage is not very high because it's a newer feature and we haven't gone back to add it everywhere in older code.

See https://github.com/DataDog/dd-trace-dotnet/tree/master/tracer/src/Datadog.Trace/Telemetry/Metrics

But I think there are issues with using "gauge" metrics (as opposed to "count"). @andrewlock is the expert there.

gauge and count metrics are fine; distributions are not

I don't see how this use case (StringBuilder size) is different from the Baggage size we were talking about earlier. I'm not a metrics expert, so I'm probably missing something.

🤷‍♂️ it's not different AFAICT 😅 That has the same issues with distributions... Most likely we want a distribution here, not a gauge, so most likely we have the same problem 😄

yes we'd probably like to have counters to evaluate the % of "cache miss", but also a distribution of the sizes. I didn't know distribution metrics were causing issues.

andrewlock · 2024-11-04T21:55:18Z

Benchmarks Report for tracer 🐌

Benchmarks for #6232 compared to master:

5 benchmarks are faster, with geometric mean 1.160
1 benchmarks are slower, with geometric mean 1.148
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartStopWithChild`	net6.0	7.99μs	44.6ns	306ns	0.0124	0.00413	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10μs	55.7ns	348ns	0.0193	0.00964	0	5.8 KB
master	`StartStopWithChild`	net472	16.3μs	51.8ns	200ns	1.04	0.303	0.0984	6.21 KB
#6232	`StartStopWithChild`	net6.0	8.19μs	46.5ns	357ns	0.0162	0.0081	0	5.61 KB
#6232	`StartStopWithChild`	netcoreapp3.1	10.1μs	55ns	316ns	0.0288	0.0144	0.00479	5.8 KB
#6232	`StartStopWithChild`	net472	16.1μs	52.6ns	204ns	1.07	0.343	0.111	6.22 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	489μs	468ns	1.75μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	639μs	255ns	986ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	839μs	500ns	1.93μs	0.419	3.3 KB
#6232	`WriteAndFlushEnrichedTraces`	net6.0	481μs	303ns	1.17μs	0	2.7 KB
#6232	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	657μs	202ns	754ns	0	2.7 KB
#6232	`WriteAndFlushEnrichedTraces`	net472	843μs	471ns	1.83μs	0.419	3.3 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	203μs	1.18μs	9.8μs	0.194	18.73 KB
master	`SendRequest`	netcoreapp3.1	226μs	1.3μs	10.3μs	0.216	20.89 KB
master	`SendRequest`	net472	0.00038ns	0.000225ns	0.000841ns	0	0 b
#6232	`SendRequest`	net6.0	203μs	1.18μs	11μs	0.204	18.73 KB
#6232	`SendRequest`	netcoreapp3.1	225μs	1.5μs	15μs	0.241	20.89 KB
#6232	`SendRequest`	net472	0.0017ns	0.000614ns	0.0023ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	561μs	1.87μs	7μs	0.546	0	0	41.56 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	693μs	3.51μs	16.1μs	0.342	0	0	41.78 KB
master	`WriteAndFlushEnrichedTraces`	net472	870μs	2.7μs	10.5μs	8.08	2.55	0.425	53.35 KB
#6232	`WriteAndFlushEnrichedTraces`	net6.0	576μs	3.03μs	14.5μs	0.543	0	0	41.66 KB
#6232	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	683μs	3.52μs	16.1μs	0.34	0	0	41.74 KB
#6232	`WriteAndFlushEnrichedTraces`	net472	862μs	4.17μs	17.7μs	8.3	2.62	0.437	53.32 KB

Benchmarks.Trace.DbCommandBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6232

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0	1.118	1,378.17	1,233.03

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.38μs	1.33ns	4.78ns	0.0144	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.72μs	1.45ns	5.44ns	0.0134	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.11μs	1.52ns	5.68ns	0.156	0.00105	987 B
#6232	`ExecuteNonQuery`	net6.0	1.23μs	0.805ns	2.9ns	0.0142	0	1.02 KB
#6232	`ExecuteNonQuery`	netcoreapp3.1	1.72μs	1.07ns	3.87ns	0.0139	0	1.02 KB
#6232	`ExecuteNonQuery`	net472	2.12μs	3.25ns	12.6ns	0.156	0.00106	987 B

Benchmarks.Trace.ElasticsearchBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6232

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net6.0	1.164	1,374.66	1,180.98

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.37μs	1.93ns	7.48ns	0.0136	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.51μs	0.541ns	1.95ns	0.0129	976 B
master	`CallElasticsearch`	net472	2.51μs	1.83ns	7.11ns	0.157	995 B
master	`CallElasticsearchAsync`	net6.0	1.44μs	0.643ns	2.49ns	0.0131	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	0.695ns	2.6ns	0.0137	1.02 KB
master	`CallElasticsearchAsync`	net472	2.54μs	0.708ns	2.74ns	0.167	1.05 KB
#6232	`CallElasticsearch`	net6.0	1.18μs	3.4ns	13.2ns	0.0135	976 B
#6232	`CallElasticsearch`	netcoreapp3.1	1.53μs	0.43ns	1.55ns	0.0132	976 B
#6232	`CallElasticsearch`	net472	2.71μs	9.93ns	38.5ns	0.157	995 B
#6232	`CallElasticsearchAsync`	net6.0	1.33μs	0.839ns	3.25ns	0.0133	952 B
#6232	`CallElasticsearchAsync`	netcoreapp3.1	1.71μs	1.05ns	4.06ns	0.0137	1.02 KB
#6232	`CallElasticsearchAsync`	net472	2.68μs	6.2ns	22.3ns	0.166	1.05 KB

Benchmarks.Trace.GraphQLBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6232

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0	1.148	1,132.20	1,300.01

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.13μs	0.481ns	1.86ns	0.0131	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.67μs	0.376ns	1.3ns	0.0124	952 B
master	`ExecuteAsync`	net472	1.79μs	1.6ns	5.54ns	0.145	915 B
#6232	`ExecuteAsync`	net6.0	1.3μs	0.615ns	2.3ns	0.0137	952 B
#6232	`ExecuteAsync`	netcoreapp3.1	1.6μs	1.62ns	5.85ns	0.0128	952 B
#6232	`ExecuteAsync`	net472	1.81μs	0.743ns	2.78ns	0.145	915 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.41μs	2.68ns	10ns	0.0329	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.27μs	1.8ns	6.74ns	0.037	2.85 KB
master	`SendAsync`	net472	7.42μs	3.7ns	13.8ns	0.493	3.12 KB
#6232	`SendAsync`	net6.0	4.38μs	2.58ns	9.64ns	0.0308	2.31 KB
#6232	`SendAsync`	netcoreapp3.1	5.27μs	1.7ns	6.59ns	0.0368	2.85 KB
#6232	`SendAsync`	net472	7.59μs	2.56ns	9.56ns	0.492	3.12 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.59μs	5.07ns	19.6ns	0.0233	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.14μs	1.24ns	4.81ns	0.0224	1.64 KB
master	`EnrichedLog`	net472	2.48μs	1.68ns	6.51ns	0.25	1.57 KB
#6232	`EnrichedLog`	net6.0	1.53μs	1.27ns	4.93ns	0.0229	1.64 KB
#6232	`EnrichedLog`	netcoreapp3.1	2.23μs	0.678ns	2.54ns	0.0223	1.64 KB
#6232	`EnrichedLog`	net472	2.64μs	0.972ns	3.77ns	0.249	1.57 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	120μs	174ns	675ns	0.0601	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	120μs	191ns	741ns	0	0	4.28 KB
master	`EnrichedLog`	net472	151μs	210ns	787ns	0.678	0.226	4.46 KB
#6232	`EnrichedLog`	net6.0	118μs	138ns	516ns	0.0581	0	4.28 KB
#6232	`EnrichedLog`	netcoreapp3.1	123μs	242ns	937ns	0.0616	0	4.28 KB
#6232	`EnrichedLog`	net472	153μs	106ns	397ns	0.686	0.229	4.46 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3.16μs	0.709ns	2.75ns	0.0316	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.13μs	0.543ns	1.96ns	0.0308	2.2 KB
master	`EnrichedLog`	net472	4.94μs	1.55ns	5.99ns	0.319	2.02 KB
#6232	`EnrichedLog`	net6.0	2.92μs	0.862ns	3.23ns	0.0307	2.2 KB
#6232	`EnrichedLog`	netcoreapp3.1	4.07μs	1.36ns	5.27ns	0.0284	2.2 KB
#6232	`EnrichedLog`	net472	4.94μs	1.53ns	5.72ns	0.319	2.02 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.33μs	0.356ns	1.33ns	0.0159	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.72μs	0.584ns	2.11ns	0.0146	1.14 KB
master	`SendReceive`	net472	2.05μs	0.847ns	3.06ns	0.184	1.16 KB
#6232	`SendReceive`	net6.0	1.39μs	1.14ns	4.12ns	0.0159	1.14 KB
#6232	`SendReceive`	netcoreapp3.1	1.74μs	0.935ns	3.5ns	0.0158	1.14 KB
#6232	`SendReceive`	net472	2.14μs	1.09ns	4.23ns	0.183	1.16 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.74μs	1.15ns	4.46ns	0.0218	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.94μs	2.75ns	10.3ns	0.0217	1.65 KB
master	`EnrichedLog`	net472	4.46μs	2.48ns	9.27ns	0.322	2.04 KB
#6232	`EnrichedLog`	net6.0	2.85μs	6.87ns	26.6ns	0.0225	1.6 KB
#6232	`EnrichedLog`	netcoreapp3.1	3.85μs	1.9ns	7.34ns	0.0212	1.65 KB
#6232	`EnrichedLog`	net472	4.31μs	1.54ns	5.76ns	0.323	2.04 KB

Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6232

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1	1.209	667.15	551.73
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.179	473.26	401.24

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	473ns	0.421ns	1.57ns	0.0081	576 B
master	`StartFinishSpan`	netcoreapp3.1	664ns	2.41ns	9.32ns	0.0075	576 B
master	`StartFinishSpan`	net472	720ns	0.797ns	3.09ns	0.0915	578 B
master	`StartFinishScope`	net6.0	522ns	0.457ns	1.77ns	0.00978	696 B
master	`StartFinishScope`	netcoreapp3.1	765ns	0.641ns	2.48ns	0.00916	696 B
master	`StartFinishScope`	net472	906ns	0.849ns	3.29ns	0.104	658 B
#6232	`StartFinishSpan`	net6.0	401ns	0.817ns	3.16ns	0.00803	576 B
#6232	`StartFinishSpan`	netcoreapp3.1	552ns	0.497ns	1.86ns	0.00761	576 B
#6232	`StartFinishSpan`	net472	665ns	0.815ns	3.16ns	0.0916	578 B
#6232	`StartFinishScope`	net6.0	493ns	0.452ns	1.75ns	0.00983	696 B
#6232	`StartFinishScope`	netcoreapp3.1	754ns	0.574ns	2.22ns	0.00942	696 B
#6232	`StartFinishScope`	net472	863ns	1.23ns	4.78ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6232

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0	1.130	730.85	646.98

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	731ns	0.424ns	1.59ns	0.00951	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	977ns	1.19ns	4.59ns	0.00928	696 B
master	`RunOnMethodBegin`	net472	1.15μs	1.61ns	6.23ns	0.104	658 B
#6232	`RunOnMethodBegin`	net6.0	648ns	0.701ns	2.72ns	0.00974	696 B
#6232	`RunOnMethodBegin`	netcoreapp3.1	908ns	4.8ns	25ns	0.00949	696 B
#6232	`RunOnMethodBegin`	net472	1.16μs	1.49ns	5.76ns	0.104	658 B

andrewlock · 2024-11-04T22:49:49Z

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6232) (11.149M)   : 0, 11148809
    master (11.132M)   : 0, 11131684
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6232) (7.270M)   : 0, 7270328
    master (7.202M)   : 0, 7202253
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.442M)   : 0, 7442024

    section Manual
    master (10.983M)   : 0, 10982660

    section Manual + Automatic
    This PR (6232) (6.737M)   : 0, 6737264
    master (6.645M)   : 0, 6644995

    section DD_TRACE_ENABLED=0
    master (10.234M)   : 0, 10233674

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6232) (9.288M)   : 0, 9287708
    master (9.505M)   : 0, 9505311
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6232) (6.468M)   : 0, 6468079
    master (6.463M)   : 0, 6462996

    section Trace stats
    master (6.709M)   : 0, 6709463

    section Manual
    master (9.346M)   : 0, 9345687

    section Manual + Automatic
    This PR (6232) (5.796M)   : 0, 5796330
    master (5.903M)   : 0, 5902776

    section DD_TRACE_ENABLED=0
    master (8.956M)   : 0, 8956040

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6232) (9.357M)   : 0, 9356773
    master (9.708M)   : 0, 9708001
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6232) (6.146M)   : 0, 6146435
    master (6.407M)   : 0, 6407146
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (7.103M)   : 0, 7102656

    section Manual
    master (9.685M)   : 0, 9685116

    section Manual + Automatic
    This PR (6232) (5.593M)   : crit ,0, 5592622
    master (6.028M)   : 0, 6027641

    section DD_TRACE_ENABLED=0
    master (9.212M)   : 0, 9211970

lucaspimentel

Thanks!

vandonr added 2 commits November 4, 2024 10:09

add default value to StringBuilderCache

47792eb

use StringBuilderCache instead of new instance

07cecbe

vandonr requested review from a team as code owners November 4, 2024 14:58

fix import

cec2323

fix code

e51e9cc

lucaspimentel reviewed Nov 4, 2024

View reviewed changes

tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/NativeLibrary.cs Show resolved Hide resolved

lucaspimentel reviewed Nov 4, 2024

View reviewed changes

lucaspimentel mentioned this pull request Nov 6, 2024

Baggage part 2/3: propagation #6158

Merged

lucaspimentel approved these changes Nov 7, 2024

View reviewed changes

vandonr added 2 commits November 8, 2024 09:59

Merge remote-tracking branch 'origin/master' into vandonr/clean

d20076c

undo change on CI to have one less reviewing team

ca220bf

robertpi approved these changes Nov 8, 2024

View reviewed changes

tonyredondo approved these changes Nov 8, 2024

View reviewed changes

dudikeleti approved these changes Nov 8, 2024

View reviewed changes

Merge branch 'master' into vandonr/clean

0a24d67

vandonr merged commit 56397a0 into master Nov 12, 2024
81 of 83 checks passed

vandonr deleted the vandonr/clean branch November 12, 2024 14:28

github-actions bot added this to the vNext-v3 milestone Nov 12, 2024

Add default value to StringBuilderCache, and use in more places #6232

Add default value to StringBuilderCache, and use in more places #6232

Conversation

vandonr commented Nov 4, 2024 • edited by lucaspimentel Loading

Summary of changes

Reason for change

Implementation details

Test coverage

datadog-ddstaging bot commented Nov 4, 2024 • edited Loading

Datadog Report

andrewlock commented Nov 4, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Nov 4, 2024 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Faster 🎉 in #6232

Raw results

Raw results

Raw results

More allocations ⚠️ in #6232

Fewer allocations 🎉 in #6232

Raw results

lucaspimentel Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucaspimentel Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewlock commented Nov 4, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6232

Raw results

Faster 🎉 in #6232

Raw results

Slower ⚠️ in #6232

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6232

Raw results

Faster 🎉 in #6232

Raw results

andrewlock commented Nov 4, 2024 • edited Loading

Throughput/Crank Report ⚡

lucaspimentel left a comment

Choose a reason for hiding this comment

vandonr commented Nov 4, 2024 •

edited by lucaspimentel

Loading

datadog-ddstaging bot commented Nov 4, 2024 •

edited

Loading

andrewlock commented Nov 4, 2024 •

edited

Loading

andrewlock commented Nov 4, 2024 •

edited

Loading

lucaspimentel Nov 4, 2024 •

edited

Loading

lucaspimentel Nov 4, 2024 •

edited

Loading

andrewlock commented Nov 4, 2024 •

edited

Loading

andrewlock commented Nov 4, 2024 •

edited

Loading