Corefxlab: [discussion][Spans] Is TryGetArrayElseGetPointer really needed?

[adamsitnik]

I can see that there is a method called TryGetArrayElseGetPointer which returns the array or pointer.
If it's array people pin the array, if it's pointer, they pin the pointer. The code looks at least a little bit complicated (extra if/else + out variables).

Would it be sufficient to add a new method called void* GetPointerForPinning() which would return the right address for pinning the array (if present) or the unsafe pointer? Without the offset like UnsafePointer does, so it could be pinned?

then the usage could be following:

fixed(void* pinned = span.GetPointerForPinning())
{
   void* realStart = span.UnsafePointer();
   // using realStart, forgetting about pinned 
}

maybe we could go even one step further and add IDisposable Pin() method? Then the implementation would be a detail, so it would be easier to have same api and behaviour for corefxlab and coreclr Spans?

using(span.Pin())
{
   // using span, forgetting about pinning
}

@omariom @KrzysztofCwalina @davidfowl @VSadov @jkotas

[KrzysztofCwalina]

Ultimately, we need a way to pin spans, but for now we need this method. You cannot return pinned pointers. They are pinned only inside the fixed block.

[adamsitnik]

@KrzysztofCwalina I think that my description was not good enough.

Sample code before:

public int Send(Span<byte> bytes)
{
    unsafe {
        ArraySegment<byte> segment;
        void* pinned;
        if (bytes.TryGetArrayElseGetPointer(out segment, out pinned)) {
            return Send(segment);
        }
        else {
            var pointer = new IntPtr(pinned);
            return SendPinned(pointer, bytes.Length);
        }
    }
}

public int Send(ArraySegment<byte> buffer)
{
    return Send(buffer.Array, buffer.Offset, buffer.Count);
}

public int Send(byte[] bytes, int offset, int count)
{
    unsafe {
        fixed (byte* buffer = bytes) {
            IntPtr pointer = new IntPtr(buffer + offset);
            return SendPinned(pointer, count);
        }
    }
}

Sample code after:

public int Send(Span<byte> bytes)
{
    unsafe {
        fixed(void* pinned = bytes.GetPointerForPinning())
        {
            return SendPinned(bytes.UnsafePointer(), bytes.Length);
        }
    }
}

[jkotas]

The method on Span to support pinning should be something like ref T GetUnsafeFirstElementRef(). The pinning can then be done via (requires C# 7 byref returns):

fixed (int * p = &mySpanOfInt.GetUnsafeFirstElementRef())
{
}

It would be nice if C# would have shortcut for it so that you can write just:

fixed (int * p = mySpanOfInt)
{
}

This shortcut can be pattern based, not Span<T> specific - it would just look for GetUnsafeFirstElementRef method. It would also solve one of the problems with switching to more compact string representation discussed here.

cc @jaredpar

[jaredpar]

This shortcut can be pattern based, not Span specific - it would just look for GetUnsafeFirstElementRef method.

Completely agree it should be pattern based.

[KrzysztofCwalina]

@adamsitnik, what's the implementation of GetPointerForPinning?

[davidfowl]

How does any of this work for async with the stack only requirement? I need to be able to "unwrap" a span for pinvoking into native APIs that might be async.

[KrzysztofCwalina]

@davidfowl, we will need a "get GC handle" like API for this.

[davidfowl]

Right, thats why it does make sense to consider the span.Pin() disposable pattern for this (even if we do the pattern matched fixed C# API)

``` C#
public struct PinnedSpan : IDisposable
{
IntPtr _pointer;
GCHandle _handle;

public PinnedSpan(Span span)
{
ArraySegment segment;
void* pinned;
if (bytes.TryGetArrayElseGetPointer(out segment, out pinned))
{
_handle = GCHandle.Alloc(segment.Array, GCHandleType.Pinned);
_pointer = _handle.AddrOfPinnedObject();
}
else
{
_pointer = new IntPtr(pinned);
}
}

public IntPtr Pointer => _pointer;

public void Dispose()
{
_handle.Free()
}
}

``` C#
public class MyAsyncThing
{
    PinnedSpan pinned;

    public void Write(Span<byte> span)
    {
        pinned = span.Pin();
        uv_write(pinned.Pointer, OnWriteComplete, this);
    }

    public static OnWriteComplete(MyAsyncThing thing)
    {
        thing.pinned.Dispose();
    }
}

[adamsitnik]

what's the implementation of GetPointerForPinning?

@KrzysztofCwalina Something like this (pseudocode):

struct Span<T>
{
    public unsafe void* GetPointerForPinning()
    {
        if(Object != null && Object is T[])
        {
            // this is the pointer calculated by compiler when fixed is used for Array
            return Object + (uint)SpanHelpers<T>.OffsetToArrayData); 
        }

        return Offset; // for span created with Span(void* ptr, int length)
    }
}

struct ReadOnlySpan<T>
{
    public unsafe void* GetPointerForPinning()
    {
        if(Object != null)
        {
            if(Object is T[])
            {
                return Object + (uint)SpanHelpers<T>.OffsetToArrayData);
            }
            else if(Object is string)
            {
                return Object + (uint)SpanHelpers.OffsetToStringData;
            }
        }

        return Offset; // for span created with Span(void* ptr, int length)
    }
}

I could implement that, add unit test and update related projects (including David's Channell project).

[adamsitnik]

@jkotas I really like your idea, it's very simple and efficient. But I have some concerns.

The current representation for corefxlab span is following:

For coreclr:

Jan is my understanding right:

• we need to pin the pointer which points to the beggining of the underlying buffer (array, string, unmanaged pointer) for corefxlab span (Pointer field on the image, Object field in the code), but use Pointer + Offset to get the address of the first element (first in the given Span, not in the underlying buffer)
• with some changes in the coreclr we could only get the address of the first element in the Span and pin in? (for example pointer that points to the middle of the underlying array?)

If so how could the api for pinning spans be unified?

[KrzysztofCwalina]

@adamsitnik, the usage code won't compile as you cannot used fixed block to fix pointers (the void* returned from GetPointerForPinning).

[jkotas]

we need to pin the pointer which points to the beggining of the underlying buffer

You can pin any interior pointer using fixed. It does not have to be the object reference.

[jkotas]

For example, you can do this in C# today:

...
    int[] myArray = new int[100];
    bar(ref myArray[5], 10);
...

int UnsafeSum(ref int start, int count)
{
     fixed (int * p = &start) // pins the whole array
     {
          int sum = 0;
          for (int i = 0; i < count; i++) sum += p[i];
          return sum;
     }
}

[davidfowl]

So any thoughts on the PinnedSpan?

[KrzysztofCwalina]

It would be best to reuse GCHandle, but for now, we could do PinnedSpan/Bytes. @jkotas, are you ok with it?

[jkotas]

It has same set of problems that come with allowing storing span on GC heap. And it won't be possible to implement for two field span (unless we do a GC feature that makes GC slower).

[KrzysztofCwalina]

@davidfowl, Jan is right.

Do you think we can do this "handle" feature for System.Buffers.Bytes instead of Span? i.e. Does your pipline still have access to IBuffer/Bytes when it needs to do async interop?

[KrzysztofCwalina]

Besides making GC slower, we cannot pin even theoretically if the span wraps a stack pointer.

Span s = stackalloc byte[100];
var handle = s.GetPinnedHandle(); // ?????
DoAsyncCall(handle);

[adamsitnik]

Does the idea of Spans comes from the Midori project? If so do you guys know how this problem was solved there?

Btw. Even if we make GC slower by adding some extra work for it, the possiblities that Span can create like zero copy streams etc can improve overall .NET performance.

[davidfowl]

So span is unusable for async is what you're saying? What if I come at this from another angle, today in channels the default memory pool impl, pins the byte[] up front so the use of unsafe pointer is safe for my scenarios. What I'm hearing on this thread seems to be, you can't get an IntPtr to pass to an async native io call safely from span. Do you guys consider Span more special than other Ts? Maybe we can do something special byte[] ?

[jkotas]

span is unusable for async is what you're saying

Right, span does not work well for async. This was discussed number of times (e.g. https://github.com/dotnet/coreclr/issues/5851). We really need to have the design doc written so that observations like this do not have to be reinvented ... @KrzysztofCwalina is working on it.

how this problem was solved there?

Key part of the solution was SharedData type. You can pass it around any way you like. You checked out Span from it when you needed to synchronously process the data that it was holding. Pretty similar to what is described here.

[davidfowl]

@jkotas I understand that but I don't think that helps. You're basically saying I need to fragment my APIs into IntPtr/byte[] behind the scenes only handing out Span<byte> at the last minute. That's very unfortunate and breaks the abstraction but if that's the direction then it would be good to know sooner than later 😄 .

BTW the SharedData pattern @KrzysztofCwalina outlines here (https://github.com/dotnet/coreclr/issues/5851#issuecomment-236643434) is what channels does already. For reading, you get access to the buffers after the async call returns. _BUT_ If you're trying to implement a channel based on libuv for example you need to allocate a buffer from the pool and pass it to libuv APIs directly:

https://github.com/davidfowl/Channels/blob/master/src/Channels.Networking.Libuv/UvTcpConnection.cs#L199

The channel implementation hands out an IBuffer that wraps a Span<byte> (the whole Span promise pattern) and passes the IntPtr to libuv. BTW this all works great for producing until the producer has an async callback. Maybe this is where we use Bytes @KrzysztofCwalina ?

I have strong opinions on this topic so if you need help fleshing out some design I'd be glad to give feedback.

[davidfowl]

Ok so lets say we pretend like spans and async are incompatible, it means that the unification of managed and native memory happens for readers and sync writers but not async writers. It means if you need to write to memory that may be later represented by a Span<byte>, you have to manually expose a byte* or IntPtr or byte[] or something like PinnableSpan from your own abstraction.

[davidfowl]

After playing with this for a little bit here are my thoughts:

• Rename Bytes to Memory<T>
• Move TryGetArrayElseGetPointer, UnsafePointer and TryGetArray from Span<T> to Memory<T> to make the underlying memory available (I don't like TryGetArrayElseGetPointer).
• Expose a Span or similar property on Memory<T> that does the same thing as the implicit cast (returns a Span over the memory)
• The memory pools return Memory<byte>, that API would need to be exposed to get access to the underlying data. There shouldn't be a way to unwrap the Span<T> -> Memory<T> but Memory<T> -> Span<T> is ok.

C# public struct Memory<T> { // .. same ctor as Span<T> public void* UnsafePointer { get; } public int Length { get; } public Span<T> Span { get; } public Memory<T> Slice(int offset, int length); public bool TryGetArray(out ArraySegment<T> buffer); public static implicit operator Span<T>(Memory<T> memory); }

My thoughts are that you use Memory<T> at the seams of the API when you need to break out of the sandbox and interop with something real (Stream/Native IO APIs) and Span<T> otherwise.

Open questions:

• Need a way to specify and unsafe pointer even when the memory is backed by an array. This might be a special case?
• Do you need to be able to pin Memory<T>?
• There are safety concerns around tearing etc, do we care (what happens when you pass stack memory..)?

[jaredpar]

@davidfowl

The channel implementation hands out an IBuffer that wraps a Span

This can't work though for the same reason Span<T> can't be used with async: it implicitly requires that Span<T> instance be on the heap whenever a reference to an IBuffer exists.

So span is unusable for async is what you're saying?

Yes and more. The underlying limitation is that it can't be allowed to escape to the heap. That means in order for the compiler to produce code which is guaranteed to be correct it must disallow all of the following related to Span<T>:

• An async method cannot have a Span<T> parameter or local.
• A Span<T> value cannot be captured in a lambda.
• A Span<T> cannot appear as a local or parameter in iterator method.
• A type which has a field, transitively, of type Span<T> cannot:
  
  
  
  • Implement interfaces.
  
  
  • Be a field of a class.
  
  
  • Be used a a generic argument.
  
  
• Cannot have arrays of Span<T> (or types that contain them).
• A Span<T> value cannot be converted to object.

There are surely more that I am missing here. But really Span<T> is only safely used as:

• A non-async method return.
• A non-async, non-iterator, method parameter or local.
• A field of another struct which cannot implement interfaces and has all the restrictions of Span<T>.

[davidfowl]

@jaredpar See the latest proposal for the Memory<T> type. @KrzysztofCwalina @jaredpar maybe time to beef up the analyzer so we can test out these limitations in real life?

[adamsitnik]

We really need to have the design doc written so that observations like this do not have to be reinvented ... @KrzysztofCwalina is working on it.

It would be great to have it. I hope that it will not only contain a list of limitations but also reason for each limitation.

@davidfowl I like your idea. May I ask what is the "memory flow" for kestrel? Does the http stuff comes from some native api, where it's allocated on unmanaged heap / stack and then it's copied to managed memory? I am asking because I would like to have broader picture so I could try to come up with better ideas.

[davidfowl]

May I ask what is the "memory flow" for kestrel? Does the http stuff comes from some native api, where it's allocated on unmanaged heap / stack and then it's copied to managed memory? I am asking because I would like to have broader picture so I could try to come up with better ideas.

Kestrel works just like channels does (channels is a derivative of things we did in Kestrel but made more general purpose and pluggable). In Kestrel specifically, managed memory is allocated in slabs and pinned up front, then those buffers are passed directly to libuv (they ask us to allocate, we hand them memory, then they tell us how much was read into the buffer). Since we implement Stream over those buffers, the higher layers in the stack end up copying data from Kestrel's pool to another set of pooled memory when reading the body of the request. We recently took steps towards solving this by implementing CopyToAsync on the request stream https://github.com/aspnet/KestrelHttpServer/commit/63509b9e1076c3039ea61e6c3d10a05bde2296dd.

Channels aims at solving the copies more generally by exposing the memory directly from the underlying system/pool. It also formalizes things like transferring buffer ownership so that pooled memory can be passed around without copies (by reference counting or whatever strategy the pool decides to implement). Channels currently takes a heavy dependency on Span<byte> and I think it represents the type of system that can be built on the promise of Span<T>. It also shows the need to do async to interop with the OS for things File and network IO.

Here are some examples of fully functioning things built with channels:

• Libuv TCP Channel - https://github.com/davidfowl/Channels/blob/master/src/Channels.Networking.Libuv/UvTcpConnection.cs
• FileChannel using Win32 IOCP - https://github.com/davidfowl/Channels/blob/master/samples/Channels.Samples/IO/ReadableFileChannel.cs
• Low alloc webSocket server - https://github.com/StackExchange/Channels.WebSockets
• Libuv http server - https://github.com/davidfowl/Channels/blob/master/samples/Channels.Samples/RawLibuvHttpServerSample.cs

I've added a Memory<T> implementation to flesh out any problems with the design https://github.com/davidfowl/Channels/blob/master/src/Channels/Memory.cs.

[davidfowl]

PR: https://github.com/dotnet/corefxlab/pull/844

[mgravell]

@jaredpar note - as I read your notes, this means that the existing Span<T> : IEnumerable<T> is also fundamentally broken. You can _just about_ get away with custom struct-based GetEnumerator, as long as the enumerator is also struct-only. But not IEnumerable<T>.

[adamsitnik]

@mgravell We are currently getting rid of this problem.

[mgravell]

@adamsitnik great, thanks; although now my brain is asking: should Memory<T> : IEnumerable<T> (brain quietly explodes)

[adamsitnik]

although now my brain is asking: should Memory : IEnumerable (brain quietly explodes)

@mgravell I would say no. Not because it could affect memory/type safety but because LINQ is slow. If people want to use Spans/Memory to get best possible performance they should also forget about LINQ. But that's my opinion and I am only contributor here.

[KrzysztofCwalina]

I agree with @adamsitnik

[KrzysztofCwalina]

closing as these methods have been removed from Span and ROSpan.