Runtime: Allow recovering writable/heapable buffers from "lower level" types.

Created on 27 Nov 2018  路  14Comments  路  Source: dotnet/runtime

Thanks to @ChadNedzlek for the "why can't you?" as a followup to the "how many copies of this?" do we need from dotnet/runtime#26785 and dotnet/runtime#934:

The general idea is that when using utility methods that "downcast" from Memory toward ReadOnlySpan, a caller can recover their writable (and/or heapable) needs operating on the same memory range. This means that for cases where writable doesn't feel very common (or heapable doesn't feel very common) the caller has a common "how do I slice my original data to match?" escape valve; and frameworks can fill in the {ReadOnly|}{Memory|Span} matrix as needed by scenario usage (for the places that the performance memory.Span twice is unacceptable).

```C#
public partial struct Span
{
///


/// Get a slice of this (writable) span which has the same start and length as a
/// contained readonly span.
///

///
/// If containedSpan is empty, the behavior is the same as Slice(0, 0).
///

/// containedSpan is not a sub-span of this span
public Span SliceTo(ReadOnlySpan containedSpan);
}

public partial struct ReadOnlyMemory
{
///


/// Get a slice of this memory which has the same start and length as a span over the
/// same memory
///

///
/// If containedSpan is empty, the behavior is the same as Slice(0, 0).
///

///
/// containedSpan is not a sub-span of this span
///

public ReadOnlyMemory SliceTo(ReadOnlySpan containedSpan);
}

public partial struct Memory
{
///


/// Get a slice of this memory which has the same start and length as a span
/// over the same memory
///

///
/// If containedSpan is empty, the behavior is the same as Slice(0, 0).
///

/// containedSpan is
/// not a sub-span of this span

public Memory SliceTo(ReadOnlySpan containedSpan);

/// <summary>
/// Get a slice of this (writable) memory which has the same start and length
/// as a contained readonly memory.
/// </summary>
/// <remarks>
/// If containedSpan is empty, the behavior is the same as Slice(0, 0).
/// </remarks>
/// <exception cref="ArgumentOutOfRangeException">containedSpan is
/// not a sub-span of this span</exception>
public Memory<T> SliceTo(ReadOnlyMemory<T> containedSpan);

}

Sample usage for Split:

```C#
Span<byte> data = ...;

foreach (ReadOnlySpan<byte> segment in ((ReadOnlySpan<byte>)data).Split(0xFF))
{
    Span<byte> writableSegment = data.SliceTo(segment);
    writableSegment.Clear();
}

CC: @stephentoub @GrabYourPitchforks @terrajobst @ahsonkhan @KrzysztofCwalina

area-System.Memory

All 14 comments

Do we really need the Memory<T>.SliceTo(ROM<T>) overload? Presumably we can accomplish the same thing via the ROS<T> overload: var memory = memory.SliceTo(otherMemory.Span);'.

Assuming we keep the ROM<T> overload, in person I had previously stated that we should require this._object and other._object have referential equality, but upon further reflection I don't think that's necessary. The ROS<T> overload can never enforce such a requirement regardless, so for symmetry the ROM<T> overload also shouldn't enforce that requirement.

Do we really need the Memory<T>.SliceTo(ROM<T>) overload?

AFAIK: VB can't say the word Span, so yeah. Plus, it could (but maybe won't be) be more optimal than deferring to Span.

Sample usage for Split

C# foreach (ReadOnlySpan<byte> segment in data.AsReadOnly().Slice(0xFF))

I am not sure I understand the sample. We don't have an AsReadOnly() API on Span<T>. And what are we foreaching over? How does slicing a span return an enumerator?

Edit: OK, I think I get it now. Slice -> Split and we are enumerating over SpanSplitEnumerator from https://github.com/dotnet/corefx/issues/26528. Still, how did we end up with "readonly-ness"? I understand the scenario/benefit of such an API, but the sample isn't clear.

@cdmihai, I would love to hear what you think of such an API given your previous porting effort/feedback such as https://github.com/dotnet/corefx/issues/31525#issuecomment-442143768

Would such an API help reduce the interchangeability burden between ROM and ROS for your scenario?

@ahsonkhan Fixed the AsReadOnly() to a cast, and corrected my finger-brain "Slice" to "Split".

The readonly-ness is because Split is only being defined over ReadOnlySpan, so it had to be converted for extension invocation. (I could have called the method statically, but that'd require remembering where it was going to end up :smile:)

Would such an API help reduce the interchangeability burden between ROM and ROS for your scenario?

Not dramatically, but would be useful to have. In msbuild's scenario, we start with a big string and then we parse it down to many small parts (slicing, splitting, trimming). So we don't have a one to one mapping between input string/memory and output span, but one string/memory to many atomic parts represented as spans. Along the way, many of these small spans either need to get stored in some heap based object model, or need to be stringified to get sent to .net reflection APIs (the big input string has some pseudo c# parts which get interpreted via reflection). One of the most common needs here is to address string / memory dictionaries and sets via a Span. Most of the atomic parts are reoccurring, so the ability to cache Span.ToString via some lookup table would be awesome (hence the need for addressing hash based string collections).

Maybe in msbuild's case we should just use Memory and not Span. But even with Memory, it's hard to address hash based collections.

@cdmihai Theoretically this would help with a dictionary; in that you could use the ReadOnlyMemory<char> as the key; and after getting back a ReadOnlySpan<char> from an operation on the string you could take the bigger ReadOnlyMemory<char> and s_table[bigMem.SliceTo(span)] (though that wouldn't work if the span was something newly allocated, and not a slice of the big string).

The operation seems rare while Slice is very common, which seems unfortunate.

Options:

  • Make this a static helper. We don't have a great type for that (MemoryExtension only houses extension methods and MemoryMarshal feels like the wrong type as this operation isn't unsafe).
  • Different name. For example, bool TryGetBounds(...).

We could also just add another set of overloads to overlaps, like:

```C#
namespace System
{
public static class MemoryExtensions
{
// Existing:
// public static bool Overlaps(this Span span, ReadOnlySpan other);
// public static bool Overlaps(this Span span, ReadOnlySpan other, out int elementOffset);
// public static bool Overlaps(this ReadOnlySpan span, ReadOnlySpan other);
// public static bool Overlaps(this ReadOnlySpan span, ReadOnlySpan other, out int elementOffset);

    // New:
    public static bool Overlaps<T>(this Memory<T> memory, ReadOnlyMemory<T> other);
    public static bool Overlaps<T>(this Memory<T> memory, ReadOnlyMemory<T> other, out int elementOffset);
    public static bool Overlaps<T>(this ReadOnlyMemory<T> memory, ReadOnlyMemory<T> other);
    public static bool Overlaps<T>(this ReadOnlyMemory<T> memory, ReadOnlyMemory<T> other, out int elementOffset);
}

}
```

@KrzysztofCwalina, any thoughts?

I think there is a set of Overlaps<T>(Memory<T> memory, ReadOnlySpan<T> span) missing, since with only those overloads, there isn't a good way to recover a Memory slice from a ReadOnlySpan (which is the most castable type that someone would make a method return, which is what prompted this suggestion). It's also a fairly opaque line of code to recover the memory, since, if I'm understanding correctly, one's code would need to be something like myMemory.Overlaps(someInnerSpan, out int offset) ? myMemory.Slice(offeset, someSpan.Length) : throw new SomeException(). That's a bit of a mouthful. Though I guess nothing is stopping a customer from writing the clearer extension method themselves as long as we have an appropriate set of Overlaps method.

since with only those overloads, there isn't a good way to recover a Memory slice from a ReadOnlySpan (which is the most castable type that someone would make a method return, which is what prompted this suggestion)

You could do that even with the already exposed Overlaps methods, no?
C# Memory<T> memory = ...; ReadOnlySpan<T> span = ...; bool overlapped = MemoryMarshal.Overlaps(memory.Span, span, out int elementOffset); ...

I hadn't thought about casting the first parameter to a Span to solve it. You're right. Though that makes the line even more exciting.

@bartonjs

Should we put this in the 3.0 milestone?

Video

  • Overlaps<T>() with memories. We concluded we don't want these overloads. We generally want to poeple to go from memory to span; we also believe they are fast enough. If we ever need them for perf reasons, we can revisit this.

  • SliceTo. We're unsure whether this API will fix the usability issues of the Overlap<T>() methods. For now, we don't think we want this API. However, we might want add a different API that will address these issues and makes working with spans when you need to compare bounds of other values (memories, other span, strings etc) based on how to two other spans relate to each other.

We could also just add another set of overloads to overlaps, like:

```cs
namespace System
{
public static class MemoryExtensions
{
// Existing:
// public static bool Overlaps(this Span span, ReadOnlySpan other);
// public static bool Overlaps(this Span span, ReadOnlySpan other, out int elementOffset);
// public static bool Overlaps(this ReadOnlySpan span, ReadOnlySpan other);
// public static bool Overlaps(this ReadOnlySpan span, ReadOnlySpan other, out int elementOffset);

    // New:
    public static bool Overlaps<T>(this Memory<T> memory, ReadOnlyMemory<T> other);
    public static bool Overlaps<T>(this Memory<T> memory, ReadOnlyMemory<T> other, out int elementOffset);
    public static bool Overlaps<T>(this ReadOnlyMemory<T> memory, ReadOnlyMemory<T> other);
    public static bool Overlaps<T>(this ReadOnlyMemory<T> memory, ReadOnlyMemory<T> other, out int elementOffset);
}

}

@terrajobst I don't know if this matters but in your existing API quotation you list the left side as "ReadOnlySpan" while the actual existing API is just a Span.

Was this page helpful?
0 / 5 - 0 ratings