Move Span.NonPortableCast to MemoryMarshal and rename to Cast

[ahsonkhan]

From https://github.com/dotnet/corefx/issues/26139#issuecomment-358090527:

Move and rename NonPortableCast to MemoryMarshal.Cast

Current API:

namespace System
{
    public static partial class MemoryExtensions
    {
        public static ReadOnlySpan<TTo> NonPortableCast<TFrom, TTo>(this ReadOnlySpan<TFrom> source) where TFrom : struct where TTo : struct { throw null; }
        public static Span<TTo> NonPortableCast<TFrom, TTo>(this Span<TFrom> source) where TFrom : struct where TTo : struct { throw null; }
    }
}

Proposed API:

namespace System.Runtime.InteropServices
{
    public static partial class MemoryMarshal
    {
        public static bool TryCast<TFrom, TTo>(ReadOnlySpan<TFrom> source, out ReadOnlySpan<TTo> output) where TFrom : struct where TTo : struct { throw null; }
        public static bool TryCast<TFrom, TTo>(Span<TFrom> source, out Span<TTo> output) where TFrom : struct where TTo : struct { throw null; }
    }
}

Edit: Removed this
Edit by @ianhays: Changed Cast to TryCast per discussion

cc @jkotas, @KrzysztofCwalina

[jkotas]

The API on MemoryMarshal should not be an extension method.

[KrzysztofCwalina]

@jkotas, what's the code we recommend people write when they want to use this API in a reusable library? i.e. How should they check if the cast is safe or not? If it turns out it's not, what should the fallback code do?

[jkotas]

what's the code we recommend people write when they want to use this API in a reusable library?

Options:

• Guarantee the alignment by pinning the managed memory or using unmanaged memory
• Have platform checks, e.g. if I am on Intel, can use faster path that assumes misaligned memory access is ok

If it turns out it's not, what should the fallback code do?

Options:

• Read one byte at a time. It is similar to what you do for BigEndian portability.
• Use BinaryPrimitives helper methods.
• Create aligned copy first

[KrzysztofCwalina]

A couple more questions:

1. Is it really as simple as ARM vs Intel? Based on my reading it's only a severe problem in ARM7 and older (which are not produced since 2006). On ARM7 (and even better 11), the unaligned reads seem to be 2x in worst case and in many cases have no impact.
2. Wouldn't reading one byte at a time be a cure worse than the disease? It seems like it would be even slower in most cases. Similarly with the other suggestions.

[jkotas]

Is it really as simple as ARM vs Intel?

For .NET Core today - yes. For Mono - the matrix is much larger.

ARM9 and older (which are not produced since 2006)

Are you sure that they are not produced? I think ARM stopped iterating on the base line designs sometime around 2006, but I believe these older ARM version are still produced in huge quantities, e.g. RPi0, Lego Mindstorm, ... .

On ARM11, the unaligned reads seem to be 2x in worst case and in many cases have no impact.

Unless you pass the unaligned memory to the OS, read floating point values from it, or perform interlocked operations on it - I believe all these fault even on latest ARM. (The list may be incomplete.)

Wouldn't reading one byte at a time be a cure worse than the disease?

It is case-by-case. Many places read one byte a time today and they work just fine. The reading one byte at a time is far from being a performance bottleneck in many cases.

[jkotas]

For example, we have switched System.Reflection.Metadata to read one byte a time in most places to make it portable and uses special paths in a few bulkier places : dotnet/corefx#16583 . The performance impact of this change was non-measurable.

[KrzysztofCwalina]

From what I read, ARM7 can in general read unaligned, but some read instructions cannot. ARM9+ all instructions support unaligned reads, but the feature can be turned on/off (at processor level) and so software cannot assume any given settings. Supposedly most OSes have the setting on, but some don't.

[KrzysztofCwalina]

I kind of wish we made it possible to write

Span<byte> bytes = ...
if(!bytes.CanCast<int>()){
    bytes = bytes.ToArray();
}
Span<int> ints = bytes.Cast<int>(); // fails is misaligned on all processors

[jkotas]

Why not bool TryCast(Span<TFrom> from, out Span<TTo> to) ?

[jkotas]

ARM9+ all instructions support unaligned reads

I do not think it is the case. My copy of the ARM manual has a big table in chapter A3.2.1 Unaligned data access that shows what works and what does not.

[ahsonkhan]

Why not bool TryCast(Span<TFrom> from, out Span<TTo> to) ?

How would we discern when to return false (i.e. whether the platform supports misaligned memory access) without significant perf regression?

[jkotas]

For slow span, it would be slow like everything else. For fast span, it would need to be special crafted in the runtime per platform to be fast.

[jkotas]

Just noticed a problem with this:

if(!bytes.CanCast<int>()){
    bytes = bytes.ToArray();
}
Span<int> ints = bytes.Cast<int>(); 

To be portable, you would rather want allocate array of the destination type and then copy the bytes to it. The array of bytes may not have sufficient alignment for the destination type.

[KrzysztofCwalina]

I like the idea of TryCast.

[ianhays]

I like TryCast too. Updated the proposal.

[KrzysztofCwalina]

so, the idiomatic code would be:

Span<int> ints;
if(!bytes.TryCast(out ints)){
    ints = new int[bytes.Length/sizeof(int)];
    bytes.Slice(0, ints.Length * sizeof(int)).CopyTo(ints.AsBytes()); 
}

As a side note, I again wish out Copyto copied as much as it could and returned the number of items copied as opposed to demanding the right fit, i.e.

Span<int> ints;
if(!bytes.TryCast(out ints)){
    ints = new int[bytes.Length/sizeof(int)];
    bytes.CopyTo(ints.AsBytes); 
}

[ianhays]

It wouldn't be an extension method if its in MemoryMarshal though, so an example would be more like:

Span<int> ints;
if(!MemoryMarshal.TryCast<byte, int>(bytes, out ints)){
    ints = new int[bytes.Length/sizeof(int)];
    bytes.Slice(0, ints.Length * sizeof(int)).CopyTo(ints.AsBytes()); 
}

[ianhays]

Edit: removed.

[terrajobst]

Video

Looks good as proposed. We can add a more specialized API that handles the else case if we we want to. Only difference from the proposal is to leave it on MemoryExtensions as the API is now safe.

namespace System
{
    public static partial class MemoryExtensions
    {
        public static bool TryCast<TFrom, TTo>(this ReadOnlySpan<TFrom> source, out ReadOnlySpan<TTo> output) where TFrom : struct where TTo : struct;
        public static bool TryCast<TFrom, TTo>(this Span<TFrom> source, out Span<TTo> output) where TFrom : struct where TTo : struct;
    }
}

We'll also leave the NonPortableCast API but we'll move it MemoryMarshal. We need to make sure it's not an extension method, under which case we can rename it to just Cast.

[ianhays]

I split the TryCast addition to https://github.com/dotnet/corefx/issues/26465 to make tracking easier.

This issue will track just the move and rename of NonPortableCast.