From 8ff49324491e895d259403377aa434b6c2576db9 Mon Sep 17 00:00:00 2001 From: mgravell Date: Wed, 16 Sep 2026 19:58:44 +0100 Subject: [PATCH 1/2] Compare strings against blobs without decoding them into a string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Equality between a string and a byte-backed RedisValue fell through to (string?)x == (string?)y, which materialises the blob as a transient string before comparing. That allocation is proportional to the payload: at 64KB it is a 131KB string, over the large-object-heap threshold, on every comparison. And because the whole payload is decoded before anything is compared, a value that differs in its first byte costs exactly as much as one that matches. Answer the same question without building the string: decode a chunk at a time into a small stack buffer and compare as we go, stopping at the chunk that contains a mismatch. A bounds check first - UTF-8 yields at most one char per byte, and never more than three bytes per char - rejects mismatched lengths before touching any content. Sequence-backed values carry up to four bytes across a segment boundary, since a sequence split mid-character has to be resumed rather than treated as invalid data. The relation is deliberately unchanged: still "the blob, read as UTF-8 text, equals the string". Comparing the other way - encoding the string and matching raw bytes - looks equivalent but is not, because UTF-8 does not round-trip strings holding unpaired surrogates: "\uD800" encodes to the same bytes as "�", so the two would compare equal while GetHashCode, which hashes the decoded form, keeps them in different buckets. Equal values with different hash codes silently break Dictionary and HashSet. Measured against the local six-node figures in RedisValueEqualityBenchmarks (net10.0, string vs byte[]): 64KB, equal 43,121ns / 131,110B -> 5,397ns / 0B (8x) 64KB, differs first 57,286ns / 131,110B -> 52ns / 0B (1092x) 1KB, differs first 119ns / 2,072B -> 55ns / 0B (2.2x) 16B, equal 59ns / 56B -> 56ns / 0B Allocation is gone at every size. Small values are otherwise a wash, and two cases are marginally slower - 1KB differing at the last byte goes 138ns -> 153ns - where the per-chunk overhead lands on a path that has to decode everything anyway. Down-level targets keep the previous implementation: System.Text.Unicode.Utf8 does not exist there. The tests compare against the old behaviour directly, over text that does not survive a UTF-8 round trip (truncated, overlong and invalid sequences, surrogate pairs, lone surrogates) and, for sequences, at every possible split point. --- src/StackExchange.Redis/RedisValue.cs | 146 +++++++++++++++++- .../RedisValueEqualityBenchmarks.cs | 113 ++++++++++++++ .../RedisValueEquivalencyTests.cs | 139 +++++++++++++++++ 3 files changed, 396 insertions(+), 2 deletions(-) create mode 100644 tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs diff --git a/src/StackExchange.Redis/RedisValue.cs b/src/StackExchange.Redis/RedisValue.cs index accb21efa..0b3820f61 100644 --- a/src/StackExchange.Redis/RedisValue.cs +++ b/src/StackExchange.Redis/RedisValue.cs @@ -11,6 +11,9 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; +#if NET +using System.Text.Unicode; +#endif using RESPite; namespace StackExchange.Redis @@ -477,10 +480,149 @@ internal string RawString() // 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; + // otherwise: exactly one side is a string and the other is byte-backed (everything else has + // been dealt with above). The relation is unchanged - "the blob, read as UTF-8 text, equals the + // string" - but it is answered without materialising that text: see StringEqualsBlob. + return xType == StorageType.String + ? StringEqualsBlob(x.RawString(), in y) + : StringEqualsBlob(y.RawString(), in x); } + /// + /// Compares a string against the UTF-8 text of a byte-backed value, without decoding it into a string. + /// + /// + /// + /// Identical in meaning to s == (string)blob, deliberately: equality here is defined by + /// decoding the blob, and hashes the same decoded form. Comparing + /// the other way around - encoding the string and matching bytes - is *not* the same relation, because + /// UTF-8 does not round-trip strings holding unpaired surrogates, and it would put equal values in + /// different hash buckets. + /// + /// + /// The decode runs a chunk at a time into a small stack buffer, so a long value costs no allocation + /// (the previous form allocated a transient string, which for values over ~42k chars landed on the + /// large object heap), and a mismatch stops at the chunk that contains it rather than after decoding + /// the whole payload. + /// + /// + private static bool StringEqualsBlob(string s, in RedisValue blob) + { + // Bounds, before touching any content: UTF-8 never yields more chars than it has bytes, and never + // spends more than 3 bytes per char (4-byte sequences produce 2 chars, i.e. 2 bytes per char). + int byteLength = blob.BlobLength; + if (s.Length > byteLength || (long)s.Length * 3 < byteLength) return false; + +#if NET + if (blob.Type == StorageType.Sequence) + { + var seq = blob.RawSequence(); + if (!seq.IsSingleSegment) return StringEqualsUtf8(s, seq); + return StringEqualsUtf8(s, seq.First.Span); + } + return StringEqualsUtf8(s, blob.UnsafeRawSpan(out _)); +#else + // no System.Text.Unicode.Utf8 on the down-level targets; keep the original behaviour there + return s == (string?)blob; +#endif + } + +#if NET + /// Chars decoded per pass; small enough that the stack cost is irrelevant. + private const int CompareChunkChars = 128; + + /// Longest UTF-8 sequence, i.e. the most that can be left pending at a segment boundary. + private const int MaxUtf8SequenceLength = 4; + + private static bool StringEqualsUtf8(string s, scoped ReadOnlySpan utf8) + { + Span chars = stackalloc char[CompareChunkChars]; + int matched = 0; + while (!utf8.IsEmpty) + { + // isFinalBlock: false - a chunk boundary landing mid-sequence must be resumed on the next + // pass, not reported as invalid data + Utf8.ToUtf16(utf8, chars, out int bytesRead, out int charsWritten, replaceInvalidSequences: true, isFinalBlock: false); + if (bytesRead == 0 && charsWritten == 0) + { + // what remains is a truncated sequence at the very end; decoding it as final is what + // yields the replacement char that the string form would have carried + Utf8.ToUtf16(utf8, chars, out bytesRead, out charsWritten, replaceInvalidSequences: true, isFinalBlock: true); + if (bytesRead == 0 && charsWritten == 0) return false; // no progress: cannot match + } + if (!AdvanceMatch(s, chars.Slice(0, charsWritten), ref matched)) return false; + utf8 = utf8.Slice(bytesRead); + } + return matched == s.Length; + } + + private static bool StringEqualsUtf8(string s, scoped in ReadOnlySequence utf8) + { + Span chars = stackalloc char[CompareChunkChars]; + Span pending = stackalloc byte[MaxUtf8SequenceLength]; + int pendingLength = 0, matched = 0; + + foreach (var segment in utf8) + { + var span = segment.Span; + while (!span.IsEmpty) + { + if (pendingLength != 0) + { + // a sequence split across segments: top it up from this one and decode just that + int take = Math.Min(MaxUtf8SequenceLength - pendingLength, span.Length); + span.Slice(0, take).CopyTo(pending.Slice(pendingLength)); + int available = pendingLength + take; + + Utf8.ToUtf16(pending.Slice(0, available), chars, out int joinedBytes, out int joinedChars, replaceInvalidSequences: true, isFinalBlock: false); + if (joinedBytes == 0) + { + // still short: absorb what we took and look to the next segment + pendingLength = available; + span = span.Slice(take); + continue; + } + + if (!AdvanceMatch(s, chars.Slice(0, joinedChars), ref matched)) return false; + span = span.Slice(joinedBytes - pendingLength); // give back the bytes we borrowed but did not use + pendingLength = 0; + continue; + } + + Utf8.ToUtf16(span, chars, out int bytesRead, out int charsWritten, replaceInvalidSequences: true, isFinalBlock: false); + if (bytesRead == 0 && charsWritten == 0) + { + // trailing partial sequence: carry it into the next segment + span.CopyTo(pending); + pendingLength = span.Length; + break; + } + + if (!AdvanceMatch(s, chars.Slice(0, charsWritten), ref matched)) return false; + span = span.Slice(bytesRead); + } + } + + if (pendingLength != 0) + { + // truncated at the end of the payload + Utf8.ToUtf16(pending.Slice(0, pendingLength), chars, out _, out int finalChars, replaceInvalidSequences: true, isFinalBlock: true); + if (!AdvanceMatch(s, chars.Slice(0, finalChars), ref matched)) return false; + } + + return matched == s.Length; + } + + /// Matches freshly decoded chars against the next part of the string; false ends the compare. + private static bool AdvanceMatch(string s, scoped ReadOnlySpan decoded, ref int matched) + { + if (matched + decoded.Length > s.Length) return false; + if (!decoded.SequenceEqual(s.AsSpan(matched, decoded.Length))) return false; + matched += decoded.Length; + return true; + } +#endif + /// /// See . /// diff --git a/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs b/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs new file mode 100644 index 000000000..4b426d0f5 --- /dev/null +++ b/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs @@ -0,0 +1,113 @@ +using System; +using System.Buffers; +using System.Text; +using BenchmarkDotNet.Attributes; + +namespace StackExchange.Redis.Benchmarks +{ + /// + /// Sizes the cost of equality where one side is a string and the other a blob. + /// That mixed case currently decodes the blob into a transient string (see operator ==), which is the + /// allocation PR #3116 set out to remove; this measures whether it is worth removing, and - via the + /// DiffAtStart case - how much is lost by comparing only after the whole payload has been materialised. + /// + [Config(typeof(SlowConfig))] + public class RedisValueEqualityBenchmarks + { + public enum Shape + { + /// Both sides identical; the full payload has to be examined either way. + Equal, + + /// Differs in the first byte: everything after it is wasted work. + DiffAtStart, + + /// Differs in the last byte: the whole payload must be examined regardless. + DiffAtEnd, + } + + [Params(16, 1024, 65536)] + public int Size { get; set; } + + [Params(Shape.Equal, Shape.DiffAtStart, Shape.DiffAtEnd)] + public Shape Form { get; set; } + + private RedisValue _string, _other, _byteArray, _sequence, _stringB; + + private static string MakeString(int size, Shape form) + { + // deliberately non-numeric: anything numeric is reduced by Simplify() and never reaches the + // string/blob comparison at all + var chars = new char[size]; + for (int i = 0; i < size; i++) chars[i] = (char)('a' + (i % 26)); + var s = new string(chars); + return form switch + { + Shape.DiffAtStart => "Z" + s.Substring(1), + Shape.DiffAtEnd => s.Substring(0, size - 1) + "Z", + _ => s, + }; + } + + private static ReadOnlySequence AsSegmented(byte[] bytes) + { + // split in the middle, so the multi-segment path is exercised rather than the fast single-span one + int mid = bytes.Length / 2; + var first = new Segment(new ReadOnlyMemory(bytes, 0, mid), null); + var second = new Segment(new ReadOnlyMemory(bytes, mid, bytes.Length - mid), first); + return new ReadOnlySequence(first, 0, second, second.Memory.Length); + } + + [GlobalSetup] + public void Setup() + { + var baseline = MakeString(Size, Shape.Equal); + var variant = MakeString(Size, Form); + + _string = baseline; + _stringB = variant; + + var bytes = Encoding.UTF8.GetBytes(variant); + _byteArray = bytes; + _sequence = AsSegmented(bytes); + _other = Encoding.UTF8.GetBytes(baseline); + } + + /// String vs byte[]: the mixed case #3116 targets. + [Benchmark(Baseline = true)] + public bool StringVsByteArray() => _string == _byteArray; + + /// String vs a multi-segment blob - same mixed case, sequence-backed. + [Benchmark] + public bool StringVsSequence() => _string == _sequence; + + /// Control: blob vs blob already compares by raw bytes, with no decode. + [Benchmark] + public bool BlobVsBlob() => _other == _byteArray; + + /// Control: string vs string, the plain managed comparison. + [Benchmark] + public bool StringVsString() => _string == _stringB; + + /// Hashing a blob: decodes the whole payload into a pooled char buffer. + [Benchmark] + public int HashBlob() => _byteArray.GetHashCode(); + + /// Hashing the equivalent string, for comparison. + [Benchmark] + public int HashString() => _string.GetHashCode(); + + private sealed class Segment : ReadOnlySequenceSegment + { + public Segment(ReadOnlyMemory value, Segment? head) + { + Memory = value; + if (head is not null) + { + RunningIndex = head.RunningIndex + head.Memory.Length; + head.Next = this; + } + } + } + } +} diff --git a/tests/StackExchange.Redis.Tests/RedisValueEquivalencyTests.cs b/tests/StackExchange.Redis.Tests/RedisValueEquivalencyTests.cs index f4ce5008e..de97ca184 100644 --- a/tests/StackExchange.Redis.Tests/RedisValueEquivalencyTests.cs +++ b/tests/StackExchange.Redis.Tests/RedisValueEquivalencyTests.cs @@ -627,4 +627,143 @@ public void RedisValueLengthNull() Assert.Equal(RedisValue.StorageType.Null, value.Type); Assert.Equal(0, value.Length()); } + + // Comparing a string against a byte-backed value no longer decodes the blob into a transient string, so + // these pin the relation itself: the answer must still be "the blob, read as UTF-8 text, equals the + // string" for every input - including text that does not survive a UTF-8 round trip, which is where a + // byte-domain comparison would silently disagree and put equal values in different hash buckets. + // + // Cases are built in code rather than as InlineData: a lone surrogate does not survive either a UTF-8 + // source file or xunit's argument serialization, and those are exactly the interesting inputs. + private static byte[][] EquivalenceBlobs() => + [ + [], + Encoding.UTF8.GetBytes("abc"), + Encoding.UTF8.GetBytes("abd"), + Encoding.UTF8.GetBytes("ab"), + Encoding.UTF8.GetBytes("abcd"), + Encoding.UTF8.GetBytes("моя строка"), // cyrillic + Encoding.UTF8.GetBytes("你好世界"), // CJK + Encoding.UTF8.GetBytes(new string([(char)0xD83D, (char)0xDE00])), // emoji: surrogate pair + Encoding.UTF8.GetBytes(new string('x', 300)), // spans several decode chunks + [0xFF], // invalid + [0xC3], // truncated 2-byte sequence + [0xE4, 0xBD], // truncated 3-byte sequence + [0xF0, 0x9F, 0x98], // truncated 4-byte sequence + [0x61, 0xFF, 0x62], // invalid byte mid-payload + [0xC0, 0xAF], // overlong encoding + [0xED, 0xA0, 0x80], // surrogate encoded as UTF-8 + [0x61, 0xF0, 0x9F, 0x98, 0x80, 0x62], // emoji mid-payload + ]; + + private static string[] EquivalenceStrings() => + [ + "", + "abc", + "abd", + "ab", + "abcd", + "моя строка", + "你好世界", + new string([(char)0xD83D, (char)0xDE00]), + new string('x', 300), + "�", + "a�b", + new string([(char)0xD800]), // lone high surrogate: UTF-8 cannot represent it + "a��b", + "a😀b", + ]; + + [Fact] + public void StringVersusBlob_MatchesDecodedComparison() + { + foreach (var blob in EquivalenceBlobs()) + { + RedisValue asBlob = blob; + var decoded = (string?)asBlob; + foreach (var s in EquivalenceStrings()) + { + RedisValue asString = s; + bool expected = s == decoded; + var because = $"'{Escape(s)}' vs {BitConverter.ToString(blob)}"; + + Assert.True(expected == (asString == asBlob), because); + Assert.True(expected == (asBlob == asString), because + " (reversed)"); + } + } + } + + [Fact] + public void StringVersusSegmentedBlob_MatchesAtEverySplit() + { + foreach (var blob in EquivalenceBlobs()) + { + if (blob.Length < 2) continue; + var decoded = (string?)(RedisValue)blob; + + // every split point, so a multi-byte sequence broken across segments is covered + for (int split = 1; split < blob.Length; split++) + { + RedisValue segmented = Segmented(blob, split); + Assert.Equal(decoded, (string?)segmented); + + foreach (var s in EquivalenceStrings()) + { + RedisValue asString = s; + Assert.True( + (s == decoded) == (asString == segmented), + $"'{Escape(s)}' vs {BitConverter.ToString(blob)} split at {split}"); + } + } + } + } + + [Fact] + public void EqualStringAndBlobShareHashCodes() + { + foreach (var blob in EquivalenceBlobs()) + { + RedisValue asBlob = blob; + foreach (var s in EquivalenceStrings()) + { + RedisValue asString = s; + if (asString != asBlob) continue; + + Assert.True( + asString.GetHashCode() == asBlob.GetHashCode(), + $"equal values must share a hash code: '{Escape(s)}' vs {BitConverter.ToString(blob)}"); + } + } + } + + private static RedisValue Segmented(byte[] payload, int split) + { + var first = new EquivalenceSegment(new ReadOnlyMemory(payload, 0, split), null); + var second = new EquivalenceSegment(new ReadOnlyMemory(payload, split, payload.Length - split), first); + return new ReadOnlySequence(first, 0, second, second.Memory.Length); + } + + private static string Escape(string value) + { + var sb = new StringBuilder(); + foreach (var c in value) + { + if (c is >= (char)32 and <= (char)126) sb.Append(c); + else sb.Append("\\u").Append(((int)c).ToString("X4")); + } + return sb.ToString(); + } + + private sealed class EquivalenceSegment : ReadOnlySequenceSegment + { + public EquivalenceSegment(ReadOnlyMemory value, EquivalenceSegment? head) + { + Memory = value; + if (head is not null) + { + RunningIndex = head.RunningIndex + head.Memory.Length; + head.Next = this; + } + } + } } From 214ce4b3fd7ae461d7b8b155ecf5a7f3e3285e04 Mon Sep 17 00:00:00 2001 From: mgravell Date: Thu, 17 Sep 2026 09:39:37 +0100 Subject: [PATCH 2/2] Review feedback: document the carry invariant and the benchmark's scope - Assert that the decoder consumed at least the carried prefix. It holds because `pending` only ever holds a sequence that was incomplete but valid, and such a sequence is consumed as a unit - but the reasoning is not local to the line, and a violation would throw from the slice rather than answer wrongly. - Say that the "no progress" branch is defence rather than a case that arises: a final-block decode of a non-empty input always yields at least the replacement character, so it is believed unreachable and is there to fail the comparison instead of spinning if that ever stops being true. - Spell out that the benchmark has no in-run before and after: its baseline is the method being changed, so comparing against a previous implementation means running it on both commits. Also drops a reference to a PR number that will read oddly once that PR is closed. --- src/StackExchange.Redis/RedisValue.cs | 10 +++++++++- .../RedisValueEqualityBenchmarks.cs | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/StackExchange.Redis/RedisValue.cs b/src/StackExchange.Redis/RedisValue.cs index 0b3820f61..3c2c2c22d 100644 --- a/src/StackExchange.Redis/RedisValue.cs +++ b/src/StackExchange.Redis/RedisValue.cs @@ -548,7 +548,10 @@ private static bool StringEqualsUtf8(string s, scoped ReadOnlySpan utf8) // what remains is a truncated sequence at the very end; decoding it as final is what // yields the replacement char that the string form would have carried Utf8.ToUtf16(utf8, chars, out bytesRead, out charsWritten, replaceInvalidSequences: true, isFinalBlock: true); - if (bytesRead == 0 && charsWritten == 0) return false; // no progress: cannot match + // Defensive, and believed unreachable: a final-block decode of a non-empty input always + // yields at least the replacement character. Kept so that a future change which makes it + // reachable fails the comparison rather than spinning here forever. + if (bytesRead == 0 && charsWritten == 0) return false; } if (!AdvanceMatch(s, chars.Slice(0, charsWritten), ref matched)) return false; utf8 = utf8.Slice(bytesRead); @@ -584,6 +587,11 @@ private static bool StringEqualsUtf8(string s, scoped in ReadOnlySequence } if (!AdvanceMatch(s, chars.Slice(0, joinedChars), ref matched)) return false; + // The decoder consumed the carried prefix and possibly more, never less: `pending` + // only ever holds a sequence that was incomplete but valid, and such a sequence is + // consumed as a unit. Asserted because if that ever stopped holding, the slice below + // would throw rather than quietly answer wrongly. + Debug.Assert(joinedBytes >= pendingLength, "decoder consumed less than the carried prefix"); span = span.Slice(joinedBytes - pendingLength); // give back the bytes we borrowed but did not use pendingLength = 0; continue; diff --git a/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs b/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs index 4b426d0f5..de695b48f 100644 --- a/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs +++ b/tests/StackExchange.Redis.Benchmarks/RedisValueEqualityBenchmarks.cs @@ -7,10 +7,21 @@ namespace StackExchange.Redis.Benchmarks { /// /// Sizes the cost of equality where one side is a string and the other a blob. - /// That mixed case currently decodes the blob into a transient string (see operator ==), which is the - /// allocation PR #3116 set out to remove; this measures whether it is worth removing, and - via the - /// DiffAtStart case - how much is lost by comparing only after the whole payload has been materialised. /// + /// + /// + /// That mixed case is the one with room to move: everything else already compares in place. The + /// DiffAtStart shape is the interesting one, since it separates the cost of comparing from the cost of + /// materialising - an implementation that decodes before it compares pays the same for a value that + /// differs in its first byte as for one that matches. + /// + /// + /// Note that the baseline here is , so the ratio column compares the other + /// shapes against the mixed case *within one build*. Comparing an implementation against its predecessor + /// means running this on both commits and lining the two tables up by hand; there is no in-run before and + /// after. + /// + /// [Config(typeof(SlowConfig))] public class RedisValueEqualityBenchmarks { @@ -73,7 +84,7 @@ public void Setup() _other = Encoding.UTF8.GetBytes(baseline); } - /// String vs byte[]: the mixed case #3116 targets. + /// String vs byte[]: the mixed case, where one side has to be converted to meet the other. [Benchmark(Baseline = true)] public bool StringVsByteArray() => _string == _byteArray;