Skip to content

Use XOR with shift for cache key calculation - #413

Open
kelhusseiny wants to merge 1 commit into
mainfrom
karim/fix-cache-key-collision
Open

Use XOR with shift for cache key calculation#413
kelhusseiny wants to merge 1 commit into
mainfrom
karim/fix-cache-key-collision

Conversation

@kelhusseiny

Copy link
Copy Markdown
Member

Summary

Changes the cache key calculation in CompiledMetric from rotate-left + XOR to XOR with position-based shifts.

Motivation

The previous implementation used a rotate-left + XOR approach which is more complex and slower. Benchmarking shows that XOR with shift achieves the same performance as plain XOR while maintaining order-dependency to prevent collisions when tag values are swapped.

Changes

  • Replaced rotate-left + XOR with simpler XOR + position-based shifts
  • Updated tests and comments to reflect the new algorithm

Benchmark Results

Calculating -------------------------------------
                 XOR     19.702 (±15.2%) i/s   (50.76 ms/i) -     97.000 in   5.025484s
   XOR with rotation     16.574 (± 6.0%) i/s   (60.34 ms/i) -     83.000 in   5.020653s
      XOR with shift     19.240 (±10.4%) i/s   (51.97 ms/i) -     96.000 in   5.021512s

Comparison:
                 XOR:       19.7 i/s
      XOR with shift:       19.2 i/s - same-ish: difference falls within error

XOR with shift is ~15% faster than rotation while maintaining the same collision-resistance properties.

The previous XOR-based cache key computation was commutative, meaning
swapped tag values would produce the same cache key. For example:
(true, false) and (false, true) would both produce the same hash.

This replaces XOR with rotate-left + XOR:
- Rotation makes it order-dependent, preventing collisions when values swap
- XOR keeps the result bounded, avoiding Bignum allocations

The multiply-and-add approach (like Java's Objects.hash) was considered
but causes Bignum allocations when hash values overflow during multiplication.
@kelhusseiny
kelhusseiny force-pushed the karim/fix-cache-key-collision branch from 14f550e to 969ad48 Compare January 29, 2026 12:48
@kelhusseiny
kelhusseiny marked this pull request as ready for review January 29, 2026 12:49
gmalette added a commit to gmalette/statsd-instrument that referenced this pull request Aug 3, 2026
The tag-combination cache keys precompiled datagrams by a 32-bit
rotate-left-5 + XOR hash of the tag values. Collisions are detected by
comparing full tag values, but never resolved: the losing tag combination
is never cached, so every emit of it for the rest of the process lifetime
allocates a fresh PrecompiledDatagram (the allocation CompiledMetric exists
to avoid) and increments
statsd_instrument.compiled_metric.hash_collision_detected. Once a colliding
pair lands among hot tag combinations, the counter fires continuously until
the process restarts, and because String#hash is seeded per process, which
pair collides differs per process, making the signal intermittent and
fleet-wide.

At 32 bits this is inevitable at real volumes. A process that has cached D
distinct tag combinations sees expected collisions of roughly D^2 / 2^33;
with the cache holding up to 5000 entries and hot metrics seeing far more
distinct combinations looked up against it, a busy fleet produces a steady
stream of collision events every day.

## Fix

Widen the key to 57 bits at zero cost. The only operation that can escape
Fixnum range is the intermediate (__cache_key__ << 5), and 57-bit keys keep
it within 2**62 - 1 on 64-bit CRuby:

  k <= 2**57 - 1
  k << 5 <= 2**62 - 32
  (k << 5) | (k >> 52) <= 2**62 - 1

String#hash exposes ~60 bits of SipHash entropy, so the extra 25 bits are
real. Expected collisions per process drop by 2^25 (~33 million-fold),
turning a daily occurrence into a never-in-practice one, with the same op
count and no new allocations on either the hit or miss path. The collision
detection branch stays as a correctness guard.

This touches the same lines as Shopify#413 but is orthogonal: that changes the
mixing function for speed, this widens the key space. Either can land
first; the conflict is trivial to resolve.

Assisted-By: devx/07082c33-4cad-46be-b311-20f734743ed7
gmalette added a commit to gmalette/statsd-instrument that referenced this pull request Aug 3, 2026
The tag-combination cache keys precompiled datagrams by a 32-bit
rotate-left-5 + XOR hash of the tag values. Collisions are detected by
comparing full tag values, but never resolved: the losing tag combination
is never cached, so every emit of it for the rest of the process lifetime
allocates a fresh PrecompiledDatagram (the allocation CompiledMetric exists
to avoid) and increments
statsd_instrument.compiled_metric.hash_collision_detected. Once a colliding
pair lands among hot tag combinations, the counter fires continuously until
the process restarts, and because String#hash is seeded per process, which
pair collides differs per process, making the signal intermittent and
fleet-wide.

At 32 bits this is inevitable at real volumes. A process that has cached D
distinct tag combinations sees expected collisions of roughly D^2 / 2^33;
with the cache holding up to 5000 entries and hot metrics seeing far more
distinct combinations looked up against it, a busy fleet produces a steady
stream of collision events every day.

## Fix

Widen the key to 57 bits at zero cost. The only operation that can escape
Fixnum range is the intermediate (__cache_key__ << 5), and 57-bit keys keep
it within 2**62 - 1 on 64-bit CRuby:

  k <= 2**57 - 1
  k << 5 <= 2**62 - 32
  (k << 5) | (k >> 52) <= 2**62 - 1

String#hash exposes ~60 bits of SipHash entropy, so the extra 25 bits are
real. Expected collisions per process drop by 2^25 (~33 million-fold),
turning a daily occurrence into a never-in-practice one, with the same op
count and no new allocations on either the hit or miss path. The collision
detection branch stays as a correctness guard.

This touches the same lines as Shopify#413 but is orthogonal: that changes the
mixing function for speed, this widens the key space. Either can land
first; the conflict is trivial to resolve.

Assisted-By: devx/07082c33-4cad-46be-b311-20f734743ed7
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.

2 participants