[Profiler] Fix UB in DebugInfoStore::Get(): re-acquire iterator after map insertion#8468
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8468) 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 (8468) - mean (72ms) : 69, 75
master - mean (74ms) : 70, 79
section Bailout
This PR (8468) - mean (78ms) : 74, 83
master - mean (78ms) : 74, 81
section CallTarget+Inlining+NGEN
This PR (8468) - mean (1,076ms) : 1034, 1119
master - mean (1,074ms) : 1032, 1116
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 (8468) - mean (113ms) : 108, 117
master - mean (116ms) : 109, 122
section Bailout
This PR (8468) - mean (119ms) : 112, 125
master - mean (115ms) : 111, 119
section CallTarget+Inlining+NGEN
This PR (8468) - mean (797ms) : 774, 820
master - mean (793ms) : 771, 814
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8468) - mean (103ms) : 98, 108
master - mean (103ms) : 97, 109
section Bailout
This PR (8468) - mean (101ms) : 98, 105
master - mean (102ms) : 97, 106
section CallTarget+Inlining+NGEN
This PR (8468) - mean (943ms) : 898, 987
master - mean (938ms) : 906, 970
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8468) - mean (101ms) : 95, 107
master - mean (99ms) : 95, 104
section Bailout
This PR (8468) - mean (104ms) : 97, 110
master - mean (102ms) : 97, 107
section CallTarget+Inlining+NGEN
This PR (8468) - mean (824ms) : 784, 865
master - mean (825ms) : 775, 875
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 (8468) - mean (208ms) : 203, 213
master - mean (206ms) : 202, 210
section Bailout
This PR (8468) - mean (211ms) : 207, 214
master - mean (210ms) : 205, 214
section CallTarget+Inlining+NGEN
This PR (8468) - mean (1,215ms) : 1166, 1264
master - mean (1,206ms) : 1168, 1243
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 (8468) - mean (300ms) : 294, 306
master - mean (299ms) : 293, 305
section Bailout
This PR (8468) - mean (300ms) : 294, 306
master - mean (300ms) : 295, 304
section CallTarget+Inlining+NGEN
This PR (8468) - mean (1,003ms) : 978, 1027
master - mean (997ms) : 962, 1031
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8468) - mean (294ms) : 287, 301
master - mean (294ms) : 286, 302
section Bailout
This PR (8468) - mean (294ms) : 288, 300
master - mean (293ms) : 288, 298
section CallTarget+Inlining+NGEN
This PR (8468) - mean (1,175ms) : 1130, 1221
master - mean (1,176ms) : 1138, 1213
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8468) - mean (291ms) : 284, 298
master - mean (293ms) : 285, 300
section Bailout
This PR (8468) - mean (292ms) : 287, 296
master - mean (293ms) : 287, 298
section CallTarget+Inlining+NGEN
This PR (8468) - mean (1,074ms) : 991, 1156
master - mean (1,064ms) : 997, 1131
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmark execution time: 2026-04-16 13:01:10 Comparing candidate commit 6e49e41 in PR branch Found 1 performance improvements and 1 performance regressions! Performance is the same for 25 metrics, 0 unstable metrics, 87 known flaky benchmarks.
|
Summary of changes
Inspired by #8283 (review)
Reason for change
In
DebugInfoStore::Get(),ParseModuleDebugInfo(moduleId)inserts into the_modulesInfounordered_map via_modulesInfo[moduleId], which may trigger a rehash that invalidates all iterators. The old code reused a stale iterator obtained before the insertion — undefined behavior per the C++ standard. Fix: re-acquire the iterator via find() after insertion, and simplify the now-unnecessary ternary.Implementation details
re-acquire the iterator
Test coverage
Other details