Enable UseSizeOptimizedLinq by default on native AOT

[MichalStrehovsky]

The motivation is:

• This has been enabled by default on mobile platforms for several releases and it wasn't a source of issues there
• It is easy to opt out of it. (That cannot be said about mobile platforms in the past that hardcoded it at repo build).
• Doesn't have measurable throughput impact on any ASP.NET scenarios we measure perf of.
• It is a meaningful size saving (seen 5+%).

Cc @dotnet/ilc-contrib

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

[MichalStrehovsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[EgorBo]

Doesn't have measurable throughput impact on any ASP.NET scenarios we measure perf of.

I assume none of them have LINQ in the hot path, at least the ones NativeAOT supports

[am11]

Does it need a breaking change tag per #109978 (comment)?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Numerics;

MyList<int> list = new([1, 2, 3, 4, 5, 6]);
foreach (var item in list.Skip(3).Take(2).Select(item => item)) Console.WriteLine(item);

public class MyList<T>(IList<T> innerList) : IList<T> where T : IMultiplyOperators<T, int, T>
{
    public T this[int index]
    {
        get => innerList[index] * 2;
        set => innerList[index] = value;
    }

    public int Count => innerList.Count;
    public bool IsReadOnly => innerList.IsReadOnly;
    public void Add(T item) => innerList.Add(item);
    public void Clear() => innerList.Clear();
    public bool Contains(T item) => innerList.Contains(item);
    public void CopyTo(T[] array, int arrayIndex) => innerList.CopyTo(array, arrayIndex);
    public int IndexOf(T item) => innerList.IndexOf(item);
    public void Insert(int index, T item) => innerList.Insert(index, item);
    public bool Remove(T item) => innerList.Remove(item);
    public void RemoveAt(int index) => innerList.RemoveAt(index);
    public IEnumerator<T> GetEnumerator() => innerList.GetEnumerator();
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
}

main:

8
10

PR:

4
5

[MichalStrehovsky]

Does it need a breaking change tag per #109978 (comment)?

Hm, do we do a breaking change announcement every time we optimize something in LINQ? (The PR behavior is the same behavior as on .NET Framework.)

[jkotas]

Does it need a breaking change tag per #109978 (comment)?

I do not think so. Linq expects correct implementation of collection contracts. If the collection contracts are not implemented correctly, all bets are off with Linq.

[jkotas]

Are the failures related to the PR?

[MichalStrehovsky]

Are the failures related to the PR?

No, it's the AppContext.BaseDirectory change that broke all Linux. I'll update the branch and rerun testing. But Windows was green.

[MichalStrehovsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ericstj]

@MichalStrehovsky can you please create a breaking change document for this change to help those impacted understand the reasoning for the change and what action they should take if impacted?

[ericstj]

Also given how users are noticing this change - it might be better to consider not enabling by default but somehow advertising it's presence for those that care about size and are willing to do work to refactor their code if it causes issues.

[jkotas]

@ericstj We have discussed whether or not this should be tracked as a breaking change above and the conclusion was that is not needed:
#113214 (comment)
#113214 (comment)

Could you please share you reasoning why it is needed and what would the breaking change say?

The "break" is that if a user has invalid implementation of collection contracts, they can see behavior changes. We have done number of similar "breaks" outside of NativeAOT in the past without filling breaking change notices.

[richlander]

I think the breaking change aspect is a red herring. It might be more useful to update some design or learn doc to explain that our optimizations will favor size over startup/throughput and that we have a knob for adjusting that. I suspect that the community reaction is more one of surprise. If we don't have our intentions clearly written down in a place they know to look, then that's understandable.

[tannergooding]

A callout in the .NET 10 release notes would at least seem reasonable, even if not documented as breaking. Just so we have a centralized place to refer to.

There is an issue (115033) opened last night that is negatively impacted be the change and which is currently getting a lot of passionate community discussion.