-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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]: CollectionMarshal.AsArraySegment<T>(List<T>) overload #110275
Comments
Tagging subscribers to this area: @dotnet/area-system-collections |
You can't have the lambda capture it (take a reference to something not passed as a parameter), they work just fine for parameters. You generally want to avoid captures anyways.
This isn't a problem because of
This isn't actually a guarantee. It's an internal implementation detail, and allowed to change (however unlikely that may be in practice).
Your API is backwards. You'd take and slice the (How you'd get a Personally, if I was going to be designing a multi-threaded sorting library, my API would be using |
Generally I would agree, but as I mentioned I occasionally need to pass the (slices of the) span to a new thread. There is no API today that accepts a
Maybe I wasn't quite clear. I meant the design choice of returning a
No, the public API is for
Which I cannot accept as it kills the whole point of performant special handling
That would be quite limiting compared to |
Pass the list and a Range to each of the threads then delay the call to AsSpan to each of the worker threads. |
Right, this was my other suggestion. |
Though I could do this, it wouldn't let me get rid of the duplicate
I didn't react to this one first. The fact that Footnotes
|
This is a point in time implementation detail, not a guarantee. The actual guarantee (which was fixed when we decided to expose the The intended design is that you operate on the Exposing something like Exposing something like The best option in such scenarios is to pass through the |
So my assumption was right: "Maybe this design was chosen to prevent the callers from storing an optionally long-living reference to the momentary inner state of a
Sigh. Please read my opening post. Normally it would be OK just "as is done in LINQ or other functions today", except that This just highlights that such an API would make sense to a lot of developer. But never mind, I closed the issue as I can live with the code duplicates, I just would have liked to get rid of them if possible. |
It can, you just have to move where the span is created. Which is to say, you cannot do: var span = CollectionsMarshal.AsSpan(list);
var thread = new Thead(() => {
OverloadThatHandlesSpan(span);
});
thread.Start(); However, you can trivially do: var thread = new Thread(() => {
var span = CollectionsMarshal.AsSpan(list);
OverloadThatHandlesSpan(span);
});
thread.Start(); You can likewise follow similar principles via You don't need a specific Realistically you just need 2 implementations, one that takes |
@tannergooding Thank you for your thoughts.
Generally you are right about using just The I still will try a few approaches before releasing this version so nothing is final yet. Again, thank you for your last supportive answer. |
Background and motivation
CollectionMarshal
currently offers theAsSpan<T>(List<T>)
method forList<T>
. However,Span<T>
cannot be always used, for example when one would use it in a lambda expression or when it should be passed to another thread. Maybe this design was chosen to prevent the callers from storing an optionally long-living reference to the momentary inner state of aList<T>
, but maybe the consumers ofCollectionMarshal
are aware of the consequences.Concrete example: Multi-threaded
IList<T>
sorting, where there is a better-performing special handling forList<T>
. Unfortunately this special handling is not too effective today. Not just because it's much slower than the array version, but also because it requires an extra implementation, which wouldn't be necessary if I could access the underlying array section as a non-ref
struct.API Proposal
Why not (just)
Memory<T>
?Memory<T>
is a more general concept that can be backed by an array, a string or a memory manager. AsList<T>
always wraps an array, we can use the more specificArraySegment<T>
here. Also, creating aMemory<T>
from anArraySegment<T>
is much more trivial (and more performant) than using theMemory.TryGetArray
method.API Usage
Considering the motivation example again, my code could be rewritten like this:
And then I could simply remove the extra implementation for
List<T>
as it could now use the array path:private interface ISortHelper<T> { void Sort(IAsyncContext context, IList<T> list, int startIndex, int count, IComparer<T>? comparer); void Sort(IAsyncContext context, T[] array, int startIndex, int count, IComparer<T>? comparer); - void Sort(IAsyncContext context, List<T> list, int startIndex, int count, IComparer<T>? comparer); // along with the implementations }
Alternative Designs
No response
Risks
No response
The text was updated successfully, but these errors were encountered: