Add (ReadOnly)Memory<byte> implicit conversions to RedisKey

[kevin-montrose]

At a high level, this just changes RedisKey.KeyPrefix to a ReadOnlyMemory (from a byte[]).

The change is slightly more complicated to deal with Prepend/Append.

Also noticed this line ; which looks like a place where prefix wouldn't be written? "Fixed" it and all tests still seem to pass.

[mgravell]

which looks like a place where prefix wouldn't be written?

well, historically I believe it was "value only" or "prefix+value" - it is your change that makes "prefix only" a thing; it bothers me a little that if we are using key-prefix (which is non-trivial usage - aspnet exposes it, for example), then this gets kicked into non-working; I had a separate design somewhere that refactored the value part of the key instead, using object+int+int fields, with type testing to support:

• string, char[], MemoryManager<char> (all handled as ReadOnlyMemory<char>)
• byte[], MemoryManager<byte> (all handled as ReadOnlyMemory<byte>)

I wonder if that's the way we should go here

[mgravell]

here was the core part from last time I looked at this:

using System.Buffers;
using System.Runtime.InteropServices;

Describe(default);

string s = "abcdefg";
Describe(s);
Describe(""); // should look like default
Describe(s.AsMemory());
Describe(s.AsMemory().Slice(2,3));

var bytes = new byte[42];
Describe(bytes);
Describe(Array.Empty<byte>()); // should look like default
Describe(bytes.AsMemory());
Describe(bytes.AsMemory().Slice(10, 3));

var chars = new char[42];
Describe(chars);
Describe(chars.AsMemory());
Describe(chars.AsMemory().Slice(10, 3));

// not shown: custom memory managers

static void Describe(Prototype value)
{
    if (value.TryGetValue(out ReadOnlyMemory<byte> bytes))
    {
        Console.WriteLine($"Value is {bytes.Length} bytes");
    }
    if (value.TryGetValue(out ReadOnlyMemory<char> text))
    {
        Console.WriteLine($"Value is {text.Length} chars");
    }
    Console.WriteLine("---");
    Console.WriteLine();
}
readonly struct Prototype
{
    private readonly object? _obj;
    private readonly int _start, _length;

    private Prototype(object? obj, int start, int length)
    {
        if (length is 0)
        {
            // don't track the obj, then
            this = default;
        }
        else
        {
            _obj = obj;
            _start = start;
            _length = length;
        }
    }
    public static implicit operator Prototype(string value) => new(value, 0, value is null ? 0 : value.Length);

    public static implicit operator Prototype(byte[] value) => new(value, 0, value is null ? 0 : value.Length);
    public static implicit operator Prototype(char[] value) => new(value, 0, value is null ? 0 : value.Length);

    public static implicit operator Prototype(Memory<char> value) => (Prototype)(ReadOnlyMemory<char>)value;
    public static implicit operator Prototype(ReadOnlyMemory<char> value)
    {
        // string and array are pretty common
        if (MemoryMarshal.TryGetString(value, out var text, out int start, out int length))
        {
            return new Prototype(text, start, length);
        }
        if (MemoryMarshal.TryGetArray(value, out var segment))
        {
            return new Prototype(segment.Array, segment.Offset, segment.Count);
        }
        // this is pretty rare
        if (MemoryMarshal.TryGetMemoryManager(value, out MemoryManager<char>? mgr, out start, out length))
        {
            return new Prototype(mgr, start, length);
        }
        // we don't expect this
        return Fail();
    }

    public static implicit operator Prototype(Memory<byte> value) => (Prototype)(ReadOnlyMemory<byte>)value;
    public static implicit operator Prototype(ReadOnlyMemory<byte> value)
    {
        // array is pretty common
        if (MemoryMarshal.TryGetArray(value, out var segment))
        {
            return new Prototype(segment.Array, segment.Offset, segment.Count);
        }
        // this is pretty rare
        if (MemoryMarshal.TryGetMemoryManager(value, out MemoryManager<byte>? mgr, out var start, out var length))
        {
            return new Prototype(mgr, start, length);
        }
        // we don't expect this
        return Fail();
    }
    private static Prototype Fail() => throw new InvalidOperationException("Unexpected ReadOnlyMemory<> scenario; please report this.");

    public bool TryGetValue(out ReadOnlyMemory<byte> value)
    {
        if (_length == 0)
        {
            value = default;
            return true;
        }
        switch (_obj)
        {
            case byte[] arr:
                value = new ReadOnlyMemory<byte>(arr, _start, _length);
                return true;
            case MemoryManager<byte> mgr:
                value = mgr.Memory.Slice(_start, _length);
                return true;
            default:
                value = default;
                return false;
        }
    }
    public bool TryGetValue(out ReadOnlyMemory<char> value)
    {
        if (_length == 0)
        {
            value = default;
            return true;
        }
        switch (_obj)
        {
            case string text:
                value = text.AsMemory().Slice(_start, _length);
                return true;
            case char[] arr:
                value = new ReadOnlyMemory<char>(arr, _start, _length);
                return true;
            case MemoryManager<char> mgr:
                value = mgr.Memory.Slice(_start, _length);
                return true;
            default:
                value = default;
                return false;
        }
    }
}

[mgravell]

btw, I'm not saying "you should go do this" - this is the what I'm more inclined towards, but that doesn't make it your problem

[kevin-montrose]

which looks like a place where prefix wouldn't be written?

well, historically I believe it was "value only" or "prefix+value" - it is your change that makes "prefix only" a thing

Ah, I think that's true - I was just looking at the signatures.

[kevin-montrose]

Oh, also if you're thinking deep thoughts about keys and values...

Some way (or documentation, if these ways exist) to get returned values backed by rented byte[]s would also be handy. Got a couple cases where GC pauses are murder, so slapping an ArrayPool in there would be nice. I'd call it a pretty advanced scenario, so the consumer would be responsible for actually returning rented arrays IMO.

[mgravell]

@kevin-montrose there is a "get lease" API for redis strings, which might be what you mean? Or do you mean the keys?

[kevin-montrose]

@kevin-montrose there is a "get lease" API for redis strings, which might be what you mean? Or do you mean the keys?

@mgravell YUP, that'll do - couldn't find it (was looking for "pool" or something), and seems basically undocumented?

Ideally we'd even be able to reuse the Lease... but that's maybe overkill.