Use XOR with shift for cache key calculation - #413
Open
kelhusseiny wants to merge 1 commit into
Open
Conversation
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
force-pushed
the
karim/fix-cache-key-collision
branch
from
January 29, 2026 12:48
14f550e to
969ad48
Compare
kelhusseiny
marked this pull request as ready for review
January 29, 2026 12:49
gmalette
approved these changes
Jan 29, 2026
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
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.
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
Benchmark Results
XOR with shift is ~15% faster than rotation while maintaining the same collision-resistance properties.