From 3a345c98864591f96eade51e6cfba1d23c9080fb Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 25 Jun 2026 16:59:37 +0100 Subject: [PATCH 1/5] spike better RedisValue parsing logic --- .../RespReaderExtensions.cs | 86 +++++++++++++++++-- .../RedisValueStorageKindTests.cs | 52 +++++++++++ 2 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs diff --git a/src/StackExchange.Redis/RespReaderExtensions.cs b/src/StackExchange.Redis/RespReaderExtensions.cs index 6f80c57a44..2ba1ea57f4 100644 --- a/src/StackExchange.Redis/RespReaderExtensions.cs +++ b/src/StackExchange.Redis/RespReaderExtensions.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,22 @@ 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: for inline scalars, 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; otherwise keep the raw bytes + if (reader.TryGetSpan(out var span) && TryReadCanonicalNumber(span, out var number)) + { + return number; + } + return reader.ReadByteArray(); } public string DebugReadTruncatedString(int maxChars) @@ -184,7 +195,7 @@ public static RespPrefix GetRespPrefix(ReadOnlySpan 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 +219,67 @@ 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 or 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"), oversize + // values, and the special inf/nan tokens all return false and are kept as a byte[] payload by the caller. + private static bool TryReadCanonicalNumber(ReadOnlySpan 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 + 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': + case (byte)'.': + case (byte)'-': + case (byte)'E': + case (byte)'e': + break; + default: + return Failure(out value); + } + } + + if (Format.TryParseInt64(span, out var i64) && Format.MeasureInt64(i64) == span.Length) + { + value = i64; + return true; + } + + if (Format.TryParseDouble(span, out var dbl)) + { + if (dbl == 0.0) dbl = Math.Abs(dbl); // prevent problems with -0 formatting + Span 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); + } } diff --git a/tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs b/tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs new file mode 100644 index 0000000000..b3490b4600 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs @@ -0,0 +1,52 @@ +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// Pins the that the inbound read path +/// (RespReader.ReadRedisValue) produces for values fetched from a server: numeric-looking bulk +/// strings are stored compactly as Int64/Double when they round-trip exactly, otherwise as a byte[]. +/// +/// +/// The expected storage kind is passed by name because is internal and +/// so cannot appear as a parameter on a public theory method. +/// +public class RedisValueStorageKindTests(ITestOutputHelper output, InProcServerFixture fixture) : TestBase(output, fixture) +{ + [Theory] + // canonical integers => Int64 + [InlineData("1234", "Int64")] + [InlineData("0", "Int64")] + [InlineData("-5", "Int64")] + [InlineData("9223372036854775807", "Int64")] // long.MaxValue + // canonical finite doubles => Double + [InlineData("0.5", "Double")] + [InlineData("-2.25", "Double")] + // non-canonical / non-numeric / special => kept as bytes + [InlineData("01234", "ByteArray")] // leading zero + [InlineData("+1234", "ByteArray")] // leading '+' + [InlineData("-0", "ByteArray")] // negative zero text + [InlineData("1.50", "ByteArray")] // trailing zero (non-canonical double) + [InlineData("1e3", "ByteArray")] // exponent form + [InlineData("inf", "ByteArray")] // special token excluded + [InlineData("nan", "ByteArray")] + [InlineData("99999999999999999999999", "ByteArray")] // oversize + [InlineData("abc", "ByteArray")] // not numeric + [InlineData("12abc", "ByteArray")] // trailing junk + public async Task FetchedValue_StorageKindAndRoundTrip(string stored, string expectedKind) + { + await using var conn = Create(); + var db = conn.GetDatabase(); + var key = Me(); + db.KeyDelete(key, CommandFlags.FireAndForget); + + db.StringSet(key, stored); + var value = db.StringGet(key); + + // the optimization must be invisible: the text projection always matches what was stored + Assert.Equal(stored, (string?)value); + Log($"'{stored}' => {value.Type}"); + Assert.Equal(expectedKind, value.Type.ToString()); + } +} From ef2db575f7c2193ec4de0db0b3cbc856cd12d383 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 Jun 2026 13:27:20 +0100 Subject: [PATCH 2/5] Introduce new storage kind for small payloads (<=8 bytes) - avoids array alloc --- src/StackExchange.Redis/MessageWriter.cs | 4 +- .../ReadOnlySequenceExtensions.cs | 29 ++ src/StackExchange.Redis/RedisValue.cs | 255 ++++++++++++------ .../RespReaderExtensions.cs | 31 ++- .../RedisValueShortBlobTests.cs | 166 ++++++++++++ .../RedisValueStorageKindTests.cs | 52 ---- .../RedisValueStorageKindUnitTests.cs | 109 ++++++++ 7 files changed, 512 insertions(+), 134 deletions(-) create mode 100644 tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs delete mode 100644 tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs create mode 100644 tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs diff --git a/src/StackExchange.Redis/MessageWriter.cs b/src/StackExchange.Redis/MessageWriter.cs index 95dea4ea71..a95edd7769 100644 --- a/src/StackExchange.Redis/MessageWriter.cs +++ b/src/StackExchange.Redis/MessageWriter.cs @@ -99,8 +99,8 @@ internal static void WriteBulkString(in RedisValue value, IBufferWriter 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()); diff --git a/src/StackExchange.Redis/ReadOnlySequenceExtensions.cs b/src/StackExchange.Redis/ReadOnlySequenceExtensions.cs index fbedf6be91..303947cbc6 100644 --- a/src/StackExchange.Redis/ReadOnlySequenceExtensions.cs +++ b/src/StackExchange.Redis/ReadOnlySequenceExtensions.cs @@ -184,4 +184,33 @@ public static int SequenceCompareTo(this in ReadOnlySequence first, in Rea // everything in the overlap matched, so the longer sequence sorts after the shorter return first.Length.CompareTo(other.Length); } + + /// + /// Lexicographically compares a sequence against a contiguous span (same semantics as the + /// sequence-vs-sequence overload). + /// + public static int SequenceCompareTo(this in ReadOnlySequence first, ReadOnlySpan other) + { + if (first.IsSingleSegment) return first.FirstSpan.SequenceCompareTo(other); + + long firstLength = first.Length; + int otherLength = other.Length; + var firstPos = first.Start; + ReadOnlySpan 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); + } } diff --git a/src/StackExchange.Redis/RedisValue.cs b/src/StackExchange.Redis/RedisValue.cs index 33adb1b874..ffa8412671 100644 --- a/src/StackExchange.Redis/RedisValue.cs +++ b/src/StackExchange.Redis/RedisValue.cs @@ -1,5 +1,6 @@ using System; using System.Buffers; +using System.Buffers.Binary; using System.Buffers.Text; using System.ComponentModel; using System.Diagnostics; @@ -19,9 +20,16 @@ namespace StackExchange.Redis [StructLayout(LayoutKind.Explicit)] public readonly struct RedisValue : IEquatable, IComparable, IComparable, IConvertible { - // Note that this allocates a byte[]; fine when used for a static field from a u8 literal, but - // not a good option to call every time. - internal static RedisValue FromRaw(ReadOnlySpan bytes) => bytes.ToArray(); + // Maximum payload that fits in an inline short-blob (packed into the overlapped int64 field). + internal const int MaxInlineBytes = sizeof(long); + + // Prefers an inline (allocation-free) short-blob for <= 8 bytes; otherwise materializes a byte[]. + internal static RedisValue FromRaw(ReadOnlySpan bytes) + { + if (bytes.IsEmpty) return EmptyString; + if (bytes.Length <= MaxInlineBytes) return new RedisValue(bytes); + return bytes.ToArray(); + } internal static readonly RedisValue[] EmptyArray = Array.Empty(); @@ -56,6 +64,19 @@ private RedisValue(byte[]? value) } } + // inline short-blob (1..8 bytes): bytes are packed into the overlapped _valueInt64 field, length is + // carried by the ShortBlob sentinel. Read/write the bytes via the raw memory layout (MemoryMarshal) + // so it is endianness-agnostic - we never interpret _valueInt64 as a number for this kind. + private unsafe RedisValue(ReadOnlySpan shortBlob) + { + Debug.Assert(shortBlob.Length is > 0 and <= ShortBlob.MaxLength, "short-blob length out of range"); + Unsafe.SkipInit(out this); + long packed = 0; // zero so the unused high bytes are deterministic + shortBlob.CopyTo(new Span(Unsafe.AsPointer(ref packed), sizeof(long))); + _valueInt64 = packed; + _obj = ShortBlob.For(shortBlob.Length); + } + private RedisValue(ReadOnlyMemory value) { Unsafe.SkipInit(out this); @@ -236,8 +257,9 @@ public bool IsNullOrEmpty { get { - // primitives are never null - if (_obj == Sentinel_Double || _obj == Sentinel_SignedInteger || _obj == Sentinel_UnsignedInteger) return false; + // primitives are never null; a short-blob is by construction always 1..8 bytes (and its + // _length field is unusable anyway, as it overlaps the inline bytes) + if (_obj == Sentinel_Double | _obj == Sentinel_SignedInteger | _obj == Sentinel_UnsignedInteger | _obj is ShortBlob) return false; // everything else either null or a buffer or some kind; can use length return _length == 0; } @@ -277,7 +299,7 @@ internal ReadOnlySequenceSegmentIterator RawSequenceIterator() return default; } - internal ReadOnlySequence RawSequence() + private ReadOnlySequence RawSequence() { if (_obj is ReadOnlySequenceSegment s) return GetSequence(s, _index, _length); if (_obj is byte[] a) return new(a, _index, _length); @@ -302,14 +324,90 @@ private ReadOnlySpan CopyRawSequence(Span destination) return destination.Slice(0, offset); } - internal ReadOnlySpan RawSpan() + // Returns a span over the bytes of any contiguous-blob kind (ByteArray/MemoryManager/ShortBlob). + // For a ShortBlob the bytes are unpacked into 'stackStorage', so the caller MUST keep 'stackStorage' + // alive (it must be a genuine stack local) for as long as it uses the returned span - the span aliases + // that slot. For heap blobs 'stackStorage' is left untouched and the span points at the heap, so a + // discard ('out _') is always fine there. + // + // "Unsafe" because the contract is unstated in the type system: on older TFMs the ShortBlob span is + // built over a *raw* pointer to 'stackStorage', so passing a ref to a movable location (e.g. a field + // on a heap object) is undefined behaviour - the GC may relocate it out from under the span. On NET + // we keep a managed pointer throughout (CreateReadOnlySpan), which the GC tracks, removing that hazard. + internal +#if !NET + unsafe +#endif + ReadOnlySpan UnsafeRawSpan(out long stackStorage) { + if (_obj is ShortBlob sb) + { + stackStorage = _valueInt64; +#if NET + return MemoryMarshal.CreateReadOnlySpan(ref Unsafe.As(ref stackStorage), sb.Length); +#else + return new ReadOnlySpan(Unsafe.AsPointer(ref stackStorage), sb.Length); +#endif + } + // heap path: 'stackStorage' is unused, so skip the redundant zero-init + Unsafe.SkipInit(out stackStorage); if (_obj is byte[] b) return new ReadOnlySpan(b, _index, _length); if (_obj is MemoryManager m) return m.GetSpan().Slice(_index, _length); ThrowRawType(); return default; } + // logical byte length of any contiguous-blob kind (ByteArray/MemoryManager/ShortBlob) + private int BlobLength => _obj is ShortBlob sb ? sb.Length : _length; + + // true for the byte-backed storage kinds (everything that compares "by bytes") + private static bool IsBlob(StorageType type) + => type is StorageType.ByteArray or StorageType.MemoryManager or StorageType.ShortBlob or StorageType.Sequence; + + // byte-wise equality between any two byte-backed values, in any combination of contiguous/sequence + private static bool BlobSequenceEqual(in RedisValue x, in RedisValue y) + { + // at most two stack slots back the inline short-blobs; reuse named locals rather than relying + // on the compiler to coalesce per-call 'out _' temps + long xScratch, yScratch; + if (x.Type == StorageType.Sequence) + { + return y.Type == StorageType.Sequence + ? x.RawSequence().SequenceEqual(y.RawSequence()) + : x.RawSequence().SequenceEqual(y.UnsafeRawSpan(out yScratch)); + } + if (y.Type == StorageType.Sequence) + { + return y.RawSequence().SequenceEqual(x.UnsafeRawSpan(out xScratch)); + } + return x.UnsafeRawSpan(out xScratch).SequenceEqual(y.UnsafeRawSpan(out yScratch)); + } + + // byte-wise ordinal comparison between any two byte-backed values, in any combination of + // contiguous (ByteArray/MemoryManager/ShortBlob) and multi-segment (Sequence) + private static int BlobCompareTo(in RedisValue x, in RedisValue y) + { + long xScratch, yScratch; // at most two stack slots; reuse named locals (see BlobSequenceEqual) + var xSeq = x.Type == StorageType.Sequence; + var ySeq = y.Type == StorageType.Sequence; + if (xSeq && ySeq) return x.RawSequence().SequenceCompareTo(y.RawSequence()); + if (xSeq) return x.RawSequence().SequenceCompareTo(y.UnsafeRawSpan(out yScratch)); + if (ySeq) return -y.RawSequence().SequenceCompareTo(x.UnsafeRawSpan(out xScratch)); // negate: computed y vs x + return x.UnsafeRawSpan(out xScratch).SequenceCompareTo(y.UnsafeRawSpan(out yScratch)); + } + + // true if 'whole' starts with the bytes of 'prefix', for any combination of byte-backed kinds + private static bool BlobStartsWith(in RedisValue whole, in RedisValue prefix) + { + long wScratch, pScratch; // at most two stack slots; reuse named locals (see BlobSequenceEqual) + var wSeq = whole.Type == StorageType.Sequence; + var pSeq = prefix.Type == StorageType.Sequence; + if (wSeq && pSeq) return whole.RawSequence().StartsWith(prefix.RawSequence()); + if (wSeq) return whole.RawSequence().StartsWith(prefix.UnsafeRawSpan(out pScratch)); + if (pSeq) return whole.UnsafeRawSpan(out wScratch).StartsWith(prefix.RawSequence()); + return whole.UnsafeRawSpan(out wScratch).StartsWith(prefix.UnsafeRawSpan(out pScratch)); + } + internal string RawString() { if (_obj is string s) return s; @@ -345,10 +443,8 @@ internal string RawString() return x._valueInt64 == y._valueInt64; case StorageType.String: return x.RawString() == y.RawString(); - case StorageType.ByteArray or StorageType.MemoryManager: - return x.RawSpan().SequenceEqual(y.RawSpan()); - case StorageType.Sequence: - return x.RawSequence().SequenceEqual(y.RawSequence()); + case StorageType.ByteArray or StorageType.MemoryManager or StorageType.ShortBlob or StorageType.Sequence: + return BlobSequenceEqual(x, y); } } @@ -369,18 +465,9 @@ internal string RawString() return false; } - // both are non-null, non-numeric, and of different kinds; the blob kinds (byte[] / memory / - // sequence) compare by bytes in any combination - and we keep the span-vs-span path for the - // case where neither side is a multi-segment sequence - switch (xType) - { - case StorageType.Sequence when yType is StorageType.ByteArray or StorageType.MemoryManager: - return x.RawSequence().SequenceEqual(y.RawSpan()); - case StorageType.ByteArray or StorageType.MemoryManager when yType == StorageType.Sequence: - return y.RawSequence().SequenceEqual(x.RawSpan()); - case StorageType.ByteArray or StorageType.MemoryManager when yType is StorageType.ByteArray or StorageType.MemoryManager: - return x.RawSpan().SequenceEqual(y.RawSpan()); - } + // both are non-null, non-numeric, and of different kinds; if both are byte-backed (byte[] / + // memory / short-blob / sequence) compare by raw bytes in any combination + if (IsBlob(xType) && IsBlob(yType)) return BlobSequenceEqual(x, y); // otherwise (anything involving a string), compare as strings return (string?)x == (string?)y; @@ -521,9 +608,31 @@ internal enum StorageType ByteArray, String, Sequence, + ShortBlob, Unknown, } + // Sentinel for inline blobs of 1..8 bytes: the bytes live directly in the overlapped _valueInt64 + // field (so _index/_length are NOT usable - the length comes from this sentinel instead). This lets + // short payloads - most literals, short keys/values, and inbound DB strings - avoid a byte[] alloc. + private sealed class ShortBlob + { + internal const int MaxLength = MaxInlineBytes; + private ShortBlob(int length) => Length = length; + internal int Length { get; } + // instances for lengths 1..8 only; length 0 is always represented as EmptyString, never a + // ShortBlob - so a ShortBlob is, by construction, never null or empty + private static readonly ShortBlob[] s_byLength = + { + new(1), new(2), new(3), new(4), new(5), new(6), new(7), new(8), + }; + internal static ShortBlob For(int length) + { + Debug.Assert(length is >= 1 and <= MaxLength, "short-blob length out of range"); + return s_byLength[length - 1]; + } + } + internal StorageType Type { get @@ -533,6 +642,9 @@ internal StorageType Type if (obj == Sentinel_SignedInteger) return StorageType.Int64; if (obj == Sentinel_Double) return StorageType.Double; if (obj is string) return StorageType.String; + // short blobs are expected to be very common on the inbound/read path (most small values + // and keys are <= 8 bytes), so probe for them early + if (obj is ShortBlob) return StorageType.ShortBlob; if (obj is byte[]) return StorageType.ByteArray; if (obj == Sentinel_UnsignedInteger) return StorageType.UInt64; if (obj is MemoryManager) return StorageType.MemoryManager; @@ -580,7 +692,7 @@ internal bool TryGetForeign([NotNullWhen(true)] out T? value, out int index, public long Length() => Type switch { StorageType.Null => 0, - StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence => _length, + StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence or StorageType.ShortBlob => BlobLength, StorageType.String => Encoding.UTF8.GetByteCount(RawString()), StorageType.Int64 => Format.MeasureInt64(OverlappedValueInt64), StorageType.UInt64 => Format.MeasureUInt64(OverlappedValueUInt64), @@ -617,10 +729,8 @@ private static int CompareTo(RedisValue x, RedisValue y) return x.OverlappedValueUInt64.CompareTo(y.OverlappedValueUInt64); case StorageType.String: return string.CompareOrdinal(x.RawString(), y.RawString()); - case StorageType.MemoryManager or StorageType.ByteArray: - return x.RawSpan().SequenceCompareTo(y.RawSpan()); - case StorageType.Sequence: - return x.RawSequence().SequenceCompareTo(y.RawSequence()); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob or StorageType.Sequence: + return BlobCompareTo(x, y); } } @@ -638,11 +748,9 @@ private static int CompareTo(RedisValue x, RedisValue y) if (yType == StorageType.Double) return ((double)x.OverlappedValueUInt64).CompareTo(y.OverlappedValueDouble); if (yType == StorageType.Int64) return -1; // we only use unsigned if > int64, so: x is bigger break; - case StorageType.MemoryManager or StorageType.ByteArray when yType is StorageType.MemoryManager or StorageType.ByteArray: - return x.RawSpan().SequenceCompareTo(y.RawSpan()); - case StorageType.MemoryManager or StorageType.ByteArray when yType == StorageType.Sequence: - case StorageType.Sequence when yType is StorageType.MemoryManager or StorageType.ByteArray: - return x.RawSequence().SequenceCompareTo(y.RawSequence()); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob or StorageType.Sequence + when IsBlob(yType): + return BlobCompareTo(x, y); } // otherwise, compare as strings @@ -890,7 +998,7 @@ public static explicit operator double(RedisValue value) StorageType.Double => value.OverlappedValueDouble, // special values like NaN/Inf are deliberately not handled by Simplify, but need to be considered for casting StorageType.String when Format.TryParseDouble(value.RawString(), out var d) => d, - StorageType.MemoryManager or StorageType.ByteArray when TryParseDouble(value.RawSpan(), out var d) => d, + StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob when TryParseDouble(value.UnsafeRawSpan(out _), out var d) => d, StorageType.Sequence when value.TryParse(out double d) => d, // linearizes + handles inf/nan, like the span case above // anything else: fail _ => throw new InvalidCastException($"Unable to cast from {value.Type} to double: '{value}'"), @@ -1014,10 +1122,11 @@ private static bool TryParseDouble(ReadOnlySpan blob, out double value) case StorageType.Int64: return Format.ToString(value.OverlappedValueInt64); case StorageType.UInt64: return Format.ToString(value.OverlappedValueUInt64); case StorageType.String: return value.RawString(); - case StorageType.MemoryManager or StorageType.ByteArray: - var span = value.RawSpan(); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + var span = value.UnsafeRawSpan(out _); if (span.IsEmpty) return ""; - if (span.Length == 2 && span[0] == (byte)'O' && span[1] == (byte)'K') return "OK"; // frequent special-case + const ushort OkPackedLE = 'O' | ('K' << 8); // frequent special-case + if (span.Length is 2 && BinaryPrimitives.ReadUInt16LittleEndian(span) == OkPackedLE) return "OK"; try { return Format.GetString(span); @@ -1075,8 +1184,8 @@ private static string ToHex(ReadOnlySpan src) case StorageType.ByteArray when value._obj is byte[] arr && value._index is 0 && value._length == arr.Length: // the memory is backed by an array, and we're reading all of it return arr; - case StorageType.ByteArray or StorageType.MemoryManager: - return value.RawSpan().ToArray(); + case StorageType.ByteArray or StorageType.MemoryManager or StorageType.ShortBlob: + return value.UnsafeRawSpan(out _).ToArray(); case StorageType.Sequence: return value.RawSequence().ToArray(); case StorageType.Int64: @@ -1106,7 +1215,7 @@ private static string ToHex(ReadOnlySpan src) public int GetByteCount() => Type switch { StorageType.Null => 0, - StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence => _length, + StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence or StorageType.ShortBlob => BlobLength, StorageType.String => Encoding.UTF8.GetByteCount(RawString()), StorageType.Int64 => Format.MeasureInt64(OverlappedValueInt64), StorageType.UInt64 => Format.MeasureUInt64(OverlappedValueUInt64), @@ -1120,7 +1229,7 @@ private static string ToHex(ReadOnlySpan src) internal int GetMaxByteCount() => Type switch { StorageType.Null => 0, - StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence => _length, + StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence or StorageType.ShortBlob => BlobLength, StorageType.String => Encoding.UTF8.GetMaxByteCount(RawString().Length), StorageType.Int64 => Format.MaxInt64TextLen, StorageType.UInt64 => Format.MaxInt64TextLen, @@ -1134,7 +1243,7 @@ private static string ToHex(ReadOnlySpan src) internal int GetCharCount() => Type switch { StorageType.Null => 0, - StorageType.MemoryManager or StorageType.ByteArray => Encoding.UTF8.GetCharCount(RawSpan()), + StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob => Encoding.UTF8.GetCharCount(UnsafeRawSpan(out _)), StorageType.Sequence => Encoding.UTF8.GetCharCount(RawSequence()), StorageType.String => _length, StorageType.Int64 => Format.MeasureInt64(OverlappedValueInt64), @@ -1149,7 +1258,7 @@ private static string ToHex(ReadOnlySpan src) internal int GetMaxCharCount() => Type switch { StorageType.Null => 0, - StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence => Encoding.UTF8.GetMaxCharCount(_length), + StorageType.MemoryManager or StorageType.ByteArray or StorageType.Sequence or StorageType.ShortBlob => Encoding.UTF8.GetMaxCharCount(BlobLength), StorageType.String => _length, StorageType.Int64 => Format.MaxInt64TextLen, StorageType.UInt64 => Format.MaxInt64TextLen, @@ -1175,9 +1284,10 @@ public int CopyTo(Span destination) { case StorageType.Null: return 0; - case StorageType.MemoryManager or StorageType.ByteArray: - RawSpan().CopyTo(destination); - return _length; + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + var blob = UnsafeRawSpan(out _); + blob.CopyTo(destination); + return blob.Length; case StorageType.Sequence: RawSequence().CopyTo(destination); return _length; @@ -1203,8 +1313,8 @@ internal int CopyTo(Span destination) { case StorageType.Null: return 0; - case StorageType.MemoryManager or StorageType.ByteArray: - return Encoding.UTF8.GetChars(RawSpan(), destination); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + return Encoding.UTF8.GetChars(UnsafeRawSpan(out _), destination); case StorageType.Sequence: return Encoding.UTF8.GetChars(RawSequence(), destination); case StorageType.String: @@ -1315,8 +1425,8 @@ internal RedisValue Simplify() // note: don't simplify inf/nan, as that causes equality semantic problems if (Format.TryParseDouble(s, out var f64) && !IsSpecialDouble(f64)) return f64; break; - case StorageType.MemoryManager or StorageType.ByteArray: - if (TrySimplify(RawSpan(), out var simplified)) return simplified; + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + if (TrySimplify(UnsafeRawSpan(out _), out var simplified)) return simplified; break; case StorageType.Sequence: // numeric forms are short, so we only need to consider plausibly-numeric lengths; @@ -1386,8 +1496,9 @@ public bool TryParse(out long val) return false; case StorageType.String: return Format.TryParseInt64(RawString(), out val); - case StorageType.MemoryManager or StorageType.ByteArray: - return Format.TryParseInt64(RawSpan(), out val); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + return Format.TryParseInt64(UnsafeRawSpan(out _), out val); + case StorageType.Sequence: // longer than the largest possible Int64 text => cannot be an Int64; otherwise // linearize onto the stack and reuse the span-based parse (matching the ByteArray path) @@ -1455,8 +1566,8 @@ public bool TryParse(out double val) return true; case StorageType.String: return Format.TryParseDouble(RawString(), out val); - case StorageType.MemoryManager or StorageType.ByteArray: - return TryParseDouble(RawSpan(), out val); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + return TryParseDouble(UnsafeRawSpan(out _), out val); case StorageType.Sequence: // longer than the largest possible double text => cannot be a double; otherwise // linearize onto the stack and reuse the span-based parse (matching the ByteArray path) @@ -1536,26 +1647,15 @@ public bool StartsWith(RedisValue value) var sThis = RawString(); var sOther = value.RawString(); return sThis.StartsWith(sOther, StringComparison.Ordinal); - case StorageType.MemoryManager or StorageType.ByteArray: - return RawSpan().StartsWith(value.RawSpan()); - case StorageType.Sequence: - return RawSequence().StartsWith(value.RawSequence()); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob or StorageType.Sequence: + return BlobStartsWith(this, value); } } - if (thisType == StorageType.Sequence && - (otherType == StorageType.MemoryManager || otherType == StorageType.ByteArray)) - { - return RawSequence().StartsWith(value.RawSpan()); - } - if (otherType == StorageType.Sequence && - (thisType == StorageType.MemoryManager || thisType == StorageType.ByteArray)) - { - return RawSpan().StartsWith(value.RawSequence()); - } - if ((thisType == StorageType.MemoryManager && otherType == StorageType.ByteArray) || - (thisType == StorageType.ByteArray && otherType == StorageType.MemoryManager)) + + // mixed byte-backed kinds (byte[] / memory / short-blob / sequence) compare by raw bytes + if (IsBlob(thisType) && IsBlob(otherType)) { - return RawSpan().StartsWith(value.RawSpan()); + return BlobStartsWith(this, value); } byte[]? arr0 = null, arr1 = null; try @@ -1586,6 +1686,11 @@ private ReadOnlyMemory AsMemory(out byte[]? leased) leased = ArrayPool.Shared.Rent(_length); RawSequence().CopyTo(leased); return new ReadOnlyMemory(leased, 0, _length); + case StorageType.ShortBlob: + var blob = UnsafeRawSpan(out _); + leased = ArrayPool.Shared.Rent(blob.Length); + blob.CopyTo(leased); + return new ReadOnlyMemory(leased, 0, blob.Length); case StorageType.String: string s = RawString(); HaveString: @@ -1622,8 +1727,8 @@ internal ValueCondition Digest() { switch (Type) { - case StorageType.MemoryManager or StorageType.ByteArray: - return ValueCondition.CalculateDigest(RawSpan()); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + return ValueCondition.CalculateDigest(UnsafeRawSpan(out _)); case StorageType.Sequence: return ValueCondition.CalculateDigest(RawSequence()); case StorageType.Null: @@ -1669,8 +1774,8 @@ public bool StartsWith(ReadOnlySpan value) int len; switch (Type) { - case StorageType.MemoryManager or StorageType.ByteArray: - return RawSpan().StartsWith(value); + case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: + return UnsafeRawSpan(out _).StartsWith(value); case StorageType.Sequence: return RawSequence().StartsWith(value); case StorageType.Int64: diff --git a/src/StackExchange.Redis/RespReaderExtensions.cs b/src/StackExchange.Redis/RespReaderExtensions.cs index 2ba1ea57f4..711a43b558 100644 --- a/src/StackExchange.Redis/RespReaderExtensions.cs +++ b/src/StackExchange.Redis/RespReaderExtensions.cs @@ -24,12 +24,33 @@ public RedisValue ReadRedisValue() return reader.ReadInt64(); } - // bulk/simple/verbatim string: for inline scalars, 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; otherwise keep the raw bytes - if (reader.TryGetSpan(out var span) && TryReadCanonicalNumber(span, out var number)) + // bulk/simple/verbatim string. Only inline (non-streaming) scalars get the compact storage + // kinds; streaming scalars fall through to ReadByteArray. + if (reader.IsInlineScalar) { - return number; + 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(); } diff --git a/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs b/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs new file mode 100644 index 0000000000..687ca0248a --- /dev/null +++ b/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs @@ -0,0 +1,166 @@ +using System; +using System.Buffers; +using System.Linq; +using System.Text; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// Validates the inline "short blob" storage kind (), which +/// packs 1..8 payload bytes into the overlapped int64 field instead of allocating a byte[]. Every projection +/// must be indistinguishable from the equivalent byte[]-backed value. +/// +public class RedisValueShortBlobTests +{ + private static RedisValue Short(byte[] bytes) => RedisValue.FromRaw(bytes); // <= 8 bytes => ShortBlob + + private static RedisValue Sequence(byte[] bytes) // multi-segment, one byte per chunk + => FragmentedSegment.Create(bytes.Select((_, i) => new ReadOnlyMemory(bytes, i, 1)).ToArray()); + + [Theory] + [InlineData("a")] + [InlineData("ab")] + [InlineData("OK")] + [InlineData("hello")] + [InlineData("12345678")] // 8 bytes, the max inline size + [InlineData("1234")] // numeric-looking + [InlineData("-42")] + [InlineData("0.5")] + [InlineData("inf")] // special token (not simplified to a double) + [InlineData("a1b2c3")] // mixed alphanumeric + public void ShortBlob_IsIndistinguishableFromByteArray(string text) + { + var bytes = Encoding.UTF8.GetBytes(text); + Assert.True(bytes.Length is >= 1 and <= 8); + + var shortBlob = Short(bytes); + RedisValue byteArray = (byte[])bytes.Clone(); + + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + Assert.Equal(RedisValue.StorageType.ByteArray, byteArray.Type); + + // length / projections + Assert.Equal(byteArray.Length(), shortBlob.Length()); + Assert.Equal((string?)byteArray, (string?)shortBlob); + Assert.Equal((byte[]?)byteArray, (byte[]?)shortBlob); + + // equality + hash + compare, both directions, against the byte[] form + Assert.True(shortBlob == byteArray); + Assert.True(byteArray == shortBlob); + Assert.True(shortBlob.Equals(byteArray)); + Assert.Equal(byteArray.GetHashCode(), shortBlob.GetHashCode()); + Assert.Equal(0, shortBlob.CompareTo(byteArray)); + + // also equal to the equivalent multi-segment sequence (cross-kind) + var sequence = Sequence(bytes); + Assert.True(shortBlob == sequence); + Assert.Equal(shortBlob.GetHashCode(), sequence.GetHashCode()); + Assert.Equal(0, shortBlob.CompareTo(sequence)); + + // CopyTo round-trips + Span copy = stackalloc byte[shortBlob.GetByteCount()]; + Assert.Equal(bytes.Length, shortBlob.CopyTo(copy)); + Assert.True(copy.SequenceEqual(bytes)); + } + + [Fact] + public void ShortBlob_NumericContent_EqualsAndParsesLikeInteger() + { + var shortBlob = Short(Encoding.UTF8.GetBytes("1234")); + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + + Assert.True(shortBlob == 1234); + Assert.True(1234 == shortBlob); + Assert.Equal(((RedisValue)1234).GetHashCode(), shortBlob.GetHashCode()); // numeric-consistent hash + + Assert.True(shortBlob.TryParse(out long l)); + Assert.Equal(1234, l); + Assert.Equal(1234, (int)shortBlob); + } + + [Theory] + [InlineData("inf", double.PositiveInfinity)] + [InlineData("+inf", double.PositiveInfinity)] + [InlineData("-inf", double.NegativeInfinity)] + [InlineData("Inf", double.PositiveInfinity)] // case-insensitive + [InlineData("nan", double.NaN)] + public void ShortBlob_SpecialDouble_ParsesLikeByteArray(string text, double expected) + { + // regression: inf/nan are deliberately NOT folded by Simplify() (they'd break equality semantics), + // so they stay blob-backed; a <= 8 byte payload is a ShortBlob. The (double) cast must parse it + // exactly like the byte[] form. Previously the cast had no ShortBlob arm and threw for these. + var bytes = Encoding.UTF8.GetBytes(text); + var shortBlob = Short(bytes); + RedisValue byteArray = (byte[])bytes.Clone(); + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + Assert.Equal(RedisValue.StorageType.ByteArray, byteArray.Type); + + Assert.Equal(expected, (double)shortBlob); + Assert.Equal((double)byteArray, (double)shortBlob); // indistinguishable from the byte[] form + } + + [Fact] + public void ShortBlob_StartsWith_MatchesByteArray() + { + var shortBlob = Short(Encoding.UTF8.GetBytes("abcde")); + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + + Assert.True(shortBlob.StartsWith("abc"u8.ToArray())); + Assert.True(shortBlob.StartsWith(Short("ab"u8.ToArray()))); // ShortBlob vs ShortBlob + Assert.True(shortBlob.StartsWith(Sequence("abc"u8.ToArray()))); // ShortBlob vs Sequence + Assert.False(shortBlob.StartsWith("abd"u8.ToArray())); + Assert.False(shortBlob.StartsWith("abcdef"u8.ToArray())); + } + + [Fact] + public void FromRaw_RoutesByLength() + { + Assert.Equal(RedisValue.StorageType.String, Short(Array.Empty()).Type); // empty => EmptyString + Assert.Equal(RedisValue.StorageType.ShortBlob, Short(new byte[8]).Type); // 8 => inline + Assert.Equal(RedisValue.StorageType.ByteArray, Short(new byte[9]).Type); // 9 => allocate + } + + [Fact] + public void ShortBlob_NonTextBytes_RoundTripAndEqualByteArray() + { + // arbitrary non-UTF8 bytes including zero and high bytes, exactly 8 long (max inline) + var bytes = new byte[] { 0x00, 0x01, 0xFF, 0x80, 0x7F, 0xAB, 0x00, 0xCD }; + var shortBlob = Short(bytes); + RedisValue byteArray = (byte[])bytes.Clone(); + + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + Assert.True(shortBlob == byteArray); + Assert.Equal(byteArray.GetHashCode(), shortBlob.GetHashCode()); + Assert.Equal(bytes, (byte[]?)shortBlob); + Assert.Equal(0, shortBlob.CompareTo(byteArray)); + } + + [Fact] + public void ShortBlob_WriteBulkString_MatchesByteArray() + { + var bytes = Encoding.UTF8.GetBytes("hello"); // 5 bytes => ShortBlob + var shortBlob = Short(bytes); + RedisValue byteArray = (byte[])bytes.Clone(); + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + + var fromShortBlob = new ArrayBufferWriter(); + var fromByteArray = new ArrayBufferWriter(); + MessageWriter.WriteBulkString(shortBlob, fromShortBlob); + MessageWriter.WriteBulkString(byteArray, fromByteArray); + + // the wire bytes must be identical, and a well-formed RESP bulk string + Assert.Equal(fromByteArray.WrittenSpan.ToArray(), fromShortBlob.WrittenSpan.ToArray()); + Assert.Equal(Encoding.ASCII.GetBytes("$5\r\nhello\r\n"), fromShortBlob.WrittenSpan.ToArray()); + } + + [Fact] + public void ShortBlob_NotNullOrEmpty() + { + var shortBlob = Short(new byte[] { 0 }); // a single zero byte + Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); + Assert.False(shortBlob.IsNull); + Assert.False(shortBlob.IsNullOrEmpty); + Assert.True(shortBlob.HasValue); + } +} diff --git a/tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs b/tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs deleted file mode 100644 index b3490b4600..0000000000 --- a/tests/StackExchange.Redis.Tests/RedisValueStorageKindTests.cs +++ /dev/null @@ -1,52 +0,0 @@ -using System.Threading.Tasks; -using Xunit; - -namespace StackExchange.Redis.Tests; - -/// -/// Pins the that the inbound read path -/// (RespReader.ReadRedisValue) produces for values fetched from a server: numeric-looking bulk -/// strings are stored compactly as Int64/Double when they round-trip exactly, otherwise as a byte[]. -/// -/// -/// The expected storage kind is passed by name because is internal and -/// so cannot appear as a parameter on a public theory method. -/// -public class RedisValueStorageKindTests(ITestOutputHelper output, InProcServerFixture fixture) : TestBase(output, fixture) -{ - [Theory] - // canonical integers => Int64 - [InlineData("1234", "Int64")] - [InlineData("0", "Int64")] - [InlineData("-5", "Int64")] - [InlineData("9223372036854775807", "Int64")] // long.MaxValue - // canonical finite doubles => Double - [InlineData("0.5", "Double")] - [InlineData("-2.25", "Double")] - // non-canonical / non-numeric / special => kept as bytes - [InlineData("01234", "ByteArray")] // leading zero - [InlineData("+1234", "ByteArray")] // leading '+' - [InlineData("-0", "ByteArray")] // negative zero text - [InlineData("1.50", "ByteArray")] // trailing zero (non-canonical double) - [InlineData("1e3", "ByteArray")] // exponent form - [InlineData("inf", "ByteArray")] // special token excluded - [InlineData("nan", "ByteArray")] - [InlineData("99999999999999999999999", "ByteArray")] // oversize - [InlineData("abc", "ByteArray")] // not numeric - [InlineData("12abc", "ByteArray")] // trailing junk - public async Task FetchedValue_StorageKindAndRoundTrip(string stored, string expectedKind) - { - await using var conn = Create(); - var db = conn.GetDatabase(); - var key = Me(); - db.KeyDelete(key, CommandFlags.FireAndForget); - - db.StringSet(key, stored); - var value = db.StringGet(key); - - // the optimization must be invisible: the text projection always matches what was stored - Assert.Equal(stored, (string?)value); - Log($"'{stored}' => {value.Type}"); - Assert.Equal(expectedKind, value.Type.ToString()); - } -} diff --git a/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs b/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs new file mode 100644 index 0000000000..710f5855ca --- /dev/null +++ b/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs @@ -0,0 +1,109 @@ +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// On-box unit tests (in-proc server, no external redis) that pin the +/// produced both for values on the inbound read path (RespReader.ReadRedisValue) and for the shared +/// constants. +/// +/// +/// The expected storage kind is compared by name because is internal and +/// so cannot appear as a parameter on a public theory method. +/// +public class RedisValueStorageKindUnitTests(ITestOutputHelper output, InProcServerFixture fixture) : TestBase(output, fixture) +{ + /// + /// Payloads of <= 8 bytes are kept inline as a short-blob with no eager numeric parse (any + /// numeric interpretation is deferred to the caller); only longer canonical numeric text is + /// stored compactly as Int64/Double, and anything else is materialized as a byte[]. + /// + [Theory] + // <= 8 bytes => inline short-blob, whether or not the text looks numeric (numeric parse is deferred) + [InlineData("1234", "ShortBlob")] + [InlineData("0", "ShortBlob")] + [InlineData("-5", "ShortBlob")] + [InlineData("12345678", "ShortBlob")] // 8 bytes, the inline max + [InlineData("0.5", "ShortBlob")] + [InlineData("-2.25", "ShortBlob")] + [InlineData("01234", "ShortBlob")] // leading zero + [InlineData("+1234", "ShortBlob")] // leading '+' + [InlineData("-0", "ShortBlob")] // negative zero text + [InlineData("1.50", "ShortBlob")] // trailing zero (non-canonical double) + [InlineData("1e3", "ShortBlob")] // exponent form + [InlineData("inf", "ShortBlob")] // special token excluded from numeric, but still short + [InlineData("nan", "ShortBlob")] + [InlineData("abc", "ShortBlob")] // not numeric + [InlineData("12abc", "ShortBlob")] // trailing junk + // > 8 bytes AND a canonical number => compact Int64/Double (also avoids the byte[] alloc) + [InlineData("123456789", "Int64")] // 9 bytes + [InlineData("9223372036854775807", "Int64")] // long.MaxValue (19 bytes) + [InlineData("1048576.5", "Double")] // 9 bytes, exactly representable => canonical under G17 + [InlineData("-1048576.25", "Double")] // 11 bytes + // > 8 bytes and not a canonical number => materialized as a byte[] + [InlineData("99999999999999999999999", "ByteArray")] // oversize numeric + [InlineData("the quick brown fox", "ByteArray")] // long text + // > 8 bytes, parses as a double but does NOT round-trip under G17 (IEEE-754 noise), so it must stay a + // byte[] to preserve byte-for-byte string behaviour rather than being mangled into a reformatted double + [InlineData("119999.99", "ByteArray")] // a price; G17 => "119999.99000000001" + [InlineData("2.675000000", "ByteArray")] // classic: the nearest double is < 2.675 => G17 "2.6749999999999998" + public async Task FetchedValue_StorageKindAndRoundTrip(string stored, string expectedKind) + { + await using var conn = Create(); + var db = conn.GetDatabase(); + var key = Me(); + db.KeyDelete(key, CommandFlags.FireAndForget); + + db.StringSet(key, stored); + var value = db.StringGet(key); + + // the optimization must be invisible: the text projection always matches what was stored + Assert.Equal(stored, (string?)value); + Log($"'{stored}' => {value.Type}"); + Assert.Equal(expectedKind, value.Type.ToString()); + } + + /// + /// Every constant is built via RedisValue.FromRaw(...u8), so each is + /// either an inline short-blob (<= 8 bytes) or a byte[] (> 8 bytes); none are numeric. We assert the + /// byte[] count (crossing 8 bytes is rare and noteworthy) and that only those two storage flavors appear + /// (so a numeric/other kind creeping in is caught), while the short-blob count is only logged - so adding + /// more short literals (the common case) never forces a test edit. + /// + [Fact] + public void LiteralsResolveToExpectedStorageKinds() + { + var literals = typeof(RedisLiterals) + .GetFields(BindingFlags.Public | BindingFlags.Static) + .Where(f => f.IsInitOnly && f.FieldType == typeof(RedisValue)) + .Select(f => ((RedisValue)f.GetValue(null)!).Type) + .ToArray(); + + Assert.NotEmpty(literals); + + var byKind = new Dictionary(); + foreach (var kind in literals) + { + byKind.TryGetValue(kind, out var n); + byKind[kind] = n + 1; + } + + int Count(RedisValue.StorageType kind) => byKind.TryGetValue(kind, out var n) ? n : 0; + + foreach (var kind in byKind.Keys.OrderBy(k => k.ToString())) + { + Output.WriteLine($"{kind}: {byKind[kind]}"); + } + + Output.WriteLine($"ShortBlob (logged, not asserted): {Count(RedisValue.StorageType.ShortBlob)}"); + + // only two storage flavors should ever appear; a third means something new crept in (e.g. a numeric + // literal => Int64), which we want to notice + Assert.Equal(2, byKind.Count); + Assert.Equal(22, Count(RedisValue.StorageType.ByteArray)); // literals > 8 bytes + } +} From 244b3fc7afc285aea69e3055bf0edf4802bdb983 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 Jun 2026 13:53:20 +0100 Subject: [PATCH 3/5] fix CI --- .../RedisValueShortBlobTests.cs | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs b/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs index 687ca0248a..360174f830 100644 --- a/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs +++ b/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs @@ -1,5 +1,4 @@ using System; -using System.Buffers; using System.Linq; using System.Text; using Xunit; @@ -144,14 +143,34 @@ public void ShortBlob_WriteBulkString_MatchesByteArray() RedisValue byteArray = (byte[])bytes.Clone(); Assert.Equal(RedisValue.StorageType.ShortBlob, shortBlob.Type); - var fromShortBlob = new ArrayBufferWriter(); - var fromByteArray = new ArrayBufferWriter(); - MessageWriter.WriteBulkString(shortBlob, fromShortBlob); - MessageWriter.WriteBulkString(byteArray, fromByteArray); + var fromShortBlob = WriteBulkString(shortBlob); + var fromByteArray = WriteBulkString(byteArray); // the wire bytes must be identical, and a well-formed RESP bulk string - Assert.Equal(fromByteArray.WrittenSpan.ToArray(), fromShortBlob.WrittenSpan.ToArray()); - Assert.Equal(Encoding.ASCII.GetBytes("$5\r\nhello\r\n"), fromShortBlob.WrittenSpan.ToArray()); + Assert.Equal(fromByteArray, fromShortBlob); + Assert.True(fromShortBlob.AsSpan().SequenceEqual("$5\r\nhello\r\n"u8), "RESP bulk string mismatch"); + + // serialize a single bulk string via the shared block buffer and copy out the exact wire bytes before + // releasing it (we avoid ArrayBufferWriter, which isn't available on net48x) + static byte[] WriteBulkString(RedisValue value) + { + ReadOnlyMemory payload = default; + try + { + MessageWriter.WriteBulkString(value, MessageWriter.BlockBuffer); + payload = MessageWriter.FlushBlockBuffer(); + return payload.ToArray(); + } + catch + { + MessageWriter.RevertBlockBuffer(); + throw; + } + finally + { + MessageWriter.ReleaseBlockBuffer(payload); + } + } } [Fact] From 08151256e5c5cb3f2889af4bf6a0f660f40e406b Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 Jun 2026 14:15:18 +0100 Subject: [PATCH 4/5] CI: tweak failing test --- .../StackExchange.Redis.Tests/Issues/Issue1103Tests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs b/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs index fb7fbfb5e5..98870d38f6 100644 --- a/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs +++ b/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs @@ -8,13 +8,13 @@ namespace StackExchange.Redis.Tests.Issues; public class Issue1103Tests(ITestOutputHelper output) : TestBase(output) { [Theory] - [InlineData(142205255210238005UL, (int)StorageType.Int64)] + [InlineData(142205255210238005UL, (int)StorageType.Int64, (int)StorageType.Int64)] [InlineData(ulong.MaxValue, (int)StorageType.UInt64)] - [InlineData(ulong.MinValue, (int)StorageType.Int64)] + [InlineData(ulong.MinValue, (int)StorageType.Int64, (int)StorageType.ShortBlob)] [InlineData(0x8000000000000000UL, (int)StorageType.UInt64)] [InlineData(0x8000000000000001UL, (int)StorageType.UInt64)] - [InlineData(0x7FFFFFFFFFFFFFFFUL, (int)StorageType.Int64)] - public async Task LargeUInt64StoredCorrectly(ulong value, int storageType) + [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 = (int)StorageType.ByteArray) { await using var conn = Create(); @@ -29,7 +29,7 @@ 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()); From 601220abbe0fbd2624d087b2725e0efc50479253 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 26 Jun 2026 14:50:15 +0100 Subject: [PATCH 5/5] tidying (support ulong as canonical) --- src/StackExchange.Redis/RedisValue.cs | 1 - .../RespReaderExtensions.cs | 47 ++++++++++++++----- .../Issues/Issue1103Tests.cs | 10 ++-- .../RedisValueShortBlobTests.cs | 19 ++++++++ .../RedisValueStorageKindUnitTests.cs | 3 ++ 5 files changed, 63 insertions(+), 17 deletions(-) diff --git a/src/StackExchange.Redis/RedisValue.cs b/src/StackExchange.Redis/RedisValue.cs index ffa8412671..fb2a6c0b01 100644 --- a/src/StackExchange.Redis/RedisValue.cs +++ b/src/StackExchange.Redis/RedisValue.cs @@ -1498,7 +1498,6 @@ public bool TryParse(out long val) return Format.TryParseInt64(RawString(), out val); case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob: return Format.TryParseInt64(UnsafeRawSpan(out _), out val); - case StorageType.Sequence: // longer than the largest possible Int64 text => cannot be an Int64; otherwise // linearize onto the stack and reuse the span-based parse (matching the ByteArray path) diff --git a/src/StackExchange.Redis/RespReaderExtensions.cs b/src/StackExchange.Redis/RespReaderExtensions.cs index 711a43b558..2107d03977 100644 --- a/src/StackExchange.Redis/RespReaderExtensions.cs +++ b/src/StackExchange.Redis/RespReaderExtensions.cs @@ -243,10 +243,11 @@ internal bool AnyNull() private static readonly int MaxCanonicalLength = Math.Max(Format.MaxInt64TextLen, Format.MaxDoubleTextLen); - // Recognizes the canonical decimal text of an integer or 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"), oversize - // values, and the special inf/nan tokens all return false and are kept as a byte[] payload by the caller. + // 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 span, out RedisValue value) { static bool Failure(out RedisValue value) @@ -258,7 +259,10 @@ static bool Failure(out RedisValue value) // 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 + // 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) @@ -273,23 +277,44 @@ static bool Failure(out RedisValue value) case (byte)'7': case (byte)'8': case (byte)'9': - case (byte)'.': + break; case (byte)'-': + seenNegative = true; + break; + case (byte)'.': case (byte)'E': case (byte)'e': + seenDotOrExp = true; break; default: return Failure(out value); } } - if (Format.TryParseInt64(span, out var i64) && Format.MeasureInt64(i64) == span.Length) + if (!seenDotOrExp) { - value = i64; - return true; - } + // 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; + } - if (Format.TryParseDouble(span, out var dbl)) + // 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 formatted = stackalloc byte[Format.MaxDoubleTextLen]; diff --git a/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs b/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs index 98870d38f6..47b67f3f1a 100644 --- a/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs +++ b/tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs @@ -9,12 +9,12 @@ public class Issue1103Tests(ITestOutputHelper output) : TestBase(output) { [Theory] [InlineData(142205255210238005UL, (int)StorageType.Int64, (int)StorageType.Int64)] - [InlineData(ulong.MaxValue, (int)StorageType.UInt64)] + [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)] - [InlineData(0x8000000000000001UL, (int)StorageType.UInt64)] + [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 = (int)StorageType.ByteArray) + public async Task LargeUInt64StoredCorrectly(ulong value, int storageType, int fromRedisType) { await using var conn = Create(); @@ -35,7 +35,7 @@ public async Task LargeUInt64StoredCorrectly(ulong value, int storageType, int f 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()); } diff --git a/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs b/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs index 360174f830..57a90f901b 100644 --- a/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs +++ b/tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs @@ -112,6 +112,25 @@ public void ShortBlob_StartsWith_MatchesByteArray() Assert.False(shortBlob.StartsWith("abcdef"u8.ToArray())); } + [Fact] + public void ShortBlob_CompareTo_Sequence_NonEqualOrdering() + { + // the equal-content tests only assert CompareTo == 0; this pins the *non-zero* cross-kind branches + // of BlobCompareTo (contiguous ShortBlob vs multi-segment Sequence), in both directions. + var abc = Short("abc"u8.ToArray()); + var abd = Sequence("abd"u8.ToArray()); + + // differing content: "abc" < "abd" + Assert.True(abc.CompareTo(abd) < 0); // ShortBlob (x) vs Sequence (y) - the ySeq branch + Assert.True(abd.CompareTo(abc) > 0); // Sequence (x) vs ShortBlob (y) - the negated xSeq branch + + // length mismatch: "ab" is a prefix of "abc", so the shorter value sorts first + var ab = Short("ab"u8.ToArray()); + var abcSeq = Sequence("abc"u8.ToArray()); + Assert.True(ab.CompareTo(abcSeq) < 0); + Assert.True(abcSeq.CompareTo(ab) > 0); + } + [Fact] public void FromRaw_RoutesByLength() { diff --git a/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs b/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs index 710f5855ca..748589e2b7 100644 --- a/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs @@ -42,6 +42,9 @@ public class RedisValueStorageKindUnitTests(ITestOutputHelper output, InProcServ // > 8 bytes AND a canonical number => compact Int64/Double (also avoids the byte[] alloc) [InlineData("123456789", "Int64")] // 9 bytes [InlineData("9223372036854775807", "Int64")] // long.MaxValue (19 bytes) + // canonical, non-negative, and > long.MaxValue => UInt64 (covers the full ulong range on read) + [InlineData("9223372036854775808", "UInt64")] // long.MaxValue + 1 (19 bytes) + [InlineData("18446744073709551615", "UInt64")] // ulong.MaxValue (20 bytes) [InlineData("1048576.5", "Double")] // 9 bytes, exactly representable => canonical under G17 [InlineData("-1048576.25", "Double")] // 11 bytes // > 8 bytes and not a canonical number => materialized as a byte[]