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

Reflection-free Native AOT and Enum.IsDefined #75487

Closed
exyi opened this issue Sep 12, 2022 · 23 comments
Closed

Reflection-free Native AOT and Enum.IsDefined #75487

exyi opened this issue Sep 12, 2022 · 23 comments

Comments

@exyi
Copy link

exyi commented Sep 12, 2022

Problem: Enum.IsDefined always returns false in NativeAot with IlcDisableReflection=true, which triggers guards at many places. For example, it's not possible to use Environment.GetFolderPath for this reason.

I understand the point that the metadata is not there, so this method can't be properly implemented. I'd just like to suggest changing the fallback value to true.

I'm attempting to use Native AOT for a simple library written in C# and that reason, I'd like to like to avoid shipping a huge library. I know there is #67193, but that's about actually adding the enum metadata - which is more complex, this could be simpler solution to the very specific problem.

@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 Sep 12, 2022
@exyi
Copy link
Author

exyi commented Sep 12, 2022

I don't have permissions, but label should IMO be area-NativeAOT-coreclr

@Fabi
Copy link

Fabi commented Sep 12, 2022

Problem: Enum.IsDefined always returns false in NativeAot with IlcDisableReflection=true, which triggers guards at many places. For example, it's not possible to use Environment.GetFolderPath for this reason.

I understand the point that the metadata is not there, so this method can't be properly implemented. I'd just like to suggest changing the fallback value to true.

I'm attempting to use Native AOT for a simple library written in C# and that reason, I'd like to like to avoid shipping a huge library. I know there is #67193, but that's about actually adding the enum metadata - which is more complex, this could be simpler solution to the very specific problem.

I suggest providing source generators for those cases. With them you can actually generate extensions for enums to handle Enum.Parse and IsDefined properly.

@exyi
Copy link
Author

exyi commented Sep 13, 2022

I don't think that would help - I'm not the one using Enum.IsDefined, a library which I'm trying to use does that.

@MichalStrehovsky
Copy link
Member

Duplicate of #67193.

@MichalStrehovsky MichalStrehovsky closed this as not planned Won't fix, can't repro, duplicate, stale Sep 14, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 14, 2022
@exyi
Copy link
Author

exyi commented Sep 14, 2022

I don't need full enum support, I just to get rid of the guards. Either to remove it from GetFolderPath or IsDefined to lie and always say true.

@MichalStrehovsky
Copy link
Member

I don't need full enum support, I just to get rid of the guards. Either to remove it from GetFolderPath or IsDefined to lie and always say true.

It's not possible to do it from files that are "hackable" (the System.Private.DisabledReflection assembly):

public override EnumInfo GetEnumInfo(Type type)
{
return new EnumInfo(
RuntimeAugments.GetEnumUnderlyingType(type.TypeHandle),
rawValues: Array.Empty<object>(),
names: Array.Empty<string>(),
isFlags: false);
}

We would need to return an array with 4 billion elements to make it possible for IsDefined to be always true.

Reflection disabled mode is not shipping and will probably never ship as a supported thing. The only product changes we do for reflection disabled mode are changes that can be justified without reflection disabled mode as well.

@exyi
Copy link
Author

exyi commented Sep 15, 2022

Ok, thanks, I understand. It's a shame, disabling reflection really saves me almost half of the binary size

@MichalStrehovsky
Copy link
Member

IlcDisableReflection is a tech demo that was introduced before we had things like IlcTrimMetadata=true (which is now the default after .NET 7 Preview 7). With metadata trimming, I don't believe you'll see meaningful size differences with/without reflection for apps that are bigger than a console hello world. IlcDisableReflection produces meaningful savings for hello-world-sized apps because it allows removing the whole reflection stack, but that's a fixed ~1MB cost that is not worth it for most. We'll likely do work that will indirectly make this cost smaller over time. The reflection stack in NativeAOT is a bit overengineered.

If you still see a meaningful difference for apps that are bigger than a hello world, please add <ItemGroup><IlcArg Include="--make-repro-path:c:\some\path\where\a\zip\can\be\created" /></ItemGroup> to the project and share the resulting ZIP with me (in a new issue) - it's likely a bug - IlcDisableReflection has been useful in finding those in the past.

@Fabi
Copy link

Fabi commented Sep 16, 2022

IlcDisableReflection is a tech demo that was introduced before we had things like IlcTrimMetadata=true (which is now the default after .NET 7 Preview 7). With metadata trimming, I don't believe you'll see meaningful size differences with/without reflection for apps that are bigger than a console hello world. IlcDisableReflection produces meaningful savings for hello-world-sized apps because it allows removing the whole reflection stack, but that's a fixed ~1MB cost that is not worth it for most. We'll likely do work that will indirectly make this cost smaller over time. The reflection stack in NativeAOT is a bit overengineered.

If you still see a meaningful difference for apps that are bigger than a hello world, please add <ItemGroup><IlcArg Include="--make-repro-path:c:\some\path\where\a\zip\can\be\created" /></ItemGroup> to the project and share the resulting ZIP with me (in a new issue) - it's likely a bug - IlcDisableReflection has been useful in finding those in the past.

I think you a very wrong there. On apps that would be like 6-7mb with all optimizations on in my case i save another 3+mb with reflection free mode. And that is only because Enum.Parse is required for me (I can solve that part with source generators tho)
Reflection free mode can save even more for bigger apps. It’s great

@neon-sunset
Copy link
Contributor

neon-sunset commented Sep 16, 2022

I think you a very wrong there. On apps that would be like 6-7mb with all optimizations on in my case i save another 3+mb with reflection free mode. And that is only because Enum.Parse is required for me (I can solve that part with source generators tho) Reflection free mode can save even more for bigger apps. It’s great

Use UPX, it compresses the binary down to 3MB size on hello world apps for win-x64 RID. The smallest size, as far as my understanding goes, is a non-goal for NativeAOT and reasonable compromises are taken to balance it with overall available functionality.

@Fabi
Copy link

Fabi commented Sep 16, 2022

I think you a very wrong there. On apps that would be like 6-7mb with all optimizations on in my case i save another 3+mb with reflection free mode. And that is only because Enum.Parse is required for me (I can solve that part with source generators tho) Reflection free mode can save even more for bigger apps. It’s great

Use UPX if you need small binary, it achieves 3MB size on hello world apps for win-x64 RID. The smallest binary size, as far as my understanding goes, is a non-goal for NativeAOT and reasonable compromises are taken to balance functionality and binary size.

Hello world apps are already under 3mb anyways. upx and other compression tools are no option due to them being basically auto detected by any anti virus vendor (native art binaries itself are already very often suspicious for many generic detection algos)

@MichalStrehovsky
Copy link
Member

I think you a very wrong there. On apps that would be like 6-7mb with all optimizations on in my case i save another 3+mb with reflection free mode

What settings are in your csproj? Do you have:

  • <UseSystemResourceKeys>true</UseSystemResourceKeys>
  • <EventSourceSupport>false</EventSourceSupport>
  • (<IlcScanReflection>false</IlcScanReflection> - this one is not documented and if it makes meaningful difference in size, I want to know)

IlcDisableReflection adds the above implicitly because those features don't work at all with reflection disabled. But they can be disabled individually using fully supported and documented switches and are not tied to reflection disabled mode.

@neon-sunset
Copy link
Contributor

Hello world apps are already under 3mb anyways. upx and other compression tools are no option due to them being basically auto detected by any anti virus vendor (native art binaries itself are already very often suspicious for many generic detection algos)

UPX writes its tags and Windows Defender seems to be happy with my binaries. In advanced scenarios as such, you can also look separately into binary signing to stop other solutions from complaining. Personally, I can't imagine a scenario where 3MB vs 6, 10 or even 20MB would be a problem for Windows hosts in 2022.

@Fabi
Copy link

Fabi commented Sep 16, 2022

I think you a very wrong there. On apps that would be like 6-7mb with all optimizations on in my case i save another 3+mb with reflection free mode

What settings are in your csproj? Do you have:

* `<UseSystemResourceKeys>true</UseSystemResourceKeys>`

* `<EventSourceSupport>false</EventSourceSupport>`

* (`<IlcScanReflection>false</IlcScanReflection>` - this one is not documented and if it makes meaningful difference in size, I want to know)

IlcDisableReflection adds the above implicitly because those features don't work at all with reflection disabled. But they can be disabled individually using fully supported and documented switches and are not tied to reflection disabled mode.

This is a snippet of my project file from a project where I have the smallest benefit of reflection disabled which is at 2.1MB (On somee other I have 3-4MB)

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
    <OutputPath>..\..\..\build\Release\launcher</OutputPath>
    <RootAllApplicationAssemblies>false</RootAllApplicationAssemblies>
    <IlcGenerateCompleteTypeMetadata>false</IlcGenerateCompleteTypeMetadata>
    <IlcGenerateStackTraceData>false</IlcGenerateStackTraceData>
    <IlcDisableReflection>false</IlcDisableReflection>
    <RootAllApplicationAssemblies>false</RootAllApplicationAssemblies>
    <IlcOptimizationPreference>Speed</IlcOptimizationPreference>
    <GenerateRuntimeConfigurationFiles>false</GenerateRuntimeConfigurationFiles>
    <IlcFoldIdenticalMethodBodies>true</IlcFoldIdenticalMethodBodies>
    <IlcTrimMetadata>true</IlcTrimMetadata>
    <IlcInvariantGlobalization>false</IlcInvariantGlobalization>
    <IlcGenerateStackTraceData>false</IlcGenerateStackTraceData>
    <IlcGenerateCompleteTypeMetadata>false</IlcGenerateCompleteTypeMetadata>
    <DebugType>none</DebugType>
    <DebugSymbols>false</DebugSymbols>
    <FileAlignment>1024</FileAlignment>
</PropertyGroup>
<Import Project="..\Arctium.WoW.Launcher.Core\Arctium.WoW.Launcher.Core.projitems" Label="Shared" />
<ItemGroup>
    <PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
</ItemGroup>
<ItemGroup>
    <TrimmerRootAssembly Include="mscorlib" />
    <TrimmerRootAssembly Include="System.Runtime" />
    <TrimmerRootAssembly Include="System.Diagnostics.FileVersionInfo" />
    <TrimmerRootAssembly Include="System.Threading.Thread" />
    <TrimmerRootAssembly Include="System.Security.Cryptography.Csp" />
    <TrimmerRootAssembly Include="Microsoft.Win32.Registry" />
</ItemGroup>

@Fabi
Copy link

Fabi commented Sep 16, 2022

Hello world apps are already under 3mb anyways. upx and other compression tools are no option due to them being basically auto detected by any anti virus vendor (native art binaries itself are already very often suspicious for many generic detection algos)

UPX writes its tags and Windows Defender seems to be happy with my binaries. In advanced scenarios as such, you can also look separately into binary signing to stop other solutions from complaining. Personally, I can't imagine a scenario where 3MB vs 6, 10 or even 20MB would be a problem for Windows hosts in 2022.

Every MB you can save is worth it. You guys have to go away from the "we are in 2022, we don't have to care about those things philosophy. That wont end well..."

@MichalStrehovsky
Copy link
Member

Add the <UseSystemResourceKeys> and <EventSourceSupport> to get a MB or so down without disabling reflection.

And delete the TrimmerRootAssembly entries - I don't know if it does anything with reflection disabled, but with reflection enabled it probably roots some junk.

@Fabi
Copy link

Fabi commented Sep 16, 2022

TrimmerRootAssembly

Will try to remove those. A few months ago these were required otherwise native aot would strip too many things

@MichalStrehovsky
Copy link
Member

TrimmerRootAssembly

Will try to remove those. A few months ago these were required otherwise native aot would strip too many things

I thought this is for an app that works in reflection disabled mode. Comparing the size of an app with reflection disabled that doesn't work at all to size of the app with reflection enabled is apples and oranges. There can be huge chunks of the app that got removed only because reflection got disabled. The size number is not meaningful because app logic got deleted.

@Fabi
Copy link

Fabi commented Sep 16, 2022

Add the <UseSystemResourceKeys> and <EventSourceSupport> to get a MB or so down without disabling reflection.

And delete the TrimmerRootAssembly entries - I don't know if it does anything with reflection disabled, but with reflection enabled it probably roots some junk.

Yea you are right. In that project I use those options give me about 1MB. Might be some start. I still hope that reflection free mode will evolve in future. That saves another 1.1MB on that project which brings my whole app down to 2.3MB instead of 4.6MB which is nice to have. It's just that the only reason to have reflection in many projects might just be things like Enum.Parse (for example in the ,net command line project) which could be solved overall in native aot with integrated source generators or so. Recently many Marshal structure conversations were replaced with custom marshaling which also made so many things reflection free compatible too.

As reference it was a version of this project https://github.com/Arctium/WoW-Launcher but not exactly the same. I have multiple versions of those with different features and different size optimizations by native aot

@exyi
Copy link
Author

exyi commented Sep 16, 2022

If you still see a meaningful difference for apps that are bigger than a hello world, please add to the project and share the resulting ZIP with me (in a new issue) - it's likely a bug - IlcDisableReflection has been useful in finding those in the past.

Ok, now I think I'm doing something wrong, even when I remove all entry points (method with UnmanagedCallersOnly), the binary size (the .a file before linking) is around 14MB. I will not create a new issue until I find out which part / library is causing the massive increase

Anyway, disabling reflection saves me 11MB, from 36M to 25M.

2497942273_AnalyzerLib.zip

my current config is

  <PropertyGroup>
    <PublishAot>true</PublishAot>
    <TrimMode>link</TrimMode>
    <!-- <IlcDisableReflection>true</IlcDisableReflection> -->
    <!-- this did nothing <IlcTrimMetadata>true</IlcTrimMetadata> -->
    <NativeLib>Static</NativeLib>
    <InvariantGlobalization>true</InvariantGlobalization>
    <!-- did nothing significant <UseSystemResourceKeys>false</UseSystemResourceKeys>
    <!-- did nothing significant <EventSourceSupport>false</EventSourceSupport> -->
  </PropertyGroup>

  <PropertyGroup Condition="$(Configuration) == 'Release'">
    <StripSymbols>true</StripSymbols>
    <IlcOptimizationPreference>Size</IlcOptimizationPreference>
  </PropertyGroup>

@MichalStrehovsky
Copy link
Member

The size of .a files is not very meaningful. It includes a lot of symbol names that are going to be removed after linking. The static overhead of the reflection stack is going to be a lot more than 1 MB (I don't know the number, it's not particularly relevant).

NativeLib=Shared would give a more meaningful size estimate for the finished product - the size difference is more relevant there. (Sorry, I don't want to set up a Linux environment just to collect these numbers.)

@exyi
Copy link
Author

exyi commented Sep 19, 2022

Ok, that's right, thanks. After linking the difference is "only" 5.85 MB (23 -> 18)

@ghost ghost locked as resolved and limited conversation to collaborators Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants