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

[Performance improvement] Minimize number of properties placed in the runtimeconfig.json file #112956

Open
grendello opened this issue Feb 26, 2025 · 6 comments
Assignees
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners untriaged New issue has not been triaged by the area owner

Comments

@grendello
Copy link
Contributor

When building the default dotnet new android runtime application, we can see that the following runtimeconfig.json file is generated:

{
  "runtimeOptions": {
    "tfm": "net10.0",
    "frameworks": [
      {
        "name": "Microsoft.NETCore.App",
        "version": "10.0.0-preview.2.25119.7"
      },
      {
        "name": "Microsoft.Android",
        "version": "**FromWorkload**"
      }
    ],
    "configProperties": {
      "System.Diagnostics.Metrics.Meter.IsSupported": false,
      "Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability": false,
      "System.ComponentModel.DefaultValueAttribute.IsSupported": true,
      "System.ComponentModel.Design.IDesignerHost.IsSupported": false,
      "System.ComponentModel.TypeConverter.EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization": false,
      "System.ComponentModel.TypeDescriptor.IsComObjectDescriptorSupported": false,
      "System.Data.DataSet.XmlSerializationIsSupported": false,
      "System.Diagnostics.Debugger.IsSupported": false,
      "System.Diagnostics.Tracing.EventSource.IsSupported": false,
      "System.Globalization.Invariant": false,
      "System.Linq.Enumerable.IsSizeOptimized": true,
      "System.Net.Http.EnableActivityPropagation": false,
      "System.Net.Http.UseNativeHttpHandler": true,
      "System.Reflection.Metadata.MetadataUpdater.IsSupported": false,
      "System.Resources.ResourceManager.AllowCustomResourceTypes": false,
      "System.Resources.UseSystemResourceKeys": true,
      "System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported": true,
      "System.Runtime.InteropServices.BuiltInComInterop.IsSupported": false,
      "System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting": false,
      "System.Runtime.InteropServices.EnableCppCLIHostActivation": false,
      "System.Runtime.InteropServices.Marshalling.EnableGeneratedComInterfaceComImportInterop": false,
      "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false,
      "System.StartupHookProvider.IsSupported": false,
      "System.Text.Encoding.EnableUnsafeUTF7Encoding": false,
      "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault": true,
      "System.Threading.Thread.EnableAutoreleasePool": false,
      "Xamarin.Android.Net.UseNegotiateAuthentication": false,
      "Switch.System.Reflection.ForceInterpretedInvoke": true,
      "Microsoft.Extensions.DependencyInjection.DisableDynamicEngine": true
    }
  }
}

After a discussion on Discord, it appears that most of those options aren't needed and might not be checked at run time, because the relevant code could have been trimmed away by the trimmer.

Each of those options is passed to CoreCLR runtime at initialization and each of them (both the name and the value) needs to be converted from UTF-8 to Unicode, which means memory allocation and the actual conversion. If a property isn't used, it means the time processing the property and memory allocated for it are both wasted.

It would be very good if we were able to omit unnecessary properties/switches from the JSON file. This could be done in two ways, it appears:

  1. By adding support to the trimmer to annotate in msbuild those properties it trimmed out, so we can skip them when building the application.
  2. By adding a way to learn what is the default value of each switch/property, so that we can go over the list and decide which options are set to their defaults, to skip them when building the application.
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 26, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Feb 26, 2025
@MichalStrehovsky
Copy link
Member

Dropping these from runtimeconfig.json would be an observable change (these will no longer show up with AppContext.TryGetSwitch after trimming at runtime). Whenever trimming causes an observable change it needs to generate a compile-time warning. Just dropping things from AppContext without warnings would be illegal optimization.

One could argue that these text strings are our internal implementation details and nobody should be querying for them. I tried to make such argument for a different but related optimization in #95948 (comment), and that basically set a precedent that we don't consider external users using these specific strings as undefined behavior.

What's the purpose of runtimeconfig.json on mobile in general? Could this just be embedded into the host executable in the expected format on mobile? That way we don't even need to parse a json or convert strings.

@grendello
Copy link
Contributor Author

@MichalStrehovsky we embed the data on mobile and pass the strings to CoreCLR on initialization, and that's the actual issue - there are ~30 entries, which means 60 strings that need to be converted from UTF8 to Unicode and parsed (for values), and the conversion (with memory allocation, copying etc) costs us around 15% of the initialization time. Purpose of runtimeconfig.json is identical to desktop/server.

With regards to trimmer - the way I understood what was said in the linked to discussion on Discord, trimmer would remove the settings only if the relevant code was actually linked out, thus the settings would become irrelevant, since code that accesses/reads/uses them is no longer there. This sounds like a reasonable approach to me.

@MichalStrehovsky
Copy link
Member

With regards to trimmer - the way I understood what was said in the linked to discussion on Discord, trimmer would remove the settings only if the relevant code was actually linked out, thus the settings would become irrelevant, since code that accesses/reads/uses them is no longer there. This sounds like a reasonable approach to me.

Trimming analysis would need to figure out that all calls to AppContext.TryGetSwitch (and GetData and related) are accounted for and it's not possible for someone to call it with the string that is being removed.

In the limit if I write following code:

Console.WriteLine(AppContext.TryGetSwitch(Console.ReadLine(), out _);

This code needs to behave the same way before/after trimming irrespective of what string I type on the keyboard (I can e.g. type System.Diagnostics.Metrics.Meter.IsSupported). So if the analysis sees the API is called with an unknown string, the optimization cannot be performed and everything needs to be left as-is.

We'd need to build a lot of new analysis capabilities to be able to do this. For example, it's common to wrap this API and the string becomes difficult to find (we don't have capabilities to do this right now):

internal static bool GetSwitchValue(string switchName, ref bool switchValue) =>
AppContext.TryGetSwitch(switchName, out switchValue);

and the conversion (with memory allocation, copying etc) costs us around 15% of the initialization time

How is "initialization time" defined for this? From the C main in the VM to executing the first instruction of the C# Main?

@grendello
Copy link
Contributor Author

@MichalStrehovsky there's no main in Android applications, the init time is the call to coreclr_initialize itself (we have other measurements, but the 15% applies to this particular single call)

@grendello
Copy link
Contributor Author

@MichalStrehovsky I optimized the process at some point by pre-converting all the strings to Unicode and modifying coreclr_initialize to accept a pointer to a single array of key/value structures. This improved things a lot, since the init code would only create a single array and copy pointers, no encoding transformation. That worked fine but, alas, wasn't approved for inclusion yet. Even if it was, however, I would still love to be able to limit the number of properties passed to the runtime - find a way to discover which are set to their default values, for instance (in which case they can be safely removed from the set)

@grendello
Copy link
Contributor Author

Mono way of dealing with runtimeconfig.json is described in this document. I'm not sure if we really need it for CoreCLR, but it might be a food for thought.

The main motivation behind filing this issue here is to seek ways to optimize processing of those properties.

For their names, processing entails encoding from UTF-8 to Unicode - Android can do it at application build time, thus eliminating the overhead completely. It requires modifications to the CoreCLR API (which was part of this PR but were rejected, for the moment) to accept Unicode (char16) strings instead.

For values, it would mean eliminating conversion from UTF-8 to Unicode for strings and accepting actual values for other types so that no parsing occurs. Set of value types could be limited to the basic signed integer, a boolean and, maybe, a basic floating point number - anything else would be passed as string.

Even if all the above changes were implemented, elimination of properties set to their default values would still be very welcome - in the spirit of minimizing the amount of dynamic processing at run time, especially for something that doesn't need to be there (since it's set to the default value).

Please treat this as a very high level overview, a wish list if you will :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants