Skip to content

[NativeAOT] Avoid redundant unwind info lookup during stackwalks - #87419

Merged
jkotas merged 1 commit into
dotnet:mainfrom
jkotas:redundant-lookup
Jun 13, 2023
Merged

[NativeAOT] Avoid redundant unwind info lookup during stackwalks#87419
jkotas merged 1 commit into
dotnet:mainfrom
jkotas:redundant-lookup

Conversation

@jkotas

Copy link
Copy Markdown
Member

Unwind info was looked up twice on non-Windows: Once during initial frame inspection and second during the actual unwind. Cache the lookup results from the initial frame inspection and use them from the actual unwind later, same as it is done on Windows.

Unwind info was looked up twice on non-Windows: Once during initial frame inspection
and second during the actual unwind. Cache the lookup results from the
initial frame inspection and use them from the actual unwind later, same
as it is done on Windows.
@jkotas
jkotas requested a review from VSadovJune 12, 2023 17:50
@ghostghost assigned jkotasJun 12, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Unwind info was looked up twice on non-Windows: Once during initial frame inspection and second during the actual unwind. Cache the lookup results from the initial frame inspection and use them from the actual unwind later, same as it is done on Windows.

Author:jkotas
Assignees:jkotas
Labels:

area-NativeAOT-coreclr

Milestone:-

@jkotas

Copy link
Copy Markdown
MemberAuthor

Todo API w/ Workstation GC:

Before this change:

| Requests/sec | 72,812 |

With this change:

| Requests/sec | 76,727 |

@jkotas

Copy link
Copy Markdown
MemberAuthor

This is follow up on #86690 (comment) . I have more changes on top this one to make stack unwinding faster on Unix.

@jkotasjkotas added the tenet-performance Performance related issue label Jun 12, 2023
@filipnavara

Copy link
Copy Markdown
Member

Related: #75807

@dotnetdotnet deleted a comment from azure-pipelinesBotJun 12, 2023
@jkotas

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@VSadov

Copy link
Copy Markdown
Member

Unwind info was looked up twice on non-Windows

I think the code makes assumption that fetching this info is a relatively cheap operation - like indexing in a table, but it is more complex than that. And there is space in the MethodInfo to keep the info. Nice change!

@VSadovVSadov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thanks!

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jkotas@filipnavara@VSadov