Revisit ImmutableExtensions.TryCopyTo behavior for ICollection<T>

[adamsitnik]

From https://github.com/dotnet/runtime/pull/88093#discussion_r1246050257:

We should revisit this. https://github.com/dotnet/runtime/blob/e5ddc6c9b7e481d36d6350960868188d7b275ef7/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs#L93-L96 That logic, while well-meaning, isn't worth making the implementations slower. If code wants to get at the underlying array of an ImmutableArray, there are easier ways, including a now fully supported public API.

The comment that triggered the conversation https://github.com/dotnet/runtime/pull/88093#discussion_r1245436308:

The custom ToArray implementation is slower, because it uses CopyTo only for collections that implement IList<T>, while both KeyCollection and ValueCollection don't. They do implement ICollection<T> which is used by LINQ.

https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs#L158-L164

https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs#L106

https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections/ref/System.Collections.cs#L504

https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections/ref/System.Collections.cs#L532

cc @stephentoub

[ghost]

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

Issue Details

---

From https://github.com/dotnet/runtime/pull/88093#discussion_r1246050257: > We should revisit this. https://github.com/dotnet/runtime/blob/e5ddc6c9b7e481d36d6350960868188d7b275ef7/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs#L93-L96 That logic, while well-meaning, isn't worth making the implementations slower. If code wants to get at the underlying array of an ImmutableArray, there are easier ways, including a now fully supported public API. The comment that triggered the conversation https://github.com/dotnet/runtime/pull/88093#discussion_r1245436308: > The custom `ToArray` implementation is slower, because it uses `CopyTo` only for collections that implement `IList`, while both `KeyCollection` and `ValueCollection` don't. They do implement `ICollection` which is used by LINQ. https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs#L158-L164 https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs#L106 https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections/ref/System.Collections.cs#L504 https://github.com/dotnet/runtime/blob/c06d77a74f64d6c5b0280db56fb0c6a43c0aa907/src/libraries/System.Collections/ref/System.Collections.cs#L532 cc @stephentoub

Author:	adamsitnik
Assignees:	-
Labels:	`area-System.Collections`, `tenet-performance`
Milestone:	-