Skip to content

[Fix] Correct RECLAIM_KEY digest in remove_kernel#240

Merged
rhdong merged 1 commit intoNVIDIA-Merlin:masterfrom
klzwii:fix/remove-kernel-incorrect-digest
Feb 18, 2026
Merged

[Fix] Correct RECLAIM_KEY digest in remove_kernel#240
rhdong merged 1 commit intoNVIDIA-Merlin:masterfrom
klzwii:fix/remove-kernel-incorrect-digest

Conversation

@klzwii
Copy link

@klzwii klzwii commented Feb 2, 2026

Description: This PR fixes a performance issue in remove_kernel and remove_kernel_v2 where the digest for a RECLAIM_KEY was incorrectly set to empty_digest().

Issue: When a key is removed and marked as RECLAIM_KEY, setting its digest to empty_digest causes the probing logic to falsely identify the slot as a potential candidate for an empty key. This triggers unnecessary memory accesses (loading the Key) to confirm the slot status, only to discover it is actually a RECLAIM_KEY and continue probing.

Fix: Updated the code to set the digest to reclaim_digest() instead of empty_digest. This allows the SIMD filter to correctly distinguish between true Empty slots and Reclaimed slots, avoiding redundant Key loads and improving probing efficiency.

Set `RECLAIM_KEY` digest to `reclaim_digest` instead of `empty_digest`.
This prevents probing kernels from mistakenly identifying reclaimed slots as empty candidates, reducing unnecessary memory accesses and improving performance.
@rhdong rhdong requested a review from jiashuy February 3, 2026 02:05
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

@rhdong
Copy link
Member

rhdong commented Feb 3, 2026

Hi @klzwii , thanks a lot for the contribution! Looping in @jiashuy to help review and merge this 👍

@rhdong
Copy link
Member

rhdong commented Feb 11, 2026

/blossom-ci

@rhdong
Copy link
Member

rhdong commented Feb 12, 2026

Hi @jiashuy , the CI doesn't work. Could you help check it? Thanks!

@rhdong rhdong self-requested a review February 17, 2026 23:07
Copy link
Member

@rhdong rhdong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rhdong
Copy link
Member

rhdong commented Feb 18, 2026

The Blossom CI is currently broken and requires additional time to migrate to the new CI runner. I have run all tests locally on an A6000, and they all passed. Therefore, I’ve approved and merged the changes.

Thanks to @klzwii for the contribution! cc @jiashuy

@rhdong rhdong merged commit 84017e2 into NVIDIA-Merlin:master Feb 18, 2026
1 of 2 checks passed
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