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

Remove MS.Extensions.Logging.Console's dependency on ConfigurationBinder #81931

Closed
Tracked by #78518
eerhardt opened this issue Feb 10, 2023 · 2 comments · Fixed by #82098
Closed
Tracked by #78518

Remove MS.Extensions.Logging.Console's dependency on ConfigurationBinder #81931

eerhardt opened this issue Feb 10, 2023 · 2 comments · Fixed by #82098
Assignees
Milestone

Comments

@eerhardt
Copy link
Member

In measuring the size of a NativeAOT published ASP.NET app, a considerable amount of size is added when using Console logging.

dotnet new api -aot and dotnet publish on win-x64: 13.2 MB. Removing the line builder.Logging.AddConsole(): 12.1 MB.

From my investigation, I've determined that ~860KB of size comes from using the ConfigurationBinder, which also pulls in Sytem.ComponentModel.TypeConverter. One reason this is so big is because a bunch of methods on ICollection/IList can be trimmed if we don't include ConfigurationBinder/TypeConverter. There are a lot of generic instantiations of collections (Dictionary, List, etc), and all these interface methods across scores of collection types add up. The methods in the System.Collections.Generic namespace drop by 343 KB alone.

For a full picture of all the code that can be trimmed here, diff the following mstat dumps:

no-configbinder-console.txt was created using eerhardt@af48374.
configbinder-console.txt was created from main

To eliminate this code from the application, we should remove the ConfigurationBinder usages in Logging.Console. They come from 2 places:

LoggerProviderOptions.RegisterProviderOptions<ConsoleLoggerOptions, ConsoleLoggerProvider>(builder.Services);

builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<TOptions>, ConsoleLoggerFormatterConfigureOptions<TFormatter, TOptions>>());

And instead we can hand-write the "configuration => options" deserialization code. This can also improve startup performance, since we won't be using Reflection here.

Another option would be to use #44493, but that isn't available yet. We can always hand-write the code for now, and use the source generator later once available.

@eerhardt eerhardt added this to the 8.0.0 milestone Feb 10, 2023
@eerhardt eerhardt self-assigned this Feb 10, 2023
@ghost
Copy link

ghost commented Feb 10, 2023

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

In measuring the size of a NativeAOT published ASP.NET app, a considerable amount of size is added when using Console logging.

dotnet new api -aot and dotnet publish on win-x64: 13.2 MB. Removing the line builder.Logging.AddConsole(): 12.1 MB.

From my investigation, I've determined that ~860KB of size comes from using the ConfigurationBinder, which also pulls in Sytem.ComponentModel.TypeConverter. One reason this is so big is because a bunch of methods on ICollection/IList can be trimmed if we don't include ConfigurationBinder/TypeConverter. There are a lot of generic instantiations of collections (Dictionary, List, etc), and all these interface methods across scores of collection types add up. The methods in the System.Collections.Generic namespace drop by 343 KB alone.

For a full picture of all the code that can be trimmed here, diff the following mstat dumps:

no-configbinder-console.txt was created using eerhardt@af48374.
configbinder-console.txt was created from main

To eliminate this code from the application, we should remove the ConfigurationBinder usages in Logging.Console. They come from 2 places:

LoggerProviderOptions.RegisterProviderOptions<ConsoleLoggerOptions, ConsoleLoggerProvider>(builder.Services);

builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<TOptions>, ConsoleLoggerFormatterConfigureOptions<TFormatter, TOptions>>());

And instead we can hand-write the "configuration => options" deserialization code. This can also improve startup performance, since we won't be using Reflection here.

Another option would be to use #44493, but that isn't available yet. We can always hand-write the code for now, and use the source generator later once available.

Author: eerhardt
Assignees: eerhardt
Labels:

area-Extensions-Logging

Milestone: 8.0.0

@tarekgh
Copy link
Member

tarekgh commented Feb 11, 2023

CC @layomia for awareness

eerhardt added a commit to eerhardt/runtime that referenced this issue Feb 14, 2023
This allows ConfigurationBinder, and its dependencies like TypeConverter,
to be trimmed in an application that uses Console Logging, like an
ASP.NET API application.

Fix dotnet#81931
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 14, 2023
eerhardt added a commit that referenced this issue Feb 16, 2023
* Remove ConfigurationBinder usage from Console Logging

This allows ConfigurationBinder, and its dependencies like TypeConverter,
to be trimmed in an application that uses Console Logging, like an
ASP.NET API application.

Fix #81931

* Ensure invalid configuration data throws appropriate exception.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 16, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 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