Add optimized path for IReadOnlyCollection/IReadOnlyList in System.Linq

[huoyaoyuan]

It's kind of historical reason for the readonly interfaces not inherited by ICollection<T>/IList<T>.
Adding a optimized path needs a copy of all ICollection<T>/IList<T> paths and classes.
Implementing ICollection<T> with IsReadOnly = true does get the optimizations, but it's kind of violating the interfaces.

[stephentoub]

Optimized path in what? What optimizations? What's the scenario?

[huoyaoyuan]

Sorry for forgetting type LINQ in the title.

[molinch]

@stephentoub @huoyaoyuan It's not just about Linq but also in many other areas like: List.InsertRange, see: http://referencesource.microsoft.com/#mscorlib/system/collections/generic/list.cs,711

If you give an IReadOnlyCollection, and that instance does not implement ICollection too, it will then instanciate an Enumerator and iterate, otherwise it could be super optimized.
However that's a very minor issue since BCL types implement ICollection, so it's only if you roll your own that you may get this problem.

By the way, shouldn't ICollection implement IReadOnlyCollection? Then less code would need to touched: we would only need to change the cast to IReadOnlyCollection here.

[stephentoub]

It's not just about Linq but also in many other areas like: List.InsertRange [...] otherwise it could be super optimized [...]

Yes. But adding that IReadOnlyCollection<T> type check is an expense that's paid for all enumerables, not just IReadOnlyCollection<T>. As with many optimizations, you make some things faster at the expense of making other things slower, and the upside needs to be worth the downside. As you pointed out, most enumerables in the core libraries that are IReadOnlyCollection<T> also implement other interfaces already optimized for, so the upside for the majority of collections people use is small. To take such a change, there'd need to be a solid set of data provided about what the positive and negative impacts would be and why it's worthwhile.

shouldn't ICollection implement IReadOnlyCollection?

It's actually a breaking change. See https://github.com/dotnet/corefx/issues/5489 for an example.

[molinch]

Thanks @stephentoub for the quick feedback.
Couldn't what Nick shows be solved by the C# compiler?

Just another remark related to this, Array.cs doesn't implement IReadOnlyList neither IReadOnlyCollection. Would you accept a PR doing so?

[stephentoub]

doesn't implement IReadOnlyList neither IReadOnlyCollection

Arrays do, they're just a very special type and the implementation is provided by the runtime, i.e. this works fine:

using System;
using System.Collections.Generic;

class Program
{
    static void Main()
    {
        var data = new int[1] { 42 };
        IReadOnlyList<int> list = data;
        Console.WriteLine(list[0]);
    }
}

[molinch]

That's interesting! I expected what is in Array.cs to be the source of truth.
Thanks a lot for the explanation.

[mattwarren]

@molinch <shameless plug> if you want more info on this, see a blog post I wrote Arrays and the CLR - a Very Special Relationship, in particular the 'Implementation Details' section </shameless plug>

[molinch]

Many things are much more clear, thanks @mattwar !
I love the sentence "Arrays are basically voodoo." hahaha

[stephentoub]

Closing as dup of https://github.com/dotnet/corefx/issues/26579