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

Better reflection type name parser for the AOT compiler #72833

Closed
MichalStrehovsky opened this issue Jul 26, 2022 · 1 comment · Fixed by #83657
Closed

Better reflection type name parser for the AOT compiler #72833

MichalStrehovsky opened this issue Jul 26, 2022 · 1 comment · Fixed by #83657

Comments

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Jul 26, 2022

We should refactor the type name parser used by the reflection stack to be reusable from the compiler. IL Linker already uses that parser, but we don't want another fork - we want a way to compile the exact same file, potentially with ifdefs if needed for both runtime reflection stack and the compiler.

// NativeAOT doesn't have a fully capable type name resolver yet
// Once this is implemented don't forget to wire up marking of type forwards which are used in generic parameters
if (!ILCompiler.DependencyAnalysis.ReflectionMethodBodyScanner.ResolveType(typeName, callingModule, diagnosticContext.Origin.MemberDefinition!.Context, out TypeDesc foundType, out ModuleDesc referenceModule))
{
type = default;
return false;
}

Success criteria:

Type? t = Type.GetType("System.Collections.Generic.List`1[[System.DayOfWeek, System.Runtime]], System.Collections");
Console.WriteLine(t != null);

Should print non-null.

@vitek-karas
Copy link
Member

Also the parser should detect type names without assembly qualifier and produce IL2105 warning (search for this number in the linker tests to see the expected behavior).

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 20, 2023
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 added a commit that referenced this issue Mar 30, 2023
* Proper type name parser for native AOT compiler

* Track failure of CA search rules in the type name parser

* Fix tests

* Suppress new native AOT warning for libraries tests

Co-authored-by: vitek-karas <[email protected]>

Fixes #72833
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 30, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants