Skip to content

Add switch for trim out ConfigurationManager support - #83023

Closed
kant2002 wants to merge 4 commits into
dotnet:mainfrom
kant2002:kant/trim-configmanager
Closed

Add switch for trim out ConfigurationManager support#83023
kant2002 wants to merge 4 commits into
dotnet:mainfrom
kant2002:kant/trim-configmanager

Conversation

@kant2002

Copy link
Copy Markdown
Contributor

With this change you can shave 900Kb out of 25.8Mb from sample SqlClient app from here dotnet/SqlClient#1941 (comment)

Other example is that usage of System.Runtime.Caching.MemoryCache as shown in https://learn.microsoft.com/en-us/dotnet/api/system.runtime.caching.memorycache

Applying configuration switch reduce app size by 5.2Mb (from 7.8Mb to 2.6Mb)

Most likely modern applications which are first to use NativeAOT would not need this functionality because they use other means for configuration, so would be great to have this disabled.

Low drop in size in SqlClient coming from fact that there other large dependencies which keep Xml related code around.

Discovered as part of dotnet/SqlClient#1942

With this change you can shave 900Kb out of 25.8Mb from sample SqlClient app from here dotnet/SqlClient#1941 (comment)
Other example is that usage of `System.Runtime.Caching.MemoryCache` as shown in https://learn.microsoft.com/en-us/dotnet/api/system.runtime.caching.memorycache
Applying configuration switch reduce app size by 5.2Mb (from 7.8Mb to 2.6Mb)
Most likely modern applications which are first to use NativeAOT would not need this functionality because they use other means for configuration, so would be great to have this disabled.
Low drop in size in SqlClient coming from fact that there other large dependencies which keep Xml related code around.
@ghostghost added area-System.Configuration community-contribution Indicates that the PR has been added by a community member labels Mar 6, 2023
@ghost

ghost commented Mar 6, 2023

Copy link
Copy Markdown

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

Issue Details

With this change you can shave 900Kb out of 25.8Mb from sample SqlClient app from here dotnet/SqlClient#1941 (comment)

Other example is that usage of System.Runtime.Caching.MemoryCache as shown in https://learn.microsoft.com/en-us/dotnet/api/system.runtime.caching.memorycache

Applying configuration switch reduce app size by 5.2Mb (from 7.8Mb to 2.6Mb)

Most likely modern applications which are first to use NativeAOT would not need this functionality because they use other means for configuration, so would be great to have this disabled.

Low drop in size in SqlClient coming from fact that there other large dependencies which keep Xml related code around.

Discovered as part of dotnet/SqlClient#1942

Author:kant2002
Assignees:-
Labels:

area-System.Configuration, community-contribution

Milestone:-

@ghost

ghost commented Mar 6, 2023

Copy link
Copy Markdown

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

With this change you can shave 900Kb out of 25.8Mb from sample SqlClient app from here dotnet/SqlClient#1941 (comment)

Other example is that usage of System.Runtime.Caching.MemoryCache as shown in https://learn.microsoft.com/en-us/dotnet/api/system.runtime.caching.memorycache

Applying configuration switch reduce app size by 5.2Mb (from 7.8Mb to 2.6Mb)

Most likely modern applications which are first to use NativeAOT would not need this functionality because they use other means for configuration, so would be great to have this disabled.

Low drop in size in SqlClient coming from fact that there other large dependencies which keep Xml related code around.

Discovered as part of dotnet/SqlClient#1942

Author:kant2002
Assignees:-
Labels:

area-System.Configuration, linkable-framework, community-contribution

Milestone:-

@eerhardt

Copy link
Copy Markdown
Member

I wonder if it would be better if we changed these "up stack" libraries to no longer use ConfigurationManager. Cutting it off inside ConfigurationManager is probably going to lead to "random" runtime errors when some untested path calls into ConfigurationManager, and it throws.

@kant2002

Copy link
Copy Markdown
ContributorAuthor

Oops! I definitely go overboard with some changes. My idea was to always return null when call GetSection() pretending that section does not exists. I start adding other changes for consistency and break things.

GetSection() returning null should be safe change. You already can have situation when you config is missing, and usually that mean that application use defaults. OpenXXXConfiguration throwing is debatable, but I put them only for consistency. Really can ignore these changes.

Regarding your question. In specific case of SqlClient the libraries which require ConfigurationManager.GetSection() is at least System.Runtime.Caching and some internal to SqlClient stuff. I think that I can do changes there, but how to keep them coordinated? That's how I end up with this change.
I do not see that making System.Runtime.Caching to not depend on ConfgurationManager possible (breaking change), and do not see how MemoryCache can be replaced in SqlClient into other alternatives given that they support netfx (also breaking change).

@eerhardteerhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider removing dependencies upstack on ConfigurationManager. It is a legacy component that we haven't been making investments in. See https://github.com/dotnet/runtime/blob/main/src/libraries/System.Configuration.ConfigurationManager/README.md

This library exists only to support migrating existing .NET Framework code that uses System.Configuration.

<PropertyGroup>
<IsPartialFacadeAssembly Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETFramework'">true</IsPartialFacadeAssembly>
<!-- ILLinker settings -->
<ILLinkDirectory>$(MSBuildThisFileDirectory)ILLink\</ILLinkDirectory>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need this. It is already set in

<ILLinkDirectoryCondition="'$(ILLinkDirectory)' == ''">$(MSBuildProjectDirectory)\ILLink\</ILLinkDirectory>

…stem/Configuration/ConfigurationManager.cs
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@kant2002

Copy link
Copy Markdown
ContributorAuthor

From here https://learn.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-5.0#systemruntimecachingmemorycache-1

Use System.Runtime.Caching/MemoryCache as a compatibility bridge when porting code from ASP.NET 4.x to ASP.NET Core.

I would like to remove ConfigurationManager from that package, but seems to be that can break migration efforts to ASP.NET Core.

@kant2002kant2002 mentioned this pull request Mar 6, 2023
@roji

roji commented Mar 7, 2023

Copy link
Copy Markdown
Member

@kant2002 see my comment in the SqlClient repo. tl;dr I think SqlClient needs to have a "slim" entry point for size-sensitive users, rather than introduce feature switches around ConfigurationManager (and other components).

@steveharter

Copy link
Copy Markdown
Contributor

@kant2002 thanks for the PR but closing based on feedback

Cutting it off inside ConfigurationManager is probably going to lead to "random" runtime errors when some untested path calls into ConfigurationManager, and it throws.

I think SqlClient needs to have a "slim" entry point for size-sensitive users, rather than introduce feature switches around ConfigurationManager (and other components).

@ghostghost locked as resolved and limited conversation to collaborators Apr 12, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Configurationcommunity-contributionIndicates that the PR has been added by a community memberlinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kant2002@eerhardt@roji@steveharter@MichalStrehovsky