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

Unify type name parser in linker once is in runtime #77868

Closed
Tracked by #75278
tlakollo opened this issue Nov 3, 2022 · 4 comments
Closed
Tracked by #75278

Unify type name parser in linker once is in runtime #77868

tlakollo opened this issue Nov 3, 2022 · 4 comments
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Milestone

Comments

@tlakollo
Copy link
Contributor

tlakollo commented Nov 3, 2022

Linker used o have two dependencies in the external partition, the cecil submodule (which has been converted in an internal arcade package to avoid having a submodule in runtime) and a small subset of files copied from corert. These set of corert files have since then been ported by nativeAOT, given that NativeAOT lives inside the runtime, we could just reference the most recent files and delete the external copy inside linker partition.

@tlakollo tlakollo self-assigned this Nov 3, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 3, 2022
@tlakollo tlakollo added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Nov 3, 2022
@MichalStrehovsky
Copy link
Member

Overlaps with #72833. Once this is done, #72833 should be trivial.

@marek-safar marek-safar removed the untriaged New issue has not been triaged by the area owner label Nov 7, 2022
@marek-safar marek-safar added this to the 8.0.0 milestone Nov 7, 2022
jkotas added a commit that referenced this issue Mar 25, 2023
Contributes to #72833 and #77868

The performance effect of this change on typical use of `Type.GetType` like `Type.GetType("MyType, MyAssembly")` is in the noise range. The typical use of `Type.GetType` spends most of the time in assembly loader and type loader. The time spent by parsing the type name is small fraction of the total and the performance improvement is hardly noticeable.

When the type name parser performance is measured in isolation, it is several times faster compared to the existing unmanaged CoreCLR type name parser. For example:
```
Type.GetType("System.Object, System.Private.CoreLib",
       assemblyResolver: (an) => typeof(object).Assembly,
       typeResolver: (assembly, name, ignoreCase) => typeof(object));
```
is about 3x faster with this change on CoreCLR.

Co-authored-by: Aaron Robinson <[email protected]>
@jkotas
Copy link
Member

jkotas commented Mar 31, 2023

This is shovel-ready.

It should follow the same pattern as #83657: Define TypeNameParser partial class under linker that bridges the common TypeNameParser implementation and illinker-specific type system object model.

@agocke agocke modified the milestones: 8.0.0, 9.0.0 Sep 5, 2023
@sbomer sbomer added this to AppModel Aug 13, 2024
@sbomer
Copy link
Member

sbomer commented Aug 13, 2024

This was fixed by #103740.

@sbomer sbomer closed this as completed Aug 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
Archived in project
Development

No branches or pull requests

6 participants