Skip to content

Delete stackwalk cache #99137

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

Merged
merged 2 commits into from
Mar 6, 2024
Merged

Delete stackwalk cache #99137

merged 2 commits into from
Mar 6, 2024

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Mar 1, 2024

It costs about as much as it saves on x64. It may be helping a little bit on x86, but it is not worth it to keep it around for that.

@AustinWise
Copy link
Contributor

CI should catch this, but just in case: my recently merged #98908 added a new call to the StackWalkFunctions that is deleted in this PR:

StackWalkFunctions(GetThread(), FindFailFastCallerCallback, &findCallerData);

To fix, replace with:

GetThread()->StackWalkFrames(FindFailFastCallerCallback, &findCallerData, FUNCTIONSONLY | QUICKUNWIND);

@@ -2716,7 +2716,7 @@ class Thread
// stack crawl. Eventually need to always do this but it
// breaks the debugger right now.

#define LIGHTUNWIND 0x0020 // allow using cache schema (see StackwalkCache class)
Copy link
Member Author

@jkotas jkotas Mar 5, 2024

Choose a reason for hiding this comment

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

There is confusion about what's called LIGHTUNWIND vs. QUICKUNWIND. The implementation of the LIGHTUNWIND was under HAS_QUICKUNWIND ifdef and otherwise used Quick Unwind names.

The distinction between LIGHTUNWIND vs. QUICKUNWIND seems to be important for Windows x86 unwinding in abnormal situations, so I am going to keep it, but I am going to rename everything related to light unwind to be called light unwind.

@jkotas jkotas force-pushed the stackwalkcache branch 4 times, most recently from 7f3f965 to 12113ec Compare March 5, 2024 03:20
… bit on x86, but it is not worth it to keep it around for that.
@jkotas jkotas merged commit 33fe041 into dotnet:main Mar 6, 2024
@jkotas jkotas deleted the stackwalkcache branch March 6, 2024 03:49
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants