Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
zgabi opened this issue Nov 20, 2024 · 10 comments
Open
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Collections untriaged New issue has not been triaged by the area owner

Comments

@zgabi
Copy link
Contributor

zgabi commented Nov 20, 2024

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.

@zgabi zgabi added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 20, 2024
Copy link
Contributor

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

@tannergooding
Copy link
Member

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
Copy link

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
Copy link
Member

And no events will be raised (Replace or Update #79713) etc.

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

@zgabi
Copy link
Contributor Author

zgabi commented Nov 21, 2024

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

@eiriktsarpalis
Copy link
Member

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
Copy link
Contributor Author

zgabi commented Nov 21, 2024

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
Copy link
Member

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.

@ovska
Copy link

ovska commented Nov 22, 2024

Can you use UnsafeAccessor to access the underlying storage?

@eiriktsarpalis
Copy link
Member

Can you use UnsafeAccessor to access the underlying storage?

It's possible, although this risks breaking in the future if the private field is changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Collections untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

5 participants