-
Notifications
You must be signed in to change notification settings - Fork 11.2k
Clarify BloomFilter.put return value under concurrency - #8597
Clarify BloomFilter.put return value under concurrency #8597NgoQuocViet2001 wants to merge 1 commit into
Conversation
Concurrent puts of the same object can each change different bits and return true. Limit the first-insertion and opposite-return guarantees to nonconcurrent calls and document the concurrent case.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
f2226ae to
e59b2ce
Compare
@MuhammadNiazAli
MuhammadNiazAli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh bhi ek achha open PR hai (google/guava #8597) chaliye is ka review likhte hain:
Review:
This is a meaningful correctness clarification for concurrent usage of BloomFilter.put(), and the change is well-scoped to just the Javadoc.
Core addition is the right one: the new sentence "In the absence of concurrent calls to put, ... With concurrent calls, multiple calls may return true for the same object" directly addresses a subtle race that the old doc glossed over. Without this qualifier, a reader could reasonably (and incorrectly) assume the "opposite of mightContain" guarantee always holds — which breaks down under concurrent puts racing on the same bits.
Why this matters in practice: someone using the return value of put() to decide "am I the first caller to add this object" (a common dedup pattern with Bloom filters) needs to know that under concurrency this signal isn't reliable multiple threads can all see true. Calling that out explicitly, rather than leaving it implied, is exactly the kind of thing that prevents a subtle production bug.
Wording is precise: "In the absence of concurrent calls to put" as the qualifier for the deterministic behavior, followed by the concurrent caveat at the end, reads clearly and doesn't require re-reading the whole doc block to understand which part applies when.
One thing worth double-checking: the doc doesn't say whether all concurrent callers can get true, or just "more than one" — i.e., is it possible for zero callers to get true even though the object was in fact newly added? If that's a real possibility (depends on the implementation's synchronization), it might be worth clarifying so readers don't assume "at least one caller gets true" as a hard guarantee. If the implementation guarantees at least one true, that's worth stating too, since it's the more useful invariant for callers.
Scope is clean single doc comment, no logic touched, matches the PR title exactly.
Good, precise fix. Approving, with the note above as an optional follow-up for extra clarity.
NgoQuocViet2001
commented
Sep 3, 2026
I signed it!
Fixes #3055.
BloomFilter.putis lock-free, so concurrent calls for the same object can each set a different subset of bits and both returntrue. The existing Javadoc instead states that atrueresult definitely marks the first insertion.This change scopes the first-insertion and
mightContainreturn-value guarantees to nonconcurrent calls and documents the concurrent case. Runtime behavior is unchanged.Tests:
.\mvnw.cmd -B -ntp clean install(865,730 tests; 0 failures or errors)