Proposal: SocketAsyncEventArgs.SetBuffer(Memory<T>)

[stephentoub]

See discussion here:
dotnet/corefx#17237 (comment)

[geoffkizer]

A couple thoughts on this:

I wonder if Span (or Memory) is the way to do this. That is, if there's a way a Span can tell us that its memory is pre-pinned, then we could just add overloads that take Span (which it seems like we will probably want anyway) and then have them check if the given Span is pre-pinned. I chatted with @KrzysztofCwalina about this not too long ago; he was going to think about this...

That said, if Span is not the right way to do this, then we could just add an unsafe overload that takes (byte *, length). This allows both pre-pinned and native memory.

[davidfowl]

Memory<byte> would be the way to go here once it's fleshed out. We just added Memory<T>.Pin which returns a MemoryHandle<T> https://github.com/dotnet/corefxlab/blob/91a112c619f008db7acc2e8f0a7d382889691c9b/src/System.Buffers.Primitives/System/Buffers/MemoryHandle.cs. This will pin if it needs to otherwise will return the pointer and keep the ref count up so it doesn't get disposed.

args.SetBuffer(Memory<byte> memory)

That said, if Span is not the right way to do this, then we could just add an unsafe overload that takes (byte *, length). This allows both pre-pinned and native memory.

This would be an API we could add today that would be nice as well but Memory<byte> solves the problem in a non-unsafe way. I'm open to both though.

[stephentoub]

Since we'd want to do the Memory one anyway to work with native memory, and since I think there are other overheads to work on removing first, I suggest we just hold off until that and then add that one.

[karelz]

Should we fold this request into #19444? It tracks all BCL places that should start using Span & friends, once the API surface is final enough.

[stephentoub]

Added to https://github.com/dotnet/corefx/issues/13892

[davidfowl]

@karelz I'm not a fan of uber issues like that. It's ok for overall tracking but each work item should have its own issue.

[karelz]

@davidfowl I agree that the separate issues are valuable to discuss specific design points. However, Span<T> was far from final back then (not sure where we are now) and having 10s of issues spread around was creating more confusion and distraction rather than value.
Once Span<T> is ready for its prime time in stable API surface (i.e. no huge changes like e.g. #20574), we can kick off / reopen bunch of separate issues to track specific design discussions. I assume you / @KrzysztofCwalina / @terrajobst will tell us when is the right time.

[KrzysztofCwalina]

One way to make it work for pre-pinned memory would be the following:

1. Make Memory.Pin call OwnedMemory.Pin, instead of BufferHandle.Create(_owner, _index)
2. Make OwnedMemory.Pin virtual. Pre-pinned memory would return cached handle without actually doing any pinning.

[davidfowl]

@KrzysztofCwalina that's a much less leaky way to implement it than it is today. Not sure you can cache the handle unless Free was a noop in this case (today it lowers the reference count)