dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.09k stars 4.7k forks source link

Add feature switch for size optimal output #53671

Open marek-safar opened 3 years ago

marek-safar commented 3 years ago

It's not always possible to make the right choice between size and speed when different implementations exist or are possible. For now, we hardcode that logic based on the runtime pack but that's not optimal. It's hard to test and it's not always the right choice as some scenarios can prefer size over-speed or vice versa based on the developer's need.

Examples, where this could apply, are

The proposed API could like like with AppContext settings and feature switch supported as well.

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeFeature
    {
        public static bool IsSizeOptimized { get; }
    }
}

Note: There could be a better place/type where to put the property

@eerhardt

dotnet-issue-labeler[bot] commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

stephentoub commented 3 years ago

LINQ implementation

How would this apply to LINQ? Most of the size savings there come from not implementing certain interfaces in certain builds. Having this be a choice made post-compilation would seem to require having both implementations, with and without the interface implementations, compiled into the assembly? That's very far from ideal. Have I misunderstood?

marek-safar commented 3 years ago

How would this apply to LINQ? Most of the size savings there come from not implementing certain interfaces in certain builds.

There are two cases. The first one is I think obvious where both size and speed versions exist (we would compile in both and choose the one based on the property). The second one which you are referring to as interface implementation would work similarly. The interfaces would be always compiled in and we would alter the interface reference to be conditional to allow the linker to remove the implementation. For example, changing https://github.com/dotnet/runtime/blob/01b7e73cd378145264a7cb7a09365b41ed42b240/src/libraries/System.Linq/src/System/Linq/Select.cs#L48 to something like

if (!RuntimeFeature.IsSizeOptimized && source is IPartition<TSource> partition)
{
    // existing code
}
stephentoub commented 3 years ago

Thanks. So if every interface dispatch is guarded, the linker will remove the interface and all implementations of it entirely?

marek-safar commented 3 years ago

the linker will remove the interface and all implementations of it entirely

Yep, that should be the case (it might decide to keep the type itself due to casts) for any interface method which is never dispatched over the interface.

stephentoub commented 3 years ago

Can we experiment with this just in System.Linq? For example, can we define a property in the System.Linq.csproj that's then picked up by some internal API, and do something in the linker xml files for System.Linq that teaches the linker what that means? If nothing else, we can see what kind of impact it has on the code (hopefully making it easier to maintain), see if/how the same pattern can be replicated to other libraries, and at some critical mass of use then decide to expose a switch along the lines of what you mention.

eerhardt commented 3 years ago

can we define a property in the System.Linq.csproj that's then picked up by some internal API

An issue with that approach is that if we don't reach "critical mass of use" by .NET 6 ships, we would be "stuck" maintaining that "System.Linq"-specific feature switch.

The more immediate benefit here (in my mind) is the usage is System.Text.Json. If we had this switch, we could trim all of Reflection.Emit in a default Blazor WASM app. This slows reflection-based JSON (de)serialization down for the benefit of a slightly smaller app. Users who want to switch back to the faster reflection-based JSON (de)serialization could flip the switch for more speed.

We could follow the same approach for JSON by defining a "JSON-specific" switch, but then we have the same problem that if we don't combine them into this larger feature switch before .NET 6 ships, we would need to support it long term.

stephentoub commented 3 years ago

An issue with that approach is that if we don't reach "critical mass of use" by .NET 6 ships, we would be "stuck" maintaining that "System.Linq"-specific feature switch.

I actually meant as an internal implementation detail of our assembly-level builds, e.g. so that rather than having two sets of files with different implementations, we could consolidate into one and use the linker at assembly build time to produce the right assembly for mobile vs desktop.

Also, for feature switches that are purely about reducing size and removing optimizations, are we really signing up to support them forever more?

I'm not against a larger switch in principal, but I'm also not sure every library should be treated equally, whether a single on/off is the right level of granularity, etc.

eerhardt commented 3 years ago

Also, for feature switches that are purely about reducing size and removing optimizations, are we really signing up to support them forever more?

I don't think we've actually written a policy on it. Feature switches use runtimeconfig/AppContext switches, my current working plan has been to treat them just like any other runtimeconfig switch.

I'm not sure we have a feature switch that has purely been about reducing size and/or speed optimizations, yet. Looking through the current list, all of them affect behavior in some way.

bartczernicki-msft commented 3 years ago

It looks like this will not make it into .NET 6, but .NET 7?