Skip to content

[DRAFT] Use il offset walker instead of debugger data to gather the IL offset during stack trace generation #116031

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

[DRAFT] Use il offset walker instead of debugger data to gather the IL offset during stack trace generation #116031

wants to merge 7 commits into from
+749 −42

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented May 27, 2025 •
edited
Loading

Add a path for reading the IL-Native map which doesn't create a DebuggerFunctionInfo or DebuggerJitInfo

Use it in the stackwalker code for EH.

Draft because:

  • Untested
  • Not known what impact this has on the debugger viewing crash dumps (It appears it may break certain stackwalking things)
  • Not known what impact this has on performance. (Looking at the offset stream is slower, but avoid locks. The change now has a fixed size cache to optimize this particular scenario)
  • Factoring of the code is wrong. It's likely that we want this code to be a general purpose helper instead of being directly called from the stackwalk generation path.

@davidwrighton
First pass at removing use of the debugger copy of the IL to native map
a1a9fd8 
- This removes it from normal path
- This also adds an assert that it is correct

Logically, this code should probably replace the current implementation of GetILOffsetFromNative but this first pass is just to get the code working

This is untested, but it does compile.
@davidwrighton
Get the helper to work correctly
00c702a
@Copilot Copilot AI review requested due to automatic review settings May 27, 2025 18:12
@davidwrighton davidwrighton marked this pull request as draft May 27, 2025 18:12
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Copilot
Copilot AI reviewed May 27, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@build-analysis build-analysis bot mentioned this pull request May 27, 2025
Open
3 tasks
@build-analysis build-analysis bot mentioned this pull request May 28, 2025
Open
3 tasks
@davidwrighton
- Make the NibbleReader a bit faster by loading data in pointer sized…
1e03ed6 
… chunks and doing very cheap operations per nibble read

- Add a fixed size cache around the native to IL mapping operations in the stacktrace logic
  - Unfortunately, simply reading the nibble data is pretty slow, so this is needed to avoid regressing performance
  - With this change the performance of newing up a StackTrace object on a multithreaded environment is primarily driven by GC allocation rate, and not significantly by any other locks.
@davidwrighton
Missed change to COOPERATIVE mode in contract
f13394d
@davidwrighton
Fix epilog handling
fe8a4e5
This was referenced May 29, 2025
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot

At least 1 approving review is required to merge this pull request.

Assignees

@davidwrighton davidwrighton

Labels
area-Diagnostics-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@davidwrighton