Optimize RedisValue / reader logic by using canonical parser and new ShortBlob storage kind (#3114)

* spike better RedisValue parsing logic

* Introduce new storage kind for small payloads (<=8 bytes) - avoids array alloc

* fix CI

* CI: tweak failing test

* tidying (support ulong as canonical)

Author: mgravell

src/StackExchange.Redis/MessageWriter.cs

@@ -99,8 +99,8 @@ internal static void WriteBulkString(in RedisValue value, IBufferWriter<byte> wr
             case RedisValue.StorageType.String:
                 WriteUnifiedPrefixedString(writer, null, value.RawString());
                 break;
-            case RedisValue.StorageType.MemoryManager or RedisValue.StorageType.ByteArray:
-                WriteUnifiedSpan(writer, value.RawSpan());
+            case RedisValue.StorageType.MemoryManager or RedisValue.StorageType.ByteArray or RedisValue.StorageType.ShortBlob:
+                WriteUnifiedSpan(writer, value.UnsafeRawSpan(out _));
                 break;
             case RedisValue.StorageType.Sequence:
                 WriteUnifiedSequenceIterator(writer, value.RawSequenceIterator());

src/StackExchange.Redis/ReadOnlySequenceExtensions.cs

@@ -184,4 +184,33 @@ public static int SequenceCompareTo(this in ReadOnlySequence<byte> first, in Rea
         // everything in the overlap matched, so the longer sequence sorts after the shorter
         return first.Length.CompareTo(other.Length);
     }
+
+    /// <summary>
+    /// Lexicographically compares a sequence against a contiguous span (same semantics as the
+    /// sequence-vs-sequence overload).
+    /// </summary>
+    public static int SequenceCompareTo(this in ReadOnlySequence<byte> first, ReadOnlySpan<byte> other)
+    {
+        if (first.IsSingleSegment) return first.FirstSpan.SequenceCompareTo(other);
+
+        long firstLength = first.Length;
+        int otherLength = other.Length;
+        var firstPos = first.Start;
+        ReadOnlySpan<byte> a = default;
+        while (true)
+        {
+            while (a.IsEmpty && first.TryGet(ref firstPos, out var aNext)) a = aNext.Span;
+            if (a.IsEmpty || other.IsEmpty) break;
+
+            var shared = Math.Min(a.Length, other.Length);
+            var cmp = a.Slice(0, shared).SequenceCompareTo(other.Slice(0, shared));
+            if (cmp != 0) return cmp;
+
+            a = a.Slice(shared);
+            other = other.Slice(shared);
+        }
+
+        // overlap matched, so the longer input sorts after the shorter
+        return firstLength.CompareTo((long)otherLength);
+    }
 }

src/StackExchange.Redis/RedisValue.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Buffers;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 using RESPite.Messages;
 
@@ -15,12 +16,43 @@ public RedisValue ReadRedisValue()
             reader.DemandScalar();
             if (reader.IsNull) return RedisValue.Null;
 
-            return reader.Prefix switch
+            switch (reader.Prefix)
             {
-                RespPrefix.Boolean => reader.ReadBoolean(),
-                RespPrefix.Integer => reader.ReadInt64(),
-                _ => reader.ReadByteArray(),
-            };
+                case RespPrefix.Boolean:
+                    return reader.ReadBoolean();
+                case RespPrefix.Integer:
+                    return reader.ReadInt64();
+            }
+
+            // bulk/simple/verbatim string. Only inline (non-streaming) scalars get the compact storage
+            // kinds; streaming scalars fall through to ReadByteArray.
+            if (reader.IsInlineScalar)
+            {
+                var length = reader.ScalarLength();
+
+                // Short payloads (<= 8 bytes) pack inline as a short-blob: allocation-free, and with *no*
+                // eager numeric parse - any later (long)/(double)/etc. is deferred to the caller (Simplify
+                // on demand), which is cheaper for the common case of values never interpreted as numbers.
+                // Contiguous data (the common case) is taken straight from TryGetSpan - no stackalloc. Only a
+                // scalar that straddles segments needs linearizing into the 8-byte stack buffer; the length
+                // guard is what makes that fixed buffer safe, since Buffer() silently truncates an over-long
+                // discontiguous payload.
+                if (length <= RedisValue.MaxInlineBytes)
+                {
+                    return RedisValue.FromRaw(reader.TryGetSpan(out var buffer) ?
+                        buffer : reader.Buffer(stackalloc byte[RedisValue.MaxInlineBytes]));
+                }
+
+                // Longer payloads: prefer a compact numeric storage kind when the text is the *canonical*
+                // representation of that number, so every projection (ToString, (byte[]), equality, hash)
+                // still round-trips byte-for-byte; this also avoids the byte[] alloc. Canonical parsing needs
+                // a contiguous span, so a discontiguous payload falls through to ReadByteArray.
+                if (reader.TryGetSpan(out var span) && TryReadCanonicalNumber(span, out var number))
+                {
+                    return number;
+                }
+            }
+            return reader.ReadByteArray();
         }
 
         public string DebugReadTruncatedString(int maxChars)
@@ -184,7 +216,7 @@ public static RespPrefix GetRespPrefix(ReadOnlySpan<byte> frame)
             RespPrefix.SimpleError => ResultType.Error,
             RespPrefix.Null => ResultType.Null,
             RespPrefix.VerbatimString => ResultType.VerbatimString,
-            RespPrefix.Push=> ResultType.Push,
+            RespPrefix.Push => ResultType.Push,
             _ => throw new ArgumentOutOfRangeException(nameof(prefix), prefix, null),
         };
     }
@@ -208,4 +240,92 @@ internal bool AnyNull()
         public bool IsCompletedSuccessfully => task.Status is TaskStatus.RanToCompletion;
     }
 #endif
+
+    private static readonly int MaxCanonicalLength = Math.Max(Format.MaxInt64TextLen, Format.MaxDoubleTextLen);
+
+    // Recognizes the canonical decimal text of an integer (signed Int64 or, for non-negative values up to
+    // ulong.MaxValue, UInt64) or a finite double, returning a numeric-backed RedisValue only when re-formatting
+    // the parsed value reproduces the exact input bytes. This keeps the optimization invisible to callers:
+    // non-canonical spellings ("01234", "+5", "1.50", "1e3"), values beyond the ulong/double range, and the
+    // special inf/nan tokens all return false and are kept as a byte[] payload by the caller.
+    private static bool TryReadCanonicalNumber(ReadOnlySpan<byte> span, out RedisValue value)
+    {
+        static bool Failure(out RedisValue value)
+        {
+            value = default;
+            return false;
+        }
+        // integer: canonical exactly when the round-trip text length matches (rules out leading zeros, a
+        // leading '+', "-0", trailing junk, etc.) - so no need to re-emit and compare bytes
+        if (span.IsEmpty | span.Length > MaxCanonicalLength) return Failure(out value);
+
+        // restrict to *just* basic number tokens, tracking the two facts that let us pick a single parse:
+        // whether a '-' appeared (so we know whether the integer is signed), and whether a '.'/'e'/'E'
+        // appeared (which rules out an integer entirely - only a double can be canonical)
+        bool seenNegative = false, seenDotOrExp = false;
+        foreach (var b in span)
+        {
+            switch (b)
+            {
+                case (byte)'0':
+                case (byte)'1':
+                case (byte)'2':
+                case (byte)'3':
+                case (byte)'4':
+                case (byte)'5':
+                case (byte)'6':
+                case (byte)'7':
+                case (byte)'8':
+                case (byte)'9':
+                    break;
+                case (byte)'-':
+                    seenNegative = true;
+                    break;
+                case (byte)'.':
+                case (byte)'E':
+                case (byte)'e':
+                    seenDotOrExp = true;
+                    break;
+                default:
+                    return Failure(out value);
+            }
+        }
+
+        if (!seenDotOrExp)
+        {
+            // pure integer text. For a non-negative value parse as *unsigned* so the full ulong range is
+            // covered; the RedisValue(ulong) ctor demotes to Int64 storage when the value fits, so smaller
+            // values still land as Int64 exactly as before. A negative value can only be Int64.
+            if (seenNegative)
+            {
+                if (Format.TryParseInt64(span, out var i64) && Format.MeasureInt64(i64) == span.Length)
+                {
+                    value = i64;
+                    return true;
+                }
+            }
+            else if (Format.TryParseUInt64(span, out var u64) && Format.MeasureUInt64(u64) == span.Length)
+            {
+                value = u64;
+                return true;
+            }
+
+            // note that all-digit text which isn't a canonical integer (oversize, leading zero, etc.) can't
+            // be a canonical double either - any such value re-formats with an exponent - so we simply fall
+            // through to the failure at the bottom rather than attempting a (futile) double parse
+        }
+        else if (Format.TryParseDouble(span, out var dbl))
+        {
+            if (dbl == 0.0) dbl = Math.Abs(dbl); // prevent problems with -0 formatting
+            Span<byte> formatted = stackalloc byte[Format.MaxDoubleTextLen];
+            var len = Format.FormatDouble(dbl, formatted);
+            if (formatted.Slice(0, len).SequenceEqual(span))
+            {
+                value = dbl;
+                return true;
+            }
+        }
+
+        return Failure(out value);
+    }
 }

src/StackExchange.Redis/RespReaderExtensions.cs

@@ -8,13 +8,13 @@ namespace StackExchange.Redis.Tests.Issues;
 public class Issue1103Tests(ITestOutputHelper output) : TestBase(output)
 {
     [Theory]
-    [InlineData(142205255210238005UL, (int)StorageType.Int64)]
-    [InlineData(ulong.MaxValue, (int)StorageType.UInt64)]
-    [InlineData(ulong.MinValue, (int)StorageType.Int64)]
-    [InlineData(0x8000000000000000UL, (int)StorageType.UInt64)]
-    [InlineData(0x8000000000000001UL, (int)StorageType.UInt64)]
-    [InlineData(0x7FFFFFFFFFFFFFFFUL, (int)StorageType.Int64)]
-    public async Task LargeUInt64StoredCorrectly(ulong value, int storageType)
+    [InlineData(142205255210238005UL, (int)StorageType.Int64, (int)StorageType.Int64)]
+    [InlineData(ulong.MaxValue, (int)StorageType.UInt64, (int)StorageType.UInt64)] // 20-byte canonical uint => UInt64 on read
+    [InlineData(ulong.MinValue, (int)StorageType.Int64, (int)StorageType.ShortBlob)]
+    [InlineData(0x8000000000000000UL, (int)StorageType.UInt64, (int)StorageType.UInt64)] // long.MaxValue+1: 19-byte canonical uint => UInt64 on read
+    [InlineData(0x8000000000000001UL, (int)StorageType.UInt64, (int)StorageType.UInt64)]
+    [InlineData(0x7FFFFFFFFFFFFFFFUL, (int)StorageType.Int64, (int)StorageType.Int64)] // long.MaxValue: 19-byte canonical int => Int64 on read
+    public async Task LargeUInt64StoredCorrectly(ulong value, int storageType, int fromRedisType)
     {
         await using var conn = Create();
 
@@ -29,13 +29,13 @@ public async Task LargeUInt64StoredCorrectly(ulong value, int storageType)
         var fromRedis = db.StringGet(key);
 
         Log($"{fromRedis.Type}: {fromRedis}");
-        Assert.Equal(StorageType.ByteArray, fromRedis.Type);
+        Assert.Equal((StorageType)fromRedisType, fromRedis.Type);
         Assert.Equal(value, (ulong)fromRedis);
         Assert.Equal(value.ToString(CultureInfo.InvariantCulture), fromRedis.ToString());
 
         var simplified = fromRedis.Simplify();
         Log($"{simplified.Type}: {simplified}");
-        Assert.Equal((StorageType)storageType, typed.Type);
+        Assert.Equal((StorageType)storageType, simplified.Type);
         Assert.Equal(value, (ulong)simplified);
         Assert.Equal(value.ToString(CultureInfo.InvariantCulture), fromRedis.ToString());
     }