Skip to content

Add RedisValue.EqualityComparer, with an opt-in binary reading - #3230

Open
mgravell wants to merge 1 commit into
marc/redisvalue-equality-allocfrom
marc/redisvalue-binary-comparer
Open

mgravell wants to merge 1 commit into
marc/redisvalue-equality-allocfrom
marc/redisvalue-binary-comparer

Conversation

@mgravell

Copy link
Copy Markdown
Collaborator

Stacked on #3228; retarget to main if that lands first. Replaces the approach in #3229, which I closed.

Why this shape

RedisValue equality works on the text a value decodes to — that is what lets a blob equal the string it
spells, and GetHashCode hashes the same decoded form to stay consistent with it. For callers moving large
payloads that reading costs more than they need.

#3229 tried making the type itself cheaper, and #3116 tried making its Equals byte-based. Both run into the
same wall: RedisValue.Equals(object) accepts a string, and RedisValue.GetHashCode() currently agrees with
string.GetHashCode() for the same text, so a Hashtable keyed by RedisValue and probed by string works
today. 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.

var byBytes = new Dictionary<RedisValue, T>(RedisValue.EqualityComparer.Binary);

Shape

An abstract base with Default and Binary, implementing IEqualityComparer<RedisValue> and the untyped
IEqualityComparer. The untyped side accepts whatever RedisValue itself accepts from objectstring,
byte[], the numerics — through the same forgiving TryParse that Equals(object) uses, rather than
demanding a boxed RedisValue.

Derivation is closed via a private protected constructor. Nobody needs to derive from this to write their own
rules — IEqualityComparer<RedisValue> is right there — and leaving it open would fix the shape permanently.
(closed would 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 ZRANGEBYLEX orders member bytes rather than values. If ordering is ever wanted it
should 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:

  • a string holding an unpaired surrogate encodes to the same bytes as one holding U+FFFD, so Binary calls
    those equal and Default does not;
  • a blob that is not canonical UTF-8 decodes to U+FFFD but does not re-encode to itself (0xFF), so Binary
    calls it distinct from that text and Default calls them equal.

I only found the second because the differential test failed on '�' vs FF; my first pass at the
documentation claimed lone surrogates were the only divergence. Both are now pinned by tests.

Numbers

Against Default on the same matrix (net10.0, string vs byte[]):

Size Form Default Binary
16 Equal 56.3 ns 10.8 ns 5.2×
1024 Equal 131.9 ns 41.1 ns 3.2×
1024 DiffAtStart 54.9 ns 15.2 ns 3.6×
65536 Equal 5,397 ns 2,155 ns 2.5×
65536 DiffAtStart 52.4 ns 15.5 ns 3.4×
65536 DiffAtEnd 5,793 ns 1,932 ns 3.0×

Hashing 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:

  1. Binary was initially 61× slower than Default for 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.
  2. The GetByteCount() precheck was the floor rather than the comparison — measuring a string's UTF-8 length
    walks all of it. Moving it behind the fast paths took 64KB/DiffAtStart from 897 ns to 15.5 ns.

Testing

Default is asserted to match the type's own == and GetHashCode exactly, so it is a true no-op wrapper.
Binary is asserted to agree with Default on everything that round-trips, to diverge in exactly the two
documented ways, to be self-consistent (equal implies equal hash) across malformed UTF-8 and segmented blobs,
and to work as a Dictionary comparer. The untyped API is covered for string/byte[]/RedisValue and for
inputs it cannot read.

Full suite: 6372 passed, 0 failed. All six TFMs build with 0 warnings.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant