Skip to content

Replace CollectionsMarshal.AsSpan with IList<T> indexing in ToFastArray #660

Description

@imperugo

Suggested by @LeaFrock.

Proposal

Replace the List<T> + CollectionsMarshal.AsSpan fast path in GenericsExtensions.ToFastArray(ICollection<T>) with plain IList<T> indexing:

public static TResult[] ToFastArray<TSource, TResult>(this ICollection<TSource>? source, Func<TSource, TResult> action)
{
    if (source is not { Count: > 0 })
        return [];

    if (source is TSource[] sourceArray)
        return sourceArray.ToFastArray(action);

    if (source is IList<TSource> sourceList)
    {
        var listResult = new TResult[sourceList.Count];
        for (var i = 0; i < listResult.Length; i++)
            listResult[i] = action.Invoke(sourceList[i]);

        return listResult;
    }

    var result = new TResult[source.Count];
    var index = 0;
    foreach (var item in source)
        result[index++] = action.Invoke(item);

    return result;
}

Rationale

The CollectionsMarshal.AsSpan branch was introduced in #659. Reviewing it against the actual call-sites, the case for replacing it is stronger than a pure style preference:

1. The List<T> branch is currently dead code. Every call-site of the ICollection<T> overload passes either an array (string[], T[], RedisValue[], EndPoint[] — caught by the array fast path above it) or a HashSet<string> (which is not IList<T> and falls through to the foreach in both the current and proposed versions). No List<T> is passed anywhere in the codebase, and GenericsExtensions is internal, so there are no external callers either. We are carrying an explicitly-unsafe API plus #if NET8_0_OR_GREATER conditional compilation for a path nothing reaches today.

2. CollectionsMarshal is documented as unsafe and implementation-dependent. The contract requires that the list is not structurally modified while the span is alive. Here action is a caller-supplied delegate, so that invariant is only guaranteed by convention. IList<T> indexing has no such caveat.

3. The performance difference is noise at this granularity. The per-element cost is dominated by action.Invoke() (a delegate call, typically serializer or cast work). Interface indexing versus span indexing is not measurable against that, especially with dynamic PGO devirtualizing the List<T> indexer on .NET 8+.

4. Broader type coverage. IList<T> also covers Collection<T>, ReadOnlyCollection<T>, ImmutableArray<T> and custom list wrappers, so future call-sites benefit without further changes.

The proposed version also collapses the null and empty checks into the single is not { Count: > 0 } pattern, which removes the currently duplicated early-return logic.

Acceptance criteria

  • CollectionsMarshal usage and the #if NET8_0_OR_GREATER block removed from GenericsExtensions.cs
  • System.Runtime.InteropServices import dropped if unused
  • Existing test suite green on all TFMs (netstandard2.1, net8.0, net9.0, net10.0)

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions