Handle duplicate keys in ASM DecodeMap - #8951
Conversation
BenchmarksBenchmark execution time: 2026-07-28 13:36:49 Comparing candidate commit cb1ac7f in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 70 metrics, 0 unstable metrics, 63 known flaky benchmarks, 63 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8951) 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 (8951) - mean (74ms) : 69, 79
master - mean (73ms) : 69, 77
section Bailout
This PR (8951) - mean (78ms) : 74, 83
master - mean (75ms) : 72, 77
section CallTarget+Inlining+NGEN
This PR (8951) - mean (1,093ms) : 1045, 1141
master - mean (1,086ms) : 1043, 1129
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 (8951) - mean (114ms) : 108, 120
master - mean (111ms) : 105, 118
section Bailout
This PR (8951) - mean (112ms) : 109, 115
master - mean (110ms) : 107, 112
section CallTarget+Inlining+NGEN
This PR (8951) - mean (785ms) : 760, 810
master - mean (780ms) : 755, 806
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8951) - mean (102ms) : 96, 107
master - mean (101ms) : 95, 107
section Bailout
This PR (8951) - mean (99ms) : 97, 102
master - mean (99ms) : 96, 101
section CallTarget+Inlining+NGEN
This PR (8951) - mean (951ms) : 907, 995
master - mean (944ms) : 909, 979
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8951) - mean (97ms) : 93, 102
master - mean (96ms) : 93, 99
section Bailout
This PR (8951) - mean (102ms) : 98, 107
master - mean (102ms) : 95, 109
section CallTarget+Inlining+NGEN
This PR (8951) - mean (821ms) : 783, 859
master - mean (824ms) : 778, 870
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 (8951) - mean (211ms) : 206, 216
master - mean (206ms) : 201, 211
section Bailout
This PR (8951) - mean (214ms) : 209, 220
master - mean (211ms) : 206, 215
section CallTarget+Inlining+NGEN
This PR (8951) - mean (1,273ms) : 1224, 1322
master - mean (1,236ms) : 1181, 1292
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 (8951) - mean (305ms) : 297, 312
master - mean (298ms) : 292, 305
section Bailout
This PR (8951) - mean (307ms) : 299, 315
master - mean (297ms) : 291, 304
section CallTarget+Inlining+NGEN
This PR (8951) - mean (1,007ms) : 981, 1033
master - mean (992ms) : 970, 1014
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8951) - mean (296ms) : 287, 305
master - mean (290ms) : 284, 296
section Bailout
This PR (8951) - mean (295ms) : 288, 302
master - mean (291ms) : 284, 297
section CallTarget+Inlining+NGEN
This PR (8951) - mean (1,201ms) : 1156, 1246
master - mean (1,189ms) : 1153, 1226
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8951) - mean (297ms) : 291, 303
master - mean (290ms) : 281, 299
section Bailout
This PR (8951) - mean (301ms) : 293, 310
master - mean (288ms) : 283, 294
section CallTarget+Inlining+NGEN
This PR (8951) - mean (1,113ms) : 1003, 1224
master - mean (1,063ms) : 1001, 1126
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Co-authored-by: Steven Bouwkamp <steven.bouwkamp@datadoghq.com>
Summary of changes
Handle duplicate keys being returned by WAF
Reason for change
We saw errors which look like they come from the waf returning duplicate keys, which was causing the
dictionary.Add()call to throwImplementation details
Initially, I was using
TryAdd()in .NET Core andContains()followed byAdd()in .NET Framework. but I don't know if it's worth that overhead for what is clearly a rare strange scenario, so switched just to a replace instead.That said someone from ASM should validate 1. do we Expect duplicate keys coming from the waf here?
Test coverage
Meh