Fix Kotlin Debug source mapping#11803
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcb11d1d64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| StratumExt stratumDebug = sourceMap.getStratum("KotlinDebug"); | ||
| if (stratumDebug == null) { | ||
| throw new IllegalArgumentException("No stratumDebug found for KotlinDebug"); | ||
| stratumDebug = new StratumExt("KotlinDebug"); |
There was a problem hiding this comment.
Avoid emitting mixed mappings for no-debug Kotlin SMAPs
When a Kotlin class has a SourceDebugExtension but omits KotlinDebug (the added f4 case generates SymbolExtraction16$f4$$inlined$groupingBy$1 with SourceFile: _Collections.kt and only a Kotlin stratum), this empty stratum sends every line through the main-stratum fallback. That fallback can map some bytecode lines to the app file while leaving others in the Kotlin library file, but SymbolExtractor still labels the entire scope with classNode.sourceFile, so we now emit nonsensical/mixed file-line ranges instead of just avoiding the exception. Classes with no KotlinDebug need to be skipped or remapped with a consistent source file.
Useful? React with 👍 / 👎.
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 9 metrics, 6 unstable metrics. See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (339.901 µs) : 309, 371
. : milestone, 340,
basic (300.528 µs) : 293, 308
. : milestone, 301,
loop (8.972 ms) : 8967, 8977
. : milestone, 8972,
section candidate
noprobe (340.438 µs) : 297, 383
. : milestone, 340,
basic (299.539 µs) : 292, 307
. : milestone, 300,
loop (8.969 ms) : 8962, 8975
. : milestone, 8969,
|
dudikeleti
left a comment
There was a problem hiding this comment.
Just make sure to address this comment, I can't tell if it's really issue.
ojung
left a comment
There was a problem hiding this comment.
Can't say I understand how this works, but I'm assuming you test this in staging :)
KotlinDebug stratum in SMAP is not always emitted if not present make one empty and avoid throwing exception
fcb11d1 to
778d4a9
Compare
|
🎯 Code Coverage (details) 🔗 Commit SHA: c05c200 | Docs | Datadog PR Page | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
KotlinDebug stratum in SMAP is not always emitted
if not present make one empty and avoid throwing exception
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: DEBUG-5799