dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.41k stars 4.76k forks source link

[API Proposal]: Add CollectionMarshal.AsSpan<T>(ObservableCollection<T>) overload #110024

Open zgabi opened 1 day ago

zgabi commented 1 day ago

Background and motivation

Currently it is not possible (without Reflection or other "hack") to access the internal items of the ObservableCollection<T>, which is always a List<T> as a Span<T>

It would be nice to allow to access the internal items as a Span<T>

API Proposal

namespace System.Collections.Generic;

public class CollectionMarshal
{
    public static Span<T> AsSpan(ObservableCollection<T> collection);
}

API Usage

var c = new ObservableCollection<int>();
var span = CollectionMarshal.AsSpan(c);

Alternative Designs

Add a method or property to the ObservableCollection to get the internal list. But I think the CollectionMarshal solution would be better for consistency.

Risks

Same as for CollectionMarshal.AsSpan(List), so warn the users that no items should be added or removed from the collection while the span is used. And no events will be raised (Replace or Update #79713) etc.

dotnet-policy-service[bot] commented 1 day ago

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

tannergooding commented 1 day ago

which is always a List

This is notably an implementation detail. Exposing such a CollectionsMarshal API requires us agreeing that the implementation will never change from some linear/contiguous buffer.

That isn't necessarily desirable for all collection types and needs to be considered on a per type basis.

colejohnson66 commented 1 day ago

Why would you want a span for the collection? The purpose of that type is to notify about changes, and a span would bypass that logic.

eiriktsarpalis commented 20 hours ago

And no events will be raised (Replace or Update https://github.com/dotnet/runtime/issues/79713) etc.

I think this is an important concern, arguably it's violating the contract of the collection.

zgabi commented 20 hours ago

Actually I only need ReadOnlySpan, not a writeable. I wrote Span because List has that return type.

eiriktsarpalis commented 19 hours ago

So the motivation is iteration performance? I don't think this would justify a CollectionsMarshal-like method or us pinning the implementation detail of ObservableCollection.

zgabi commented 14 hours ago

Motivation is avoid extra allocation. I'd like to pass the span to a method which is also called from another part of the code where the source is not an IList/ICollection. (For example stackallocked "list")

eiriktsarpalis commented 13 hours ago

I don't think this is sufficient to justify such an API. If you want to avoid allocations in that context you might consider calling CopyTo on a pooled buffer.