Productizing APIs for {ReadOnly}Memory<T> and friends

[ahsonkhan]

Open issues to resolve as part of productizing:

1. We changed the AsSpan() method into an abstract property named Span.
   • When do we intend to use properties versus methods for span/memory related conversions?
2. Should we add the "bulk" operations that are available on Span<T> to Memory<T> as well such as the ones listed below:
   • Where would the extension methods for Memory go? Its own MemoryExtensions class like SpanExtensions or should we merge them both into one? Given these are extension methods (and the classes won't appear in user code), I would say they should go in separate MemoryExtensions class.

// instance methods
public struct Memory<T> {
  public void CopyTo(Memory<T> destination);
  public bool TryCopyTo(Memory<T> destination);
  public void Fill(T value);
  public void Clear();
}
public struct ReadOnlyMemory<T> {
  public void CopyTo(Memory<T> destination);
  public bool TryCopyTo(Memory<T> destination);
}
// extension methods
public static class MemoryExtensions {
  public static int IndexOf<T>(this Memory<T> memory, T value);
  public static int IndexOf<T>(this Memory<T> memory, ReadOnlyMemory<T> value);
  public static int IndexOf<T>(this ReadOnlyMemory<T> memory, T value);
  public static int IndexOf<T>(this ReadOnlyMemory<T> memory, ReadOnlyMemory<T> value);
  public static bool SequenceEqual<T>(this Memory<T> first, ReadOnlyMemory<T> second);
  public static bool SequenceEqual<T>(this ReadOnlyMemory<T> first, ReadOnlyMemory<T> second);
  public static void CopyTo<T>(this T[] array, Memory<T> destination);
  public static Memory<T> AsMemory<T>(this T[] array);
  public static Memory<T> AsMemory<T>(this ArraySegment<T> arraySegment);
  public static ReadOnlyMemory<T> AsReadOnlyMemory<T>(this T[] array);
  public static ReadOnlyMemory<T> AsReadOnlyMemory<T>(this ArraySegment<T> arraySegment);
}

Here are the Memory APIs that have already gone through API review

They were added here:

• Adding {ReadOnly}Memory, OwnedMemory, MemoryHandle, and IRetainable coreclr#13583 (mscorlib)
• Adding Memory, OwnedMemory, MemoryHandle, and IRetainable & Tests corefx#23701 (System.Memory)
• Expose {ReadOnly}Memory from System.Runtime corefx#23841 (System.Runtime)

namespace System
{
    public struct Memory<T>
    {
        public static Memory<T> Empty { get; }
        public Memory(T[] array);
        public Memory(T[] array, int start, int length);
        public bool IsEmpty { get; }
        public int Length { get; }
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override bool Equals(object obj);
        public bool Equals(Memory<T> other);
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override int GetHashCode();
        public static implicit operator Memory<T>(T[] array);
        public static implicit operator Memory<T>(ArraySegment<T> arraySegment);
        public static implicit operator ReadOnlyMemory<T>(Memory<T> memory);
        public Memory<T> Slice(int start);
        public Memory<T> Slice(int start, int length);
        public Span<T> Span { get;}
        public unsafe Buffers.MemoryHandle Retain(bool pin = false);
        public T[] ToArray();
        public bool TryGetArray(out ArraySegment<T> arraySegment);
    }
    public struct ReadOnlyMemory<T>
    {
        public static ReadOnlyMemory<T> Empty { get;}
        public ReadOnlyMemory(T[] array);
        public ReadOnlyMemory(T[] array, int start, int length);
        internal ReadOnlyMemory(Buffers.OwnedMemory<T> owner, int index, int length);
        public bool IsEmpty { get;}
        public int Length { get;}
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override bool Equals(object obj);
        public bool Equals(ReadOnlyMemory<T> other);
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override int GetHashCode();
        public static implicit operator ReadOnlyMemory<T>(T[] array);
        public static implicit operator ReadOnlyMemory<T>(ArraySegment<T> arraySegment);
        public ReadOnlyMemory<T> Slice(int start);
        public ReadOnlyMemory<T> Slice(int start, int length);
        public ReadOnlySpan<T> Span { get;}
        public unsafe Buffers.MemoryHandle Retain(bool pin = false);
        public T[] ToArray();
        [EditorBrowsable(EditorBrowsableState.Never)]
        public bool DangerousTryGetArray(out ArraySegment<T> arraySegment);
    }
}

namespace System.Buffers
{
    public unsafe struct MemoryHandle : IDisposable 
    {
        public MemoryHandle(IRetainable owner, void* pointer = null, GCHandle handle = default(GCHandle));
        public void* Pointer { get; }
        public bool HasPointer { get; }
        public void Dispose();
    }

    public interface IRetainable 
    {
        bool Release();
        void Retain();
    }

    public abstract class OwnedMemory<T> : IDisposable, IRetainable 
    {
        public Memory<T> Memory { get; }
        public abstract bool IsDisposed { get; }
        protected abstract bool IsRetained { get; }
        public abstract int Length { get; }
        public abstract Span<T> Span { get; }
        public void Dispose();
        protected abstract void Dispose(bool disposing);
        public abstract MemoryHandle Pin();
        public abstract bool Release();
        public abstract void Retain();
        protected internal abstract bool TryGetArray(out ArraySegment<T> arraySegment);
    }
}

(https://github.com/dotnet/corefx/blob/master/src/System.Memory/ref/System.Memory.cs#L149)

cc @weshaggard, @KrzysztofCwalina, @shiftylogic, @stephentoub, @terrajobst, @karelz, @jkotas, @davidfowl

[svick]

public static implicit operator Memory<T>(ArraySegment<T> arraySegment);
public static implicit operator ReadOnlyMemory<T>(Memory<T> memory);
public static implicit operator ReadOnlyMemory<T>(ArraySegment<T> arraySegment);

Are these the only ways to convert from ArraySegment<T> to (ReadOnly)Memory<T> and from Memory<T> to ReadOnlyMemory<T>? Framework Design Guildelines say:

✓ CONSIDER providing methods with friendly names that correspond to each overloaded operator.

Specifically, F# does not support implicit operators, but allows calling them as op_Implicit. Does this mean that the only way to convert from Memory<T> to ReadOnlyMemory<T> in F# would be the following?

let memoryToReadOnlyMemory (memory : Memory<'a>) : ReadOnlyMemory<'a> =
    Memory<'a>.op_Implicit memory

For the ArraySegment<T> conversions, there is a fairly simple workaround: use the T[], int, int constructors. But for the "to readonly" conversion, I don't see any other way.

[ahsonkhan]

Are these the only ways to convert from ArraySegment<T> to (ReadOnly)Memory<T> ...?

I have added the following extension methods (analogous methods exist for Span) to the proposal for review:

  - public static Memory<T> AsMemory<T>(this T[] array);
  - public static Memory<T> AsMemory<T>(this ArraySegment<T> arraySegment);
  - public static ReadOnlyMemory<T> AsReadOnlyMemory<T>(this T[] array);
  - public static ReadOnlyMemory<T> AsReadOnlyMemory<T>(this ArraySegment<T> arraySegment);

But for the "to readonly" conversion, I don't see any other way.

The same issue occurs for Span to ReadOnlySpan (since only an implicit conversion is available). Can you please file a separate issue regarding this for both types?
Would an explicit conversion instance method from Memory to ReadOnlyMemory, i.e. ReadOnlyMemory<T> AsReadOnlyMemory() solve this issue (along with ReadOnlySpan<T> AsReadOnlySpan())?

[terrajobst]

Video

@ahsonkhan mentioned he's not ready for a review yet.

[ahsonkhan]

The Memory<T> APIs are ready for review.

Here is what we need to discuss.

1. We changed the AsSpan() method into an abstract property named Span.
   • When do we intend to use properties versus methods for span/memory related conversions?
2. Should we add the "bulk" operations that are available on Span<T> to Memory<T> as well such as the ones listed below:
   • Where would the MemoryExtensions go? Its own MemoryExtensions class like SpanExtensions or should we merge them both into one? Given these are extension methods (and won't appear in user code), I would say they should go in separate MemoryExtensions class.

// instance methods
public struct Memory<T> {
  public void CopyTo(Memory<T> destination);
  public bool TryCopyTo(Memory<T> destination);
  public void Fill(T value);
  public void Clear();
}
public struct ReadOnlyMemory<T> {
  public void CopyTo(Memory<T> destination);
  public bool TryCopyTo(Memory<T> destination);
}
// extension methods
public static class MemoryExtensions {
  public static int IndexOf<T>(this Memory<T> memory, T value);
  public static int IndexOf<T>(this Memory<T> memory, ReadOnlyMemory<T> value);
  public static int IndexOf<T>(this ReadOnlyMemory<T> memory, T value);
  public static int IndexOf<T>(this ReadOnlyMemory<T> memory, ReadOnlyMemory<T> value);
  public static bool SequenceEqual<T>(this Memory<T> first, ReadOnlyMemory<T> second);
  public static bool SequenceEqual<T>(this ReadOnlyMemory<T> first, ReadOnlyMemory<T> second);
  public static void CopyTo<T>(this T[] array, Memory<T> destination);
  public static Memory<T> AsMemory<T>(this T[] array);
  public static Memory<T> AsMemory<T>(this ArraySegment<T> arraySegment);
  public static ReadOnlyMemory<T> AsReadOnlyMemory<T>(this T[] array);
  public static ReadOnlyMemory<T> AsReadOnlyMemory<T>(this ArraySegment<T> arraySegment);
}

[terrajobst]

Video

We don't want to add the bulk methods. We believe it could become a performance trap and it invites having to keep Span<T> and Memory<T> in sync.

// instance methods
public struct Memory<T> {
  public void CopyTo(Memory<T> destination);
  public bool TryCopyTo(Memory<T> destination);
}
public struct ReadOnlyMemory<T> {
  public void CopyTo(Memory<T> destination);
  public bool TryCopyTo(Memory<T> destination);
}
// extension methods
// Merge with SpanExtensions
public static class MemoryExtensions {
  public static void CopyTo<T>(this T[] array, Memory<T> destination);
}

We should probably rename SpanExtensions to MemoryExtensions.

[karelz]

FYI: The API review discussion was recorded - see https://youtu.be/bHwCPVNQLwo?t=333 (37 min duration)