Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Add a mode that disables reflection #7208

Merged
merged 7 commits into from
Apr 2, 2019

Conversation

MichalStrehovsky
Copy link
Member

Fixes #6897.

This adds an option to disable generation of reflection data in the compiler.

Fixes dotnet#6897.

This adds an option to disable generation of reflection data in the compiler. A Hello world binary will shrink to about 1.7 MB with this enabled.

There is still some work left on the native layout metadata generation side (this is needed to make default EqualityComparer/Comparer to work). Native layout metadata generation in the compiler assumes that when reflection is disabled, no native layout is needed - and when reflection is enabled, everything should get native layout metadata. The correct behavior is a middle ground - we're not doing the right thing in either of those cases (in one case we do too much, in the other we do too little).
@MichalStrehovsky
Copy link
Member Author

There is still some work left on the native layout metadata generation side in the compiler - default EqualityComparer/Comparer won't work with this option enabled right now. When this option is enabled, we crash at shutdown because the shutdown path makes a new Dictionary type instance in AppContext and Dictionary touches comparers for annoying reasons.

Native layout metadata generation in the compiler assumes that when reflection is disabled, no native layout is needed - and when reflection is enabled, everything should get native layout metadata. The correct behavior is a middle ground - we're not doing the right thing in either of those cases (in one case we make too much native layout, in the other case we do too little native layout). I'm not in the mood to fix that right now because it looks like a pain to coordinate between Project N and here.

MichalStrehovsky added a commit to MichalStrehovsky/corert that referenced this pull request Mar 24, 2019
Adding to make it possible to work around current problems in dotnet#7208 when reflection is disabled.

Comparers also root the entire type loader, so making this optional puts as on a path where we can make type loader optional too. Type loader is about 500 kB of junk. If we can remove it, our Hello world size becomes competitive with Go. People who are willing to walk the extra mile to make their code compatible with this get very small deployment sizes that are especially important for e.g. the WASM target.
protected override bool IsPrimitiveImpl()
{
return RuntimeAugments.IsPrimitive(_typeHandle);
}
Copy link
Member

Choose a reason for hiding this comment

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

There are more methods to implement here, e.g. IsAssignableFrom

@jkotas
Copy link
Member

jkotas commented Mar 24, 2019

I think it would be better for this no-reflection RuntimeType to live in CoreLib and have (weak) reference to the optional full reflection support. Same as in CoreCLR. It would make the CoreLib sharing easier and it would get us on path to fix the less than ideal memory footprint of CoreRT reflection (the reflection caches stay around forever vs. they are collectible in CoreCLR).

I understand that you may not want to have appetite to do this refactoring. Just mentioning it.

@@ -63,4 +63,4 @@ The compiler can build insights into how reflection is used by analyzing the use

### Assume nothing is accessed dynamically ###

In CoreRT, reflection metadata (names of types, list of their methods, fields, signatures, etc.) is _optional_. The CoreRT runtime has its own minimal version of the metadata that represents the minimum required to execute managed code (think: base type and list of interfaces, offsets to GC pointers within an instance of the type, pointer to the finalizer, etc.). The metadata used by the reflection subsystem within the base class libraries is only used by the reflection stack and is not necessary to execute non-reflection code. For a .NET app that doesn't use reflection, the compiler can skip generating the reflection metadata completely. This option is currently [not publicly exposed](https://github.com/dotnet/corert/issues/6897), but it exists. People who would like to totally minimize the size of their applications or obfuscate their code could be interested in this option.
In CoreRT, reflection metadata (names of types, list of their methods, fields, signatures, etc.) is _optional_. The CoreRT runtime has its own minimal version of the metadata that represents the minimum required to execute managed code (think: base type and list of interfaces, offsets to GC pointers within an instance of the type, pointer to the finalizer, etc.). The metadata used by the reflection subsystem within the base class libraries is only used by the reflection stack and is not necessary to execute non-reflection code. For a .NET app that doesn't use reflection, the compiler can skip generating the reflection metadata completely. People who would like to totally minimize the size of their applications or obfuscate their code could be interested in this option, although not much existing real world code would be expected to work with this (including a lot of the framework code).
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I expect that Unity tiny mode will come with similar limitations.

@jkotas
Copy link
Member

jkotas commented Mar 24, 2019

shutdown path makes a new Dictionary type instance in AppContext

Do you happen to have a stack trace for this around?

@jkotas
Copy link
Member

jkotas commented Mar 24, 2019

LGTM otherwise

@MichalStrehovsky
Copy link
Member Author

Do you happen to have a stack trace for this around?

I don't, but it was coming from this line:

private static readonly Dictionary<string, object> s_dataStore = new Dictionary<string, object>();

Passing a custom comparer didn't help, because the first thing Dictionary does in the constructor is that it checks the passed in comparer is not the default comparer - it calls into the type loader to load it either way.

We are running the cctor because AppContext.OnProcessExit touches the statics.

I'm thinking that maybe having some private API to check if the passed in comparer is the default comparer (without actually loading the default comparer) might be overall goodness.

@MichalStrehovsky
Copy link
Member Author

I think it would be better for this no-reflection RuntimeType to live in CoreLib and have (weak) reference to the optional full reflection support

I know the structure of the reflection stack has been your pet peeve for a while, but I'm focusing my weekend activities on things that have the highest impact with the least amount of work right now. System.Private.DisabledReflection can be deleted without replacement once we restructure the reflection stack in the way you would like. Since this addition doesn't create debt (I placed it in a leaf location that nobody else depends on/is aware of), let's go with this for now.

</PropertyGroup>
<ItemGroup Condition="'$(IsProjectNLibrary)' != 'true'">
<ProjectReference Include="..\..\System.Private.CoreLib\src\System.Private.CoreLib.csproj">
<Aliases>global,System_Private_CoreLib</Aliases>
Copy link
Member

Choose a reason for hiding this comment

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

I am deleting these aliases in #7216 . They are no longer needed.

jkotas pushed a commit that referenced this pull request Mar 25, 2019
Adding to make it possible to work around current problems in #7208 when reflection is disabled.

Comparers also root the entire type loader, so making this optional puts as on a path where we can make type loader optional too. Type loader is about 500 kB of junk. If we can remove it, our Hello world size becomes competitive with Go. People who are willing to walk the extra mile to make their code compatible with this get very small deployment sizes that are especially important for e.g. the WASM target.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants