Delete reflection-free mode

[MichalStrehovsky]

This week in "contributions of the reflection-free mode to society":

• Hit a build break in Replace DAMT.All with DAMT.AllMethods in delegate creation #109772 because of the alternate reflection stack that cost me a minute to fix
• This interaction
• Found mention of IlcDisableReflection in the codebase of an internal project. A shipping project.
• 

It's time for this to go.

Cc @dotnet/ilc-contrib

[am11]

Those slides seem to have a typo StackTraceSupport=true should be false. 🙂

[MichalStrehovsky]

Those slides seem to have a typo StackTraceSupport=true should be false. 🙂

Also mention the never documented and now deleted NullabilityInfoContextSupport 😒

[MichalStrehovsky]

Size statistics

Pull request #109857

Project	Size before	Size after	Difference
avalonia.app-linux	22171288	22175384	4096
avalonia.app-windows	19116544	19118080	1536
hello-linux	1352352	1352352	0
hello-minimal-linux	1086008	1086008	0
hello-minimal-windows	858624	858624	0
hello-windows	1102848	1103360	512
kestrel-minimal-linux	5474256	5478352	4096
kestrel-minimal-windows	4908032	4909056	1024
reflection-linux	2063456	2063456	0
reflection-windows	1749504	1750016	512
webapiaot-linux	10120496	10124592	4096
webapiaot-windows	9156096	9158144	2048
winrt-component-full-windows	5600256	5601792	1536
winrt-component-minimal-windows	1746944	1747456	512

[hez2010]

I think it's still worth keeping the option in ilc as it can help diagnose binary size issues due to reflection usage by diffing two mstat files produced with the option on and off.
We can remove the msbuild propoerty.

[MichalStrehovsky]

I think it's still worth keeping the option in ilc as it can help diagnose binary size issues due to reflection usage by diffing two mstat files produced with the option on and off. We can remove the msbuild propoerty.

This is not deleting the --reflectiondata:none switch, just the stuff that was trying to cope with it at runtime.

[Sergio0694]

Just curious, how come there's some non-zero diffs? Shouldn't they all be zero given no project in rt-sz was setting the flag? 🤔

[MichalStrehovsky]

Just curious, how come there's some non-zero diffs? Shouldn't they all be zero given no project in rt-sz was setting the flag? 🤔

I expected it to be close to zero but not zero. Extra code in reachable method bodies always affects things.

[jkotas]

We can also delete ReflectionCoreCallbacks abstraction - move the implementation to ReflectionAugments and make the methods static (it is fine to do it as a follow up).

[MichalStrehovsky]

Sounds good!

[jkotas]

R.I.P.

[MichalStrehovsky]

[MichalStrehovsky]

/ba-g timeout in unrelated wasm leg

[Fabi]

The bad decisions add up currently...

[josephmoresena]

This just blew my mind in an incredible way. My libraries are full of things that I know don't make much sense at runtime with AOT and are better handled using IlcDisableReflection.

For example, right now, I’m adding functionality for dynamic stack allocations due to this discussion (#106653). In my test example, the AOT difference with or without reflection is 1MB. Similarly, when using JNetInterface, the cost of being able to dynamically create metadata for nested arrays is also 1MB, which I find excessive (at least it works without rd.xml directives).

I understand that may this is the cost of making NativeAOT general availability, but I don’t think removing functionality is the right approach. The right approach would be to democratize the documentation (Even I am guilty of not explaining the use of the switch in NativeAOT-AndroidHelloJniLib). I understand that it’s a feature that might seem cumbersome and brings many problems, but it’s actually a great alternative—not just for optimization when creating your libraries, but also when using them.

There’s currently nothing that allows me to optimize as much as IlcDisableReflection does. AOT also has a cost, and I think developers should be aware of this. I’ve had to do terrible things to make AOT compatible in some scenarios (even forcing myself to use reflection to avoid N+1 problems) and create optimal alternatives for NativeAOT, which are often better than their "reflective" counterparts and perfectly justify completely eliminating reflection.

[agocke]

The problem with IlcDisableReflection isn’t that it’s bad idea, it’s that it doesn’t work. The existing runtime and framework just don’t support running completely without a reflection stack. There are small bits that are required by core functionality.

If you want to explore turning this into a reality you should go to the source code. If this ever gets built it needs the bugs worked out, not production apps running on it. If there’s enough interest here and someone wants to push it forward, it seems reasonable to create a runtime-labs branch, like for our other experiments.

[josephmoresena]

The problem with IlcDisableReflection isn’t that it’s bad idea, it’s that it doesn’t work. The existing runtime and framework just don’t support running completely without a reflection stack. There are small bits that are required by core functionality.

If you want to explore turning this into a reality you should go to the source code. If this ever gets built it needs the bugs worked out, not production apps running on it. If there’s enough interest here and someone wants to push it forward, it seems reasonable to create a runtime-labs branch, like for our other experiments.

Well, I've never felt like it "doesn't work." It works as much as Windows 98 could work without being Windows NT. (Under that premise, NativeAOT wouldn't work either if compared to CoreCLR.)
What are the red lines to consider IlcDisableReflection useful? What are the red lines it crossed?
What is the "core functionality" you are referring to?

[hez2010]

What are the red lines to consider IlcDisableReflection useful? What are the red lines it crossed?
What is the "core functionality" you are referring to?

We didn't push any commit other than making a hello world run under IlcDisableReflection. Outsides a hello world program enabling IlcDisableReflection would introduce hard-to-observe subtle changes to the behavior of the program.

We are already trimming all unnecessary reflection metadata that won't be reached. If there're really no reflection usage, there should be no difference between IlcDisableReflection on and off.
If you see a size saving after enabling IlcDisableReflection, it means that there're already something broken internally and your program just happens to work somehow.

[agocke]

What are the red lines to consider IlcDisableReflection useful? What are the red lines it crossed?
What is the "core functionality" you are referring to?

It doesn't pass our tests even when you fix all AOT warnings. If you can get the full CoreCLR+libraries test suite to pass while only disabling things that produce AOT warnings, we can enable it.

[josephmoresena]

What are the red lines to consider IlcDisableReflection useful? What are the red lines it crossed?
What is the "core functionality" you are referring to?

We didn't push any commit other than making a hello world run under IlcDisableReflection. Outsides a hello world program enabling IlcDisableReflection would introduce hard-to-observe subtle changes to the behavior of the program.

We are already trimming all unnecessary reflection metadata that won't be reached. If there're really no reflection usage, there should be no difference between IlcDisableReflection on and off. If you see a size saving after enabling IlcDisableReflection, it means that there're already something broken internally and your program just happens to work somehow.

Wow, that's my day-to-day, in fact. Would you help me see it? In this case, it's just the test app for the new feature being introduced in PInvoke.Extensions:

Same functionality in both cases.

[Sergio0694]

Another option you could consider, which is what we're doing in CsWinRT, is to just sprinkle a bunch of checks for RuntimeFeature.IsDynamicCodeCompiled, and when that's false, you just assume you're on "some AOT equivalent environment", and skip/disable/throw on all dynamic paths in your library that you wanted to be trimmed out by the reflection-free mode. You could also introduce your very own feature switch and use it for that. This would allow your consumers to easily opt-in into that, and once the linker is done you should in theory get pretty close to what the reflection-free mode was doing before. We saw minimal size differences in the test projects we tried with this approach 🙂

[hez2010]

Same functionality in both cases.

By saying there're already something broken internally and your program just happens to work somehow I meant in ideal. The current dependency analysis in ilc is still a bit conservative on determining which to preserve and which to not preserve. The gap can be smaller or even disappearing in future updates.
For example, in your case there may be a code path that uses reflection to access some types or members, and the code path itself is being preserved instead of being trimmed because the dependency analysis thinks that there're something else calling into it. In this case your app may not break if the code path which uses reflection doesn't get actually called. Such cases can be addressed as the evolution of the dependency analysis in the future.

[josephmoresena]

Another option you could consider, which is what we're doing in CsWinRT, is to just sprinkle a bunch of checks for RuntimeFeature.IsDynamicCodeCompiled, and when that's false, you just assume you're on "some AOT equivalent environment", and skip/disable/throw on all dynamic paths in your library that you wanted to be trimmed out by the reflection-free mode. You could also introduce your very own feature switch and use it for that. This would allow your consumers to easily opt-in into that, and once the linker is done you should in theory get pretty close to what the reflection-free mode was doing before. We saw minimal size differences in the test projects we tried with this approach 🙂

Both NativeAOT and CLR return false. I don't know how to distinguish exactly if I'm running in AOT or CLR.
In fact the way I detected that I was using reflection-free mode is:

Boolean disableReflection = !typeof(String).ToString().Contains(nameof(String));

[josephmoresena]

Same functionality in both cases.

By saying there're already something broken internally and your program just happens to work somehow I meant in ideal. The current dependency analysis in ilc is still a bit conservative on determining which to preserve and which to not preserve. The gap can be smaller or even disappearing in future updates. For example, in your case there may be a code path that uses reflection to access some types or members, and the code path itself is being preserved instead of being trimmed because the dependency analysis thinks that there're something else calling into it. In this case your app may not break if the code path which uses reflection doesn't get actually called. Such cases can be addressed as the evolution of the dependency analysis in the future.

As I told you, it's my day-to-day. In many cases, to allow AOT flows, I have to use reflection to prevent the compiler from ending up in a self-referencing loop.

On the other hand, regarding the different paths, for now, I can only decide based on whether reflection is being used or not, not whether it's AOT or CLR (I don't know how to).

That said, I would be willing to accept the extra size that reflection represents for this feature if I were at least able to build what is required. But that's not the case, because, unlike the nested arrays in JNetInterface, this is based on structures and not reference types, will never work without use rd.xml directives.

[Sergio0694]

"Both NativeAOT and CLR return false. I don't know how to distinguish exactly if I'm running in AOT or CLR."

That's the neat part, you don't 👌

That's the whole point. You want consistent behavior on both, so that you can just easily debug on CoreCLR and be guaranteed the same functionality on Native AOT as well. The only important thing is whether you set "PublishAot" or not. If you do, then the app should behave the same way on both runtines. This is precisely what we do in CsWinRT too, for instance.

[MichalStrehovsky]

@josephmoresena could you link where you have a need to detect native AOT? I found this instance here:

https://github.com/josephmoresena/NativeAOT-AndroidHelloJniLib/blob/52db0390079136915ceeb5fe2edbd5ae18ec5fe2/ExportedMethods.cs#L213-L216

That's an example of having the need because the called code simply doesn't work. It merely has to work around the breakage caused by this switch.

Reflection disabled mode made sense 5 years ago when even a Hello World was still 4 MB in size and we needed a goalpost for how good it could get if we did proper factoring/implemented compiler features.

We're now at a point where difference in Hello World with/without reflection disabled mode is 1,098,752 bytes (without) vs 946,688 with reflection disabled. It reached a point where most of the size increase is justified (if you want to see why it's justified for a Hello world, simply run the hello world with stdout redirected to a full disk - we need to stringify exception name and print stack trace).

There can be instances where the difference in size is bigger. For example, if I change the Hello World line to Type.GetType("Program") the difference is 1,531,392 bytes without vs 1,160,192 with reflection disabled. The difference is more pronounced because the codepath doesn't work. It optimizes the program by breaking it.

The savings are not meaningful enough to justify having this anymore.