[Dynamic Instrumentation] DEBUG-5101 line probe two segment fallback#8543
Merged
dudikeleti merged 2 commits intoMay 19, 2026
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 733f8955fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+551
to
+552
| var suffixMatch = GetClosestPathBySuffix(pathQuery, querySegmentCount); | ||
| return suffixMatch.IsSuccessful ? suffixMatch.BestQualifiedPath : null; |
There was a problem hiding this comment.
When this new fallback succeeds, TryFindAssemblyContainingFile treats the returned path as an exact match and returns before scanning the remaining loaded assemblies. If two loaded assemblies both have a PDB document ending in the same source suffix (for example src/Shared/Feature/MyFile.cs) and the probe path only differs by casing, the first assembly enumerated is bound even though the match is globally ambiguous; the ambiguity guard added for fallback matches is bypassed here. Consider keeping this helper as a pure exact lookup or returning enough match metadata so the caller can apply the same global uniqueness check.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Will be handled in a follow-up PR.
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8543) 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 (8543) - mean (73ms) : 70, 76
master - mean (74ms) : 69, 78
section Bailout
This PR (8543) - mean (77ms) : 75, 79
master - mean (78ms) : 75, 81
section CallTarget+Inlining+NGEN
This PR (8543) - mean (1,120ms) : 1038, 1202
master - mean (1,107ms) : 1057, 1157
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 (8543) - mean (114ms) : 110, 118
master - mean (113ms) : 109, 117
section Bailout
This PR (8543) - mean (117ms) : 111, 123
master - mean (117ms) : 111, 124
section CallTarget+Inlining+NGEN
This PR (8543) - mean (792ms) : 769, 816
master - mean (784ms) : 763, 805
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8543) - mean (102ms) : 98, 106
master - mean (105ms) : 100, 110
section Bailout
This PR (8543) - mean (102ms) : 99, 104
master - mean (104ms) : 99, 108
section CallTarget+Inlining+NGEN
This PR (8543) - mean (952ms) : 911, 993
master - mean (951ms) : 916, 987
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8543) - mean (102ms) : 97, 107
master - mean (100ms) : 95, 104
section Bailout
This PR (8543) - mean (101ms) : 98, 105
master - mean (104ms) : 98, 109
section CallTarget+Inlining+NGEN
This PR (8543) - mean (823ms) : 788, 858
master - mean (821ms) : 793, 849
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 (8543) - mean (200ms) : 196, 205
master - mean (201ms) : 194, 207
section Bailout
This PR (8543) - mean (204ms) : 201, 207
master - mean (205ms) : 201, 210
section CallTarget+Inlining+NGEN
This PR (8543) - mean (1,212ms) : 1136, 1289
master - mean (1,207ms) : 1147, 1267
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 (8543) - mean (290ms) : 282, 298
master - mean (288ms) : 280, 297
section Bailout
This PR (8543) - mean (290ms) : 285, 295
master - mean (290ms) : 283, 298
section CallTarget+Inlining+NGEN
This PR (8543) - mean (968ms) : 946, 991
master - mean (961ms) : 938, 984
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8543) - mean (282ms) : 277, 287
master - mean (280ms) : 270, 289
section Bailout
This PR (8543) - mean (283ms) : 278, 287
master - mean (280ms) : 272, 287
section CallTarget+Inlining+NGEN
This PR (8543) - mean (1,162ms) : 1116, 1207
master - mean (1,160ms) : 1125, 1195
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8543) - mean (281ms) : 274, 287
master - mean (280ms) : 271, 290
section Bailout
This PR (8543) - mean (279ms) : 274, 285
master - mean (281ms) : 271, 290
section CallTarget+Inlining+NGEN
This PR (8543) - mean (1,039ms) : 990, 1089
master - mean (1,040ms) : 1005, 1076
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dudikeleti
force-pushed
the
dudik/debug-5101-line-probe-two-segment-fallback
branch
from
May 11, 2026 12:38
733f895 to
4e04d5d
Compare
dudikeleti
marked this pull request as draft
May 11, 2026 16:07
Support DEBUG-5101 source path fallback matches when the last two segments uniquely identify a loaded symbolicated assembly, while preserving diagnostics that explain the fallback binding. Made-with: Cursor
dudikeleti
force-pushed
the
dudik/debug-5101-line-probe-two-segment-fallback
branch
from
May 19, 2026 10:47
4e04d5d to
fd06c02
Compare
dudikeleti
marked this pull request as ready for review
May 19, 2026 10:48
tylfin
approved these changes
May 19, 2026
dudikeleti
enabled auto-merge (squash)
May 19, 2026 11:02
BenchmarksBenchmark execution time: 2026-05-19 11:47:26 Comparing candidate commit fd06c02 in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in baseline:
Found 2 performance improvements and 4 performance regressions! Performance is the same for 51 metrics, 15 unstable metrics, 88 known flaky benchmarks, 38 flaky benchmarks without significant changes.
|
Contributor
|
dudikeleti
disabled auto-merge
May 19, 2026 12:22
dudikeleti
enabled auto-merge (squash)
May 19, 2026 12:25
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
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
LineProbeResolverto support line probe source-file fallback matches when only the last 2 path segments match (down from 4).FallbackTrailingSuffixMatchtogether with the matched trailing-segment count.LineProbeResolverTestcoverage for 2-segment fallback binding and for picking the highest-scoring candidate when incidental low-scoring matches exist.Reason for change
DEBUG-5101requires line probes to resolve when the configured source path differs from the PDB path except for the trailing directory and filename (for exampleunknown-prefix/Controllers/HomeController.csagainst a PDB path ending inControllers/HomeController.cs).Implementation details
MinTrailingSegmentsForFallbackMatchfrom 4 to 2 so resolver fallback can match scenarios likeunknown-prefix/Controllers/File.csagainst a loaded symbolicated assembly path ending inControllers/File.cs.BestFallbackMatchSelection(replacing the standaloneref int qualifiedFallbackMatchCountparameter onTrackClosestPathMatch) and expose it asQualifiedMatchCountso the diagnostics path keeps the same information.HasAmbiguousBestMatch).LineProbePathMatchType.FallbackTrailingSuffixMatchtogether with the trailing-segment count used to resolve the probe.Test coverage
LineProbeResolverTestFallbackMatchBindsWhenOnlyTwoTrailingSegmentsMatch)BestFallbackMatchSelectionBindsHighestScoreOverIncidentalLowScoringCandidates)BestFallbackMatchSelectionBindsWhenSingleQualifiedCandidateIsTracked)