-
Notifications
You must be signed in to change notification settings - Fork 300
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
Improve binary size when using NativeAOT #1942
Comments
Retested with .NET 8 preview 1, the size goes down to 29mb. |
From what I see there following offenders
System.Private.XmlThis is coming from SqlClient/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs Lines 2856 to 2884 in 73c6b2f
and from this place SqlClient/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs Lines 2967 to 2978 in 73c6b2f
System.Text.JsonThis one is a bit tricky, since it's indirectly referenced
Output of WhyDgml if interesting.
Also from authentication here Lines 144 to 164 in 73c6b2f
Output of WhyDgml if interesting.
IdeasFor me, most easier way to reduce size of NativeAOT executable to exclude support for Xml columns by default via AppContextSwitch or at make it opt-out of Xml support. I maybe wrong, but Xml is not that important now for workloads which most likely adopt NativeAOT, but I may be wrong. If Azure-related stuff can be make opt-in that I can take a look at other places and provide more info since I think there more places which related to Azure which can be rooted-out. |
For those interested, npgsql/npgsql#4965 tracks one of the main approaches to reduce size in Npgsql. In a nutshell, this leverages the new DbDataSource abstraction from .NET 6.0 to allow users to start with only the basic featureset, and opt into specific required features in granular fashion. The fact that this is done via a new builder maintains 100% backwards compatibility. |
SqlClient/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/EnclaveDelegate.Crypto.cs Lines 98 to 102 in 73c6b2f
This class brings Also Other questionable dependency is SqlClient/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/AppConfigManager.cs Lines 22 to 30 in 73c6b2f
Lines 119 to 123 in 73c6b2f
and also hard dependency on SqlClient/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs Line 37 in 73c6b2f
Same for EnclaveProviderBase which also has instantiation of MemoryCache. After couple more attemps I see At max I was able to remove 0.5Mb from original 25.8Mb using nighlty .NET 8 ILC when attempt to disable XML support. |
@kant2002 I think that rather than introducing feature switches to strip out ConfigurationManager (and other similar blocks), a better overall approach would be to introduce a new entry point where ConfigurationManager simply isn't used by default (this is the "slim" data source builder approach I mentioned in my comment above). This means that for regular users not concerned with size, the current APIs continue to work exactly as they do today, but those concerned with perf can use the new entry point, and opt only into what they want. This kind of approach would also handle things like Azure.Identity, which you can't really put behind a feature switch but still need to solve. |
@roji I did not ignore your comment about "slim" data source builder. I clearly see other locations where it 100% would be working solution. I'm not so sure about ConfigurationManager. Look at the usages of System.Runtime.Caching.MemoryCache which bring ConfigurationManager dependency internally. https://github.com/search?q=repo%3Adotnet%2FSqlClient%20MemoryCache&type=code So I still have questions what to do. Replace
|
I don't know enough about the differences here, but that definitely could be a way forward... If that eliminates the dependency on ConfigurationManager, that possibly also means that existing settings in App.config (around cache size?) would no longer be respected - I'm guessing all that would have to be clarified. |
In addition to MemoryCache, ConfigurationManager also used in couple places in codebase https://github.com/search?q=repo%3Adotnet%2FSqlClient+ConfigurationManager+language%3AC%23&type=code&l=C%23 but unfortunately ConfigurationManager still across all samples folder. So changing samples potentially additional work item here. |
What are we using MemoryCache for anyway? And how is DataContractSerialization being used? |
DataContractSerialization is coming from Line 76 in 73c6b2f
|
@David-Engel is there a spec that defines what is going to come back from the call made in Lines 74 to 78 in 73c6b2f
If it's well defined we can easily write our own deserialization code rather than use the DataContractDeserializer. |
@Wraith2 I'm not sure. But maybe this one? |
As pointed out by @vonzshik in #1941, a minimal SqlClient application weighs 45MB, which is quite a lot. For use in size-sensitive environments, it would be good to reduce this.
Note: this may be related to #1108 - the Azure-specific functionality is currently built-in, making it opt-in would also reduce binary size.
The text was updated successfully, but these errors were encountered: