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

Bringing down the size of Hello World under 2 MB #80165

Closed
5 of 6 tasks
MichalStrehovsky opened this issue Jan 4, 2023 · 6 comments · Fixed by #81517
Closed
5 of 6 tasks

Bringing down the size of Hello World under 2 MB #80165

MichalStrehovsky opened this issue Jan 4, 2023 · 6 comments · Fixed by #81517

Comments

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Jan 4, 2023

Hello World with Native AOT is currently at around 2.7 MB. I spent a couple hours to see what would it take to bring it down to the same size as Golang (2 MB). It might actually be achievable with our default trimming settings and feature switches with a couple days of BCL/compiler work.

The benefits would be threefold:

  • Our internal efficiency (compiling non-xunit-based tests will get faster and require less disk space, which is not insignificant as long as we have thousands of tiny tests)
  • We can delete reflection disabled mode
  • Bragging rights

The 6686dbc commit has a couple hacks in it around areas of interest. With those hacks, the size of a hello world goes down to 2.16 MB. With InvariantGlobalization and UseSystemResourceKeys it goes further down to 1.53 MB. This is awfully close to what is achievable with reflection disabled mode. We actually regressed the reflection disabled mode because a Hello World now has a generic virtual method call around enums and we need the entirety of the type loader to support that. But I digress.

Should we choose to accept this challenge, we'll need to fix all of the issues in the commit to really reap the benefit. The benefit is elimination of the entire reflection stack that deals with type members (in fact my goal was to get MemberPolicies class out of the hello world).

Here's the problems:

  • Stack frame should not eagerly try to populate the MethodBase, only once it's requested.
  • Type components cache should not try to eagerly populate the members. We use this class to stash details about enums in the GenericCache property. Populating anything else is a waste.
  • Type.IsEnum and IsValueType. We expand these as intrinsics in codegen for RuntimeTypes. Hello world only needs them for RuntimeTypes. IL scanner might need to learn the RyuJITs trick. (This is really only a problem because IL scanner runs. Reduce the number of forced MethodTables #79594 might already help with this to some extent but I didn't check. Would be nice if it did.)
  • Searching for DefaultDllImportSearchPathsAttribute is avoidable if this codepath is only reached from p/invoke resolution.
  • Attribute.Equals and GetHashCode. This is a tough one. .NET Native didn't use a reflection-based implementation. We could restore that. Other alternative is to see if we can eliminate all custom attribute descendants from a hello world. The theory is that if we make generation of custom attributes conditional on the presence of the code to read them, we might be able to get rid of them all. But not sure. Chose not to address this and live with a small increase in size due to FieldInfo reflection. That one is less impactful than Method/ConstructorInfo.
  • Resource manager looking for attributes. I don't have ideas right now but I also didn't look at it in detail.

Cc @dotnet/ilc-contrib

@ghost
Copy link

ghost commented Jan 4, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Hello World with Native AOT is currently at around 2.7 MB. I spent a couple hours to see what would it take to bring it down to the same size as Golang (2 MB). It might actually be achievable with our default trimming settings and feature switches with a couple days of BCL/compiler work.

The benefits would be threefold:

  • Our internal efficiency (compiling non-xunit-based tests will get faster and require less disk space, which is not insignificant as long as we have thousands of tiny tests)
  • We can delete reflection disabled mode
  • Bragging rights

The 6686dbc commit has a couple hacks in it around areas of interest. With those hacks, the size of a hello world goes down to 2.16 MB. With InvariantGlobalization and UseSystemResourceKeys it goes further down to 1.53 MB. This is awfully close to what is achievable with reflection disabled mode. We actually regressed the reflection disabled mode because a Hello World now has a generic virtual method call around enums and we need the entirety of the type loader to support that. But I digress.

Should we choose to accept this challenge, we'll need to fix all of the issues in the commit to really reap the benefit. The benefit is elimination of the entire reflection stack that deals with type members (in fact my goal was to get MemberPolicies class out of the hello world).

Here's the problems:

  • Stack frame should not eagerly try to populate the MethodBase, only once it's requested.
  • Type components cache should not try to eagerly populate the members. We use this class to stash details about enums in the GenericCache property. Populating anything else is a waste.
  • Type.IsEnum and IsValueType. We expand these as intrinsics in codegen for RuntimeTypes. Hello world only needs them for RuntimeTypes. IL scanner might need to learn the RyuJITs trick. (This is really only a problem because IL scanner runs. Reduce the number of forced MethodTables #79594 might already help with this to some extent but I didn't check. Would be nice if it did.)
  • Searching for DefaultDllImportSearchPathsAttribute is avoidable if this codepath is only reached from p/invoke resolution.
  • Attribute.Equals and GetHashCode. This is a tough one. .NET Native didn't use a reflection-based implementation. We could restore that. Other alternative is to see if we can eliminate all custom attribute descendants from a hello world. The theory is that if we make generation of custom attributes conditional on the presence of the code to read them, we might be able to get rid of them all. But not sure.
  • Resource manager looking for attributes. I don't have ideas right now but I also didn't look at it in detail.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 4, 2023
@MichalStrehovsky
Copy link
Member Author

Other alternative is to see if we can eliminate all custom attribute descendants from a hello world. The theory is that if we make generation of custom attributes conditional on the presence of the code to read them, we might be able to get rid of them all. But not sure.

I did a quick hack and indeed if we don't generate any custom attributes, there will be no constructed System.Attribute descendant in the closure. It also produces another 20 kB of savings.

@MichalStrehovsky
Copy link
Member Author

After #80607 we should no longer need the hacks around Type.IsValueType.

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jan 16, 2023
Contributes to dotnet#80165.

Unfortunately, the `NativeLibrary` APIs contain a pattern where one can skip providing a parameter to the API and then something expensive (custom attribute reading) will happen to compute the value. We have a `TryLoad` call in a hello world that does provide the value (in GlobalizationMode.cs). Make it possible to avoid the expensive thing internally.
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jan 16, 2023
Contributes to dotnet#80165.

Resolving a `MethodBase` is a pretty expensive operation and currently even a hello world needs it because of `Exception.ToString()`. This pull request is trying to get `MethodBase` out of the picture when constructing the exception string.

We currently only need `_method` for two things - the public `GetMethod` API, and `StackFrame.ToString`. The `StackFrame.ToString` can already deal with `_method` being null (and that codepath exists solely for NativeAOT). This is the minimal change. We could potentially explore other approaches - leave `MethodBase` always at null, or unshare the `StackFrame` class.
MichalStrehovsky added a commit that referenced this issue Jan 16, 2023
Contributes to #80165.

Unfortunately, the `NativeLibrary` APIs contain a pattern where one can skip providing a parameter to the API and then something expensive (custom attribute reading) will happen to compute the value. We have a `TryLoad` call in a hello world that does provide the value (in GlobalizationMode.cs). Make it possible to avoid the expensive thing internally.
MichalStrehovsky added a commit that referenced this issue Jan 17, 2023
Contributes to #80165.

Resolving a `MethodBase` is a pretty expensive operation and currently even a hello world needs it because of `Exception.ToString()`. This pull request is trying to get `MethodBase` out of the picture when constructing the exception string.

We currently only need `_method` for two things - the public `GetMethod` API, and `StackFrame.ToString`. The `StackFrame.ToString` can already deal with `_method` being null (and that codepath exists solely for NativeAOT). This is the minimal change. We could potentially explore other approaches - leave `MethodBase` always at null, or unshare the `StackFrame` class.
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jan 17, 2023
Contributes to dotnet#80165.

The compiler is not able to get rid of the call to `PerNameQueryCache<M>.Factory` virtual method due to generic code sharing. Making this not shareable allows trimming it because now we can see `ConcurrentUnifier<StringKey, __Canon>.Factory` is never called (whereas `ConcurrentUnifier<__Canon, __Canon>.Factory` is called).

This is a ~5 kB regression for apps that do use reflection.

We could explore different strategies to make this trimmable and not be a size regression, but I'm not sure if it's really worth it. We'd like to get rid of this reflection stack implementation anyway and replace it with something more lightweight.
MichalStrehovsky added a commit that referenced this issue Jan 19, 2023
Contributes to #80165.

Dispensing of reflection member infos is done through a member policies class. This is a generic class that has specialized implementations for each kind of member info.

It used a clever trick to get to the specific implementations. Just access `MemberPolicies<EventInfo>.Default` to get one for events or `MemberPolicies<PropertyInfo>.Default` to get one for properties. It was also absolutely not trimming friendly.

This change removes the clever trick and replaces it with good old fashioned parameter passing.
MichalStrehovsky added a commit that referenced this issue Jan 19, 2023
Contributes to #80165.

Not aware of a good reason to keep using it. Removes `TypeDelegator` from apps that don't use it. Saves 7 kB (but also stepping stone for the larger savings later).
@MichalStrehovsky
Copy link
Member Author

Once #80842 and #80843 merge, the only remaining spots will be the spot in Attribute.GetHashCode/Equals and the resource manager ones. Once those are addressed, Hello World will be at 2.1 MB, or 1.45 MB with /p:UseSystemResourceKeys=true /p:InvariantGlobalization=true.

mdh1418 pushed a commit to mdh1418/runtime that referenced this issue Jan 24, 2023
Contributes to dotnet#80165.

Dispensing of reflection member infos is done through a member policies class. This is a generic class that has specialized implementations for each kind of member info.

It used a clever trick to get to the specific implementations. Just access `MemberPolicies<EventInfo>.Default` to get one for events or `MemberPolicies<PropertyInfo>.Default` to get one for properties. It was also absolutely not trimming friendly.

This change removes the clever trick and replaces it with good old fashioned parameter passing.
mdh1418 pushed a commit to mdh1418/runtime that referenced this issue Jan 24, 2023
Contributes to dotnet#80165.

Not aware of a good reason to keep using it. Removes `TypeDelegator` from apps that don't use it. Saves 7 kB (but also stepping stone for the larger savings later).
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2023
@charlesroddie
Copy link

charlesroddie commented Feb 3, 2023

Please don't delete reflection-disabled mode.

  1. Sub 2MB is not enough. NativeAOT produces 1MB binaries in reflection-disabled mode in the articles I have seen. The difference is important for apps and critical for WASM deployment to websites.

  2. Since reflection breaks type-safety, reflection-disabled mode helps to guarantee type safety in a codebase.

  3. Reflection is slow and reflection-disabled mode guarantees that this slow code is not present in deployed software.

  4. Reflection-disabled mode creates incentives for libraries to improve code hygiene to support it.

@MichalStrehovsky
Copy link
Member Author

NativeAOT produces 1MB binaries in reflection-disabled mode in the articles I have seen.

For a hello world, the difference is 1.77 MB by default, and 1.47 MB with reflection disabled mode (coming to a .NET 8 Preview 1 near you soon). It can be squashed down to less with the options to optimize codegen for size, disable stack trace data, globalization invariant mode, etc, but those options equally apply to both modes. It really is not that much. Those 300 kB fix issues people have been running into like Enum.ToString only printing numbers in reflection disabled mode.

There's not much incentive to enable it. I don't think it's going to pressure any libraries into supporting it - it breaks too much stuff. For the argument of perf - there's no law against slow software. We don't offer compiler switches to break other slow code either. There can be a lot of non-reflection slow code in the NuGets.

Not planning to fully delete it, but I think it's time for people to think about letting that mode go. I've made my peace even though it's my baby.

@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Feb 7, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 9, 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.

2 participants