Unsafe API for comparing byrefs as pointers

[jkotas]

S.R.CS.Unsafe should provide operations that allow comparing byrefs. These operations in combination with C# support for ref locals reassigments (Roslyn feature in progress - https://github.com/dotnet/csharplang/blob/master/proposals/ref-local-reassignment.md) will allow significantly more efficient unsafe implementations of low-level algorithms.

For example, Span Reverse prototype done by @GrabYourPitchforks showed up to 30% performance improvements (dotnet/corefx#26381 (comment)).

Proposed API:

public static class Unsafe
{
    // Returns true when left is pointing to lower memory address than right
    public static bool IsBelow<T>(ref T left, ref T right);

    // Returns true when left is pointing to higher memory address than right
    public static bool IsAbove<T>(ref T left, ref T right);
}

[jkotas]

cc @GrabYourPitchforks @KrzysztofCwalina @ahsonkhan

[benaadams]

Between?

public static bool IsBetween<T>(ref T left, ref T right, ref target);

[stephentoub]

Why ref bool rather than bool as the return value?

[jkotas]

ref bool

Copy&paste mistake. Updated above.

[ghost]

Could C# support the full set of comparison operators for ref types instead?

The way those names read is unfortunate: I mentally read out a line of code like:

`if (Unsafe.IsBelow(ref left, ref right))`

and I hear "if is below left" which is the opposite of what the conditional does...

[jkotas]

bool IsBetween

S.R.CS.Unsafe APIs are trying to have 1:1 mapping to IL instructions. IsBetween does not map to a single IL instruction. It is a convenience wrapper over pair of IsAbove+IsBelow calls.

Having said that, we have violated this design principle in a few places when it has a demonstrated benefits. Do you have a good case where IsBetween would help significantly?

[jkotas]

Could C# support the full set of comparison operators for ref types instead?

Adding operators for byrefs is hard to fit into C# design. This applies to all pointer-like operations, not just comparison: add/subtract, casting, ... . If it was not the case, S.R.CS.Unsafe would not be really needed.

I hear "if is below left" which is the opposite of what the conditional does

Can you think about a different name that would read better?

[ghost]

Can you think about a different name that would read better?

I'll throw in IsLeftBelowRight() as an opening bid. I'll see if I can brainstorm something more succinct but given Unsafe's low-level nature, and a really good solution probably requires an infix operator, I could live with this.

I'll assume a standard Compare method that returns +1/-1/0 is off the table (can't generate the same short IL...)

[benaadams]

Do you have a good case where IsBetween would help significantly?

Similar to the Span.IsSliceOf(Span<T> span) https://github.com/dotnet/corefx/issues/18750 which went on to be Overlaps dotnet/corefx#24980; but not necessarily using Span (though could be used for this single ref in this Span)

I don't have a strong use case

[GrabYourPitchforks]

The only argument for IsBetween that I can imagine is that it's a notoriously difficult check for the average developer to get correct. But at that point we're talking about callers that manipulate memory directly, and realistically I don't know who other than the BCL would ever be doing that.

[joshfree]

Can you think about a different name that would read better?

Something closer to Compare/CompareTo

IsLessThan(value1, value2)
IsGreatherThan(value1, value2)

[terrajobst]

Video

We think these names align closer to the framework naming:

public static class Unsafe
{
    public static bool IsAddressLessThan<T>(ref T left, ref T right);
    public static bool IsAddressGreaterThan<T>(ref T left, ref T right);
}

[morganbr]

Given that the GC can relocate the references during or after the call, how is possible to use this API correctly? If both references need to be pinned, we typically document that in the method name (similarly to GCHandle.AddrOfPinnedObject).

I'd also like to confirm that this API really is unsafe. The answer might be wrong if references aren't pinned, but the method won't corrupt memory.

[jkotas]

how is possible to use this API correctly

If the references are not pinned, you have to make sure that both point into same object or array.

I'd also like to confirm that this API really is unsafe.

You are right that this API is not going to corrupt your memory. Unsafe here really means that you have to know what you are doing. here are other similar APIs on this type like Unsafe.SizeOf<T>(). It is similar to unsafe context in C#: there is nothing strictly unsafe about sizeof(Foo), but C# compiler does not allow you to do that outside unsafe context in number of cases.

[KrzysztofCwalina]

It would be good to describe in the docs the things people need to know to use the API "safely".

[GrabYourPitchforks]

@KrzysztofCwalina See the devdoc at https://github.com/dotnet/corefx/blob/83f35ef9a79b5cd1412aeaad8d2cb84e1bfe73c4/src/System.Runtime.CompilerServices.Unsafe/src/System.Runtime.CompilerServices.Unsafe.xml#L211-L214. Do you think this is sufficient?

[morganbr]

@GrabYourPitchforks , is the API guaranteed to pin the inputs for the same array/object/span case? Otherwise a torn read could still give an undefined answer.

[jkotas]

@morganbr There is no risk of torn read. The GC moves all byrefs atomically.