Conversation
Comparing and hashing a RedisValue works on the text a value decodes to, which is what lets a blob equal the string it spells. For callers slinging large payloads that reading costs more than they want, and for them the bytes would do just as well. Rather than change what the type itself means - see #3229, where doing so turned out to break a Hashtable keyed by RedisValue and probed by string, silently - offer the byte reading as something a caller asks for: new Dictionary<RedisValue, T>(RedisValue.EqualityComparer.Binary) An abstract base with Default and Binary, implementing both the typed and the untyped comparer interfaces; the untyped side accepts anything RedisValue itself accepts from object - string, byte[], the numerics - via the same forgiving TryParse that Equals(object) uses. Derivation is closed via a private protected constructor: nobody needs to derive from this to write their own rules, and leaving it open would fix the shape forever. Deliberately equality only. A total order would have to choose between the numeric, textual and raw-byte readings of a value, and those disagree; Redis does not define one either, since sorted sets order by score and ZRANGEBYLEX orders member bytes. Binary agrees with Default wherever a value's UTF8 form round-trips - all well-formed text - and differs where it does not, in both directions: a string holding an unpaired surrogate encodes as U+FFFD does, and a blob that is not canonical UTF8 decodes to U+FFFD without re-encoding to itself. Both its equality and its hashing read the same bytes, so it stays self-consistent; that is what makes the byte reading safe here and not on the type itself, whose GetHashCode hashes decoded text. Measured against Default on the same matrix (net10.0, string vs byte[]): 16B equal 56.3ns -> 10.8ns 1KB equal 131.9ns -> 41.1ns 1KB differs first 54.9ns -> 15.2ns 64KB equal 5,397ns -> 2,155ns 64KB differs first 52.4ns -> 15.5ns hash 64KB blob 31,026ns -> 884ns Two things the benchmarks caught that the tests could not: comparing a string against a blob has to encode a chunk at a time onto the blob's own bytes, or a mismatch in the first byte still costs a full pass over both sides; and the length precheck has to come after that, because measuring a string's UTF8 length walks all of it and dominates everything else.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #3228; retarget to
mainif that lands first. Replaces the approach in #3229, which I closed.Why this shape
RedisValueequality works on the text a value decodes to — that is what lets a blob equal the string itspells, and
GetHashCodehashes the same decoded form to stay consistent with it. For callers moving largepayloads that reading costs more than they need.
#3229 tried making the type itself cheaper, and #3116 tried making its
Equalsbyte-based. Both run into thesame wall:
RedisValue.Equals(object)accepts astring, andRedisValue.GetHashCode()currently agrees withstring.GetHashCode()for the same text, so aHashtablekeyed byRedisValueand probed bystringworkstoday. Changing either half silently breaks that, and it is not a change that belongs outside a major.
A comparer sidesteps it entirely: it owns both halves of its own contract, so the byte reading is
self-consistent within it, and nothing changes for anyone who does not ask.
Shape
An abstract base with
DefaultandBinary, implementingIEqualityComparer<RedisValue>and the untypedIEqualityComparer. The untyped side accepts whateverRedisValueitself accepts fromobject—string,byte[], the numerics — through the same forgivingTryParsethatEquals(object)uses, rather thandemanding a boxed
RedisValue.Derivation is closed via a
private protectedconstructor. Nobody needs to derive from this to write their ownrules —
IEqualityComparer<RedisValue>is right there — and leaving it open would fix the shape permanently.(
closedwould say this more directly but is C# 15; we are on 14.)Equality only, deliberately. A total order would have to choose between the numeric, textual and raw-byte
readings of a value, and those disagree for the same logical value. Redis does not define one either: sorted
sets order by score, and
ZRANGEBYLEXorders member bytes rather than values. If ordering is ever wanted itshould be named for its domain rather than offered as a general "comparer".
Where Binary differs from Default
It agrees wherever a value's UTF-8 form round-trips — all well-formed text — and differs where it does not, in
both directions:
Binarycallsthose equal and
Defaultdoes not;0xFF), soBinarycalls it distinct from that text and
Defaultcalls them equal.I only found the second because the differential test failed on
'�' vs FF; my first pass at thedocumentation claimed lone surrogates were the only divergence. Both are now pinned by tests.
Numbers
Against
Defaulton the same matrix (net10.0, string vsbyte[]):DefaultBinaryHashing is the bigger story, since it never decodes: 30.4 → 4.5 ns (16B), 503.7 → 28.2 ns (1KB),
31,026 → 884 ns (64KB, 35×). Blob-against-blob: 34.6 → 5.8 ns at 16B, and 38.7 → 5.6 ns for an early
mismatch at 64KB.
Two defects the benchmarks caught that the tests could not have:
Binarywas initially 61× slower thanDefaultfor an early mismatch at 64KB (3,213 ns vs 52 ns),because it materialised both byte forms before comparing anything. It now encodes the string a chunk at a
time directly onto the blob's own bytes.
GetByteCount()precheck was the floor rather than the comparison — measuring a string's UTF-8 lengthwalks all of it. Moving it behind the fast paths took 64KB/DiffAtStart from 897 ns to 15.5 ns.
Testing
Defaultis asserted to match the type's own==andGetHashCodeexactly, so it is a true no-op wrapper.Binaryis asserted to agree withDefaulton everything that round-trips, to diverge in exactly the twodocumented ways, to be self-consistent (equal implies equal hash) across malformed UTF-8 and segmented blobs,
and to work as a
Dictionarycomparer. The untyped API is covered forstring/byte[]/RedisValueand forinputs it cannot read.
Full suite: 6372 passed, 0 failed. All six TFMs build with 0 warnings.