API Addition: Add generic Array.Reverse<T>

[justinvp]

Add generic Array.Reverse<T>.

Rationale

Array.Reverse is non-generic. It includes a fast path for a known set of primitive types via a call into the runtime (see TrySZReverse), otherwise it falls back to slower code paths that involve boxing for value types. This is a significant performance cost for non-primitive value types, ~22x slower.

Callers of Array.Reverse (like List<T>.Reverse and ImmutableArray<T>.Builder.Reverse) are affected by this performance issue. As a workaround, List<T>.Reverse and ImmutableArray<T>.Builder.Reverse are being updated to not use the non-generic Array.Reverse method until the generic Array.Reverse<T> method becomes available.

Other non-generic methods on Array have generic counterparts like Array.Sort and Array.Sort<T>, so this would just be making Reverse consistent with those.

Proposed API

public abstract class Array : ...
{
    // Proposed
    public static void Reverse<T>(T[] array);
    public static void Reverse<T>(T[] array, int index, int length);

    // Existing
    public static void Reverse(Array array);
    public static void Reverse(Array array, int index, int length);
}

[justinvp]

cc: @terrajobst

[whoisj]

I like it.

[JonHanna]

How fast a path is the primitive fast-path?
Is it fast enough to make if (TrySZReverse(array, index, length)) return; worth adding to the generic form?
Is it fast enough to consider even adding (or coercing) other blittable types to work with it? I'm imagining it should work with value types that don't have fields that aren't either primitive or also such value types.

[JonHanna]

Actually, that should be if (default(T) != null && TrySZReverse(array, index, length)) return; so the jitter can consider all reference type attempts to be dead code and not jit the path that tries it for any of them.

[mikedn]

@hackcraft

How fast a path is the primitive fast-path?

Not very fast, I did a quick test in the related CoreCLR PR. And for very small arrays TrySZReverse will always loose due to the cost of another call and dispatching to the correct native implementation.

[KrzysztofCwalina]

That would be a very nice addition. It's also a very simple API, and so I marked it "ready for API review" where we can discuss how to make this happen (mainly logistically).

[KrzysztofCwalina]

Assigned to @terrajobst to take it though the API review process.

[terrajobst]

I like the APIs as well.

@weshaggard, what are your thoughts? If there is no more pushback I'd mark it as api-approved.

[weshaggard]

I like the proposed API.

[justinvp]

dotnet/coreclr#7132
dotnet/corert#1842

This issue can now be tagged as "api-needs-exposed"