Backport bloom filter improvements #5026
Merged
Conversation
This file contains 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
str4d
added
C-upstream-port
|
@zkbot try |
zkbot
added a commit
that referenced
this pull request
Mar 5, 2021
Backport bloom filter improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7113 - bitcoin/bitcoin#7818 - Only the second commit (to resolve conflicts). - bitcoin/bitcoin#7934 - bitcoin/bitcoin#8655 - Partial backport to help resolve conflicts. - bitcoin/bitcoin#9060 - bitcoin/bitcoin#9223 - bitcoin/bitcoin#9644 - Partial backport to help resolve conflicts. - bitcoin/bitcoin#9916 - bitcoin/bitcoin#9750 - bitcoin/bitcoin#13176 - bitcoin/bitcoin#13948 - bitcoin/bitcoin#16073 - bitcoin/bitcoin#18670 - bitcoin/bitcoin#18806 - Reveals upstream's covert fix for CVE-2013-5700. - bitcoin/bitcoin#19968
str4d
commented
Mar 5, 2021
|
☀️ Test successful - pr-try |
str4d
force-pushed
the
bloom-filter-backports
branch
from
bb2463a
to
4e9665b
Compare
|
☔ The latest upstream changes (presumably #5009) made this pull request unmergeable. Please resolve the merge conflicts. |
For each 'bit' in the filter we really maintain 2 bits, which store either: 0: not set 1-3: set in generation N After (nElements / 2) insertions, we switch to a new generation, and wipe entries which already had the new generation number, effectively switching from the last 1.5 * nElements set to the last 1.0 * nElements set. This is 25% more space efficient than the previous implementation, and can (at peak) store 1.5 times the requested amount of history (though only 1.0 times the requested history is guaranteed). The existing unit tests should be sufficient. (cherry picked from commit 086ee67)
(cherry picked from commit d03e466)
This patch changes the implementation from one that stores 16 2-bit integers in one uint32_t's, to one that stores the first bit of 64 2-bit integers in one uint64_t and the second bit in another. This allows for 450x faster refreshing and 2.2x faster average speed. (cherry picked from commit 1953c40)
(cherry picked from commit 4731cab) Zcash: Excluding changes to code we haven't backported.
Fixes newly initialized bloom filters being constructed with isEmpty(false), which still works but loses the possible speedup when checking for key membership in an empty filter. (cherry picked from commit cccf73d)
Output instances of "BloomFilter" changed to "Bloom filter", in accordance with Wikipedia standard notation: https://en.wikipedia.org/wiki/Bloom_filter also to sync with the majority of cases in the self-same file (cherry picked from commit b7aa290)
(cherry picked from commit b7b48c8) Zcash: Excluding changes to code we haven't backported yet that cause too many conflicts.
On msvc14, the compiler error C4146 (unary minus operator applied to unsigned type, result still unsigned) had been occured. Use '0 - x' styled formula instead of '-x' so as to fix the error. (cherry picked from commit 292112f)
…ed type) On msvc14, int literal '-2147483648' is invalid, because '2147483648' is unsigned type and cant't apply minus operator to unsigned type. To define the int literal correctly, use '-2147483647 - 1' formula that is also used to define INT_MIN in limits.h. (cherry picked from commit 8e0720b)
(cherry picked from commit 64aa36e)
Replaces the slow modulo operation with a much faster 32bit multiplication & shift. This works because the hash should be uniformly distributed between 0 and 2^32-1. This speeds up the benchmark by a factor of about 1.3: RollingBloom, 5, 1500000, 3.73733, 4.97569e-07, 4.99002e-07, 4.98372e-07 # before RollingBloom, 5, 1500000, 2.86842, 3.81630e-07, 3.83730e-07, 3.82473e-07 # FastMod Be aware that this changes the position of the bits that are toggled, so this should probably not be used for CBloomFilter which is serialized. (cherry picked from commit 9aac9f9)
(cherry picked from commit d2dbc7d)
(cherry picked from commit df9e15f)
Co-authored-by: MarcoFalke <falke.marco@gmail.com> (cherry picked from commit 69ffddc) Zcash: Excluding change to src/test/fuzz/bloom_filter.cpp which we don't have (we haven't backported upstream's fuzzing framework).
(cherry picked from commit 1ad8ea2) Zcash: Excluding change to src/test/fuzz/bloom_filter.cpp which we don't have (we haven't backported upstream's fuzzing framework).
(cherry picked from commit d9141a0002bb508b2e94e206a1bd28ef8f97ffde)
str4d
force-pushed
the
bloom-filter-backports
branch
from
4e9665b
to
1dd379a
Compare
nuttycom
approved these changes
Apr 15, 2021
|
@zkbot r+ |
|
📌 Commit 1dd379a has been approved by |
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.
Cherry-picked from the following upstream PRs:
CBloomFilterconstructor. bitcoin/bitcoin#13948