Merged
Conversation
BenchmarksBenchmark execution time: 2026-02-02 15:47:43 Comparing candidate commit 4bf2896 in PR branch Found 14 performance improvements and 3 performance regressions! Performance is the same for 160 metrics, 15 unstable metrics. scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net472
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody netcoreapp3.1
scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest netcoreapp3.1
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net6.0
scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool net6.0
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch net6.0
scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark net6.0
scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark net6.0
scenario:Benchmarks.Trace.RedisBenchmark.SendReceive netcoreapp3.1
scenario:Benchmarks.Trace.SerilogBenchmark.EnrichedLog net472
scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore net6.0
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8120) 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 (8120) - mean (80ms) : 77, 83
master - mean (80ms) : 77, 82
section Bailout
This PR (8120) - mean (84ms) : 81, 88
master - mean (84ms) : 82, 86
section CallTarget+Inlining+NGEN
This PR (8120) - mean (1,123ms) : 1067, 1179
master - mean (1,116ms) : 1068, 1163
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 (8120) - mean (127ms) : 123, 130
master - mean (127ms) : 124, 130
section Bailout
This PR (8120) - mean (127ms) : 123, 132
master - mean (128ms) : 125, 131
section CallTarget+Inlining+NGEN
This PR (8120) - mean (813ms) : 759, 868
master - mean (810ms) : 758, 862
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8120) - mean (111ms) : 108, 115
master - mean (110ms) : 105, 115
section Bailout
This PR (8120) - mean (113ms) : 109, 116
master - mean (111ms) : 108, 114
section CallTarget+Inlining+NGEN
This PR (8120) - mean (781ms) : 704, 857
master - mean (770ms) : 701, 838
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8120) - mean (109ms) : 106, 113
master - mean (108ms) : 105, 112
section Bailout
This PR (8120) - mean (110ms) : 107, 113
master - mean (109ms) : 106, 112
section CallTarget+Inlining+NGEN
This PR (8120) - mean (738ms) : 712, 765
master - mean (725ms) : 700, 750
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 (8120) - mean (213ms) : 199, 227
master - mean (211ms) : 201, 221
section Bailout
This PR (8120) - mean (218ms) : 205, 231
master - mean (223ms) : 209, 236
section CallTarget+Inlining+NGEN
This PR (8120) - mean (1,241ms) : 1122, 1361
master - mean (1,224ms) : 1139, 1308
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 (8120) - mean (307ms) : 284, 330
master - mean (301ms) : 289, 314
section Bailout
This PR (8120) - mean (312ms) : 287, 337
master - mean (302ms) : 287, 317
section CallTarget+Inlining+NGEN
This PR (8120) - mean (988ms) : 931, 1045
master - mean (984ms) : 929, 1038
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8120) - mean (291ms) : 277, 305
master - mean (294ms) : 280, 309
section Bailout
This PR (8120) - mean (295ms) : 278, 313
master - mean (302ms) : 280, 324
section CallTarget+Inlining+NGEN
This PR (8120) - mean (975ms) : 901, 1049
master - mean (976ms) : 917, 1034
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8120) - mean (296ms) : 280, 312
master - mean (300ms) : 281, 319
section Bailout
This PR (8120) - mean (300ms) : 282, 318
master - mean (302ms) : 276, 328
section CallTarget+Inlining+NGEN
This PR (8120) - mean (971ms) : 834, 1109
master - mean (965ms) : 858, 1072
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
andrewlock
approved these changes
Feb 2, 2026
tracer/src/Datadog.Trace/Telemetry/Collectors/DependencyTelemetryCollector.cs
Outdated
Show resolved
Hide resolved
| return false; | ||
| } | ||
|
|
||
| // Check prefix character by character to avoid allocation (lowercase only) |
Member
There was a problem hiding this comment.
You know, I'm wondering if we should be using StartsWith() and similar here and in other places in this method, to take advantage of SIMD in newer runtimes 🤔 I might have a little play with benchmarking to see what the result is
Member
There was a problem hiding this comment.
I would use StartsWith() regardless. What allocation are we trying to avoid here?
Member
There was a problem hiding this comment.
FWIW, I explored this, but the results weren't that impressive...
For high number of iterations, where tiered jit kicks in, SIMD can be a lot faster (2ns vs 14ns) for the "worst" case scenario (where a match is found)
| Method | Categories | Mean | Ratio |
|---|---|---|---|
| TempPathTrue_BestCase_Simd | BestCase | 2.543 ns | 1.33 |
| TempPathTrue_BestCase_Original | BestCase | 1.911 ns | 1.00 |
| TempPathTrue_WorstCase_Simd | WorstCase | 2.557 ns | 0.18 |
| TempPathTrue_WorstCase_Original | WorstCase | 14.230 ns | 1.00 |
However, making some minor modifications (remove branches, aggressively inline) to the current code makes the discrepancy much less:
| Method | Categories | Mean | Ratio |
|---|---|---|---|
| TempPathTrue_BestCase_Simd | BestCase | 2.5783 ns | 2.61 |
| TempPathTrue_BestCase_Updated | BestCase | 0.9896 ns | 1.00 |
| TempPathTrue_WorstCase_Simd | WorstCase | 2.5189 ns | 0.78 |
| TempPathTrue_WorstCase_Updated | WorstCase | 3.2491 ns | 1.00 |
and if we force the benchmarks to be low repetition and to not hit Tier 1 jitting, simd looks much worse overall:
| Method | Categories | Mean | Ratio |
|---|---|---|---|
| TempPathTrue_BestCase_Simd | BestCase | 28.898 ns | 4.15 |
| TempPathTrue_BestCase_Updated | BestCase | 6.992 ns | 1.00 |
| TempPathTrue_WorstCase_Simd | WorstCase | 32.573 ns | 1.03 |
| TempPathTrue_WorstCase_Updated | WorstCase | 31.874 ns | 1.01 |
Given that this is prob only called 100-1000 times(?ish, i.e. once per assembly), we need to think about the "early" cases almost as much as the "later" cases. So I think not using SIMD is the better option, and minor tweaks to existing code is prob"good enough".
I'll look at making those tweaks in subsequent PRs
Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
This filters out more dynamic assemblies that we don't want to send to telemetry due to cardinality issues. The main
additions are:
a43d8b99ea)dynamicclasses254)Both patterns only apply to assemblies with version 0.0.0.0.
Reason for change
Our telemetry is still sending a lot of dynamically generated assemblies. The backend is being updated to filter these out, but we should avoid sending them in the first place to reduce payload size and processing overhead.
From doing some manual testing these can slip by our
IsDynamiccheck via:Implementation details
Extended
IsZeroVersionAssemblyPatterninDependencyTelemetryCollectorto recognize two additional patterns:IsHexStringhelper (already used for 32+ hex char filtering)IsDynamicClassesPatternmethod that checks for the literal lowercase prefix followed byone or more digits
Both checks use the character-by-character comparison.
Test coverage
Updated the tests to check for the new cases
Other details
Followed the existing design where we do the length and character-by-character comparison, unsure how scalable / maintainable / overhead this is, I guess it isn't too bad, but am a little concerned just tacking more stuff on here.