Skip to content
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

Avoid touching NameMangler during scanning #79738

Merged
merged 3 commits into from
Dec 16, 2022

Conversation

MichalStrehovsky
Copy link
Member

When we're running in a scan-only mode (such as when running trimming tests), we were still computing mangled names for things because two nodes eagerly needed them. This structures things a bit differently so that we can finish scanning without ever touching the NameMangler.

This is also an improvement for end user scenarios because we really don't need them during scanning (we avoid the work), and we don't have to hold on to a useless string for who knows how many GC collections.

Cc @dotnet/ilc-contrib

When we're running in a scan-only mode (such as when running trimming tests), we were still computing mangled names for things because two nodes eagerly needed them. This structures things a bit differently so that we can finish scanning without ever touching the `NameMangler`.

This is also an improvement for end user scenarios because we really don't need them during scanning (we avoid the work), and we don't have to hold on to a useless string for who knows how many GC collections.
@ghost
Copy link

ghost commented Dec 16, 2022

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

Issue Details

When we're running in a scan-only mode (such as when running trimming tests), we were still computing mangled names for things because two nodes eagerly needed them. This structures things a bit differently so that we can finish scanning without ever touching the NameMangler.

This is also an improvement for end user scenarios because we really don't need them during scanning (we avoid the work), and we don't have to hold on to a useless string for who knows how many GC collections.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@vitek-karas
Copy link
Member

Just curious - is this perf-only optimization, or is there some other reason to avoid touching that class?

@MichalStrehovsky
Copy link
Member Author

Just curious - is this perf-only optimization, or is there some other reason to avoid touching that class?

Perf - the mangled names need to be stable, so one of the things NameMangler does is that if we need a mangled name for a type, we need to look at all types in the assembly. Similarly, a mangled name for a method looks at all methods on the type. If we can avoid doing some of the work, it's better. During scanning, we might be looking at more things than compilation will need.

I almost went as far as keeping NameMangler as null, but we'd need it if we write scanner DGML logs or if the ETW tracing is enabled.

@jkotas jkotas merged commit 798da0c into dotnet:main Dec 16, 2022
@MichalStrehovsky MichalStrehovsky deleted the nomangle branch December 17, 2022 05:44
@ghost ghost locked as resolved and limited conversation to collaborators Jan 16, 2023
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