-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Defer resolution of StackTrace debug info until use #115019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
38fb5cc
cd7ee17
f23f1fe
62a11ef
110eef1
47cd097
da5fbca
9185c18
90a75dc
28dbd25
4c0a1de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -95,5 +95,10 @@ internal void ToString(TraceFormat traceFormat, StringBuilder builder) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| builder.AppendLine(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private StackFrame[] GetFramesCore() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _stackFrames ?? []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should NativeAOT get the same optimization? We do not want NativeAOT performance to be lagging behind. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if debug info resolution is as significant on NAOT; I haven't benched it. My driving scenario is JIT-only for entirely separate reasons anyway. It shouldn't be too hard to do there, though I'd want to profile it first. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency I think doing the same defered StackFrame creation would be nice. I suspect the optimization won't make a substantial difference unless it is run together with a DeveloperExperience implementation that does more appreciable work: runtime/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs Lines 84 to 118 in d34eb61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I would be good to see some numbers.
DeveloperExperience is on by default in NativeAOT. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to also defer Windows PDB data resolution, but that requires some nontrivial changes in the runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed but totally content if it is out of scope for this PR.