Storage type of RedisValue from network - always RAW for "string" operations?

[petrroll]

It was my (&my coworkers) assumption that if RedisValue is constructed by the SE library (and comes from network / real redis) and contains result of an operation that essentially returns Redis string (i.e. sequence of byte[s]) it's internal storage type would always be raw.

To my surprise, that probably isn't always the case? While locally it seems to hold true, when we run against Azure Cache for Redis (without any proxy), we noticed that implicitly converting one of the (as it returns RedisValue[]) RedisValue that comes from SetMembersAsync to byte[] allocates and calls GetBytes and then allocates new byte[].

Looking at the code, it could be one of these two cases:

Any thoughts / guidance?

cc @mgravell

[mgravell]

It doesn't assume that you want a string, and for non-numeric values: byte[] is the lowest common denominator that can handle all values in a lossless way.

In scenarios where you need to control memory usage, the Lease methods are usually good choices - StringGetLeaseAsync etc. this gives you a buffer that is then pooled. Alternatively, when I finish the V3 rework, it should have hooks to allow a wide range of custom things at the caller's discretion, in advanced cases.

[petrroll]

I'm not sure I understand "it does r assume you want a string". I'm not suggesting that.

I'm explictely trying to get byte[] out of the redis value. And would expect it to not have to call GetBytes on intermediate string repre. As the data should come from wire us byte[]. No?

[mgravell]

As the data should come from wire us byte[]. No?

Not quite. When in the IO layer, the data is effectively a potentially multi-segment chain (ReadOnlySequence<byte>), from which we yank chunks. Looking at the 2.9 code for reading data as RedisValue, we currently have special-cases for null, RESP3 booleans, and values that appear to be pure integers RESP2/3 integers - with everything else becoming a byte[] copy, with this last case being Raw - however, even this choice is fairly arbitrary; in some experimental v3 code, I intentionally detect short ASCII non-control sequences and assume that the caller is most likely going to want that as a string, so to prevent duplication: we do that instead of the byte[].

I am, however, intrigued that you're seeing this fallback path be hit; for most non-integer values, yes: I would have expected to see the Raw path here - and for integer values: the Int64 path (the booleans are also treated as integers here). So: I am a little intrigued as to what storage kind we're actually seeing here. The following test passes, for example:

    [Fact]
    public async Task InvestigateIssue2952()
    {
        await using var conn = Create();

        var db = conn.GetDatabase();
        var key = Me();
        await db.KeyDeleteAsync(key);
        await db.SetAddAsync(key, ["abcdefgh", "ijklmnop", "qrstuvwx"]);
        var members = await db.SetMembersAsync(key);
        Assert.Equal(3, members.Length);
        var first = members[0];
        Assert.Equal("abcdefgh", (string)first!);
        var blob = (byte[])first!;
        Assert.True(blob.AsSpan().SequenceEqual("abcdefgh"u8));

        Assert.Equal(RedisValue.StorageType.Raw, first.Type);
        var raw = Assert.IsType<byte[]>(first.DirectObject);
        Assert.Same(raw, blob); // so: cast to byte[] wasn't a new alloc
    }

Happy to help investigate what you're seeing, but without more context, I'm not sure the cast is necessarily coming from this path. Maybe you could help me repro?

I'll also note that there exists .GetByteCount() and .CopyTo(Span<byte>) which can be used to efficiently copy a blob out of a RedisValue without needing to know the storage internals.

[mgravell]

Clarification: it looks like even for integers: in the v2 code, if they arrive as bulk-strings (which is what SMEMBERS returns), we again: treat them as Raw values. The v2 code only treats RESP integers more efficiently; the v3 code is more aggressive here! I have amended the previous with strike and correction.

[petrroll]

Yeah, when I run it locally I'm not reproing it either. The only evidence I have of it happening is this perf trace (auxilary events -> allocations) from prod / cloud machines.

[petrroll]

Apologies for taking your time. I found the culrprit. It was us accidentally foreaching with "string" instead of "var" which caused implicit coversion. Only for it then be implicitly converted to RedisValue again (hence I didn't notice).

My bad. And thanks for quick responses :).