Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove while loop for setbit & getbit #2687

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

dozyio
Copy link
Contributor

@dozyio dozyio commented Sep 6, 2024

Description

Constant time setbit and getbit in the bloom filter - was hitting performance issues when using a 30mb bloom filter

Notes & open questions

Benchmarks below

Loop 8bits x 171,777,298 ops/sec ±2.70% (89 runs sampled)
Math Ops 8bits x 216,520,449 ops/sec ±6.46% (77 runs sampled)

Loop 64bits x 127,603,004 ops/sec ±3.28% (85 runs sampled)
Math Ops 64bits x 225,485,149 ops/sec ±5.94% (81 runs sampled)

Loop 512bits x 35,893,950 ops/sec ±0.76% (97 runs sampled)
Math Ops 512bits x 223,396,999 ops/sec ±5.05% (79 runs sampled)

Loop 4096bits x 4,824,731 ops/sec ±0.91% (97 runs sampled)
Math Ops 4096bits x 211,234,954 ops/sec ±5.26% (77 runs sampled)

Loop 32768bits x 579,890 ops/sec ±0.40% (100 runs sampled)
Math Ops 32768bits x 206,112,847 ops/sec ±5.97% (74 runs sampled)

Loop 262144bits x 60,922 ops/sec ±0.15% (98 runs sampled)
Math Ops 262144bits x 203,502,874 ops/sec ±6.71% (72 runs sampled)

Loop 2097152bits x 7,626 ops/sec ±0.26% (101 runs sampled)
Math Ops 2097152bits x 210,668,544 ops/sec ±5.40% (76 runs sampled)

Loop 251658240bits x 63.70 ops/sec ±0.18% (68 runs sampled)
Math Ops 251658240bits x 209,501,599 ops/sec ±6.41% (73 runs sampled)

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@dozyio dozyio changed the title perf(bloom-filter): remove while loop for setbit & getbit fix(bloom-filter): remove while loop for setbit & getbit Sep 6, 2024
@dozyio dozyio marked this pull request as ready for review September 6, 2024 15:48
@dozyio dozyio requested a review from a team as a code owner September 6, 2024 15:48
@achingbrain
Copy link
Member

This looks good, thanks for opening it.

Out of interest, where are you using BloomFilter?

I can only see it used once in the libp2p codebase (for filtering circuit relay servers) - in general I think you'd want to use ScalableCuckooFilter instead as it can grow beyond the initially configured size.

@dozyio
Copy link
Contributor Author

dozyio commented Sep 9, 2024

This looks good, thanks for opening it.

Out of interest, where are you using BloomFilter?

I can only see it used once in the libp2p codebase (for filtering circuit relay servers) - in general I think you'd want to use ScalableCuckooFilter instead as it can grow beyond the initially configured size.

Hey Alex - I'm using it in https://github.com/dozyio/js-ds-crdt to check if keys are deleted from a set - was going to use bloom-filters but saw your issue Callidon/bloom-filters#70 - the one in utils should be fine once this pr is applied

@achingbrain achingbrain changed the title fix(bloom-filter): remove while loop for setbit & getbit fix: remove while loop for setbit & getbit Sep 10, 2024
@achingbrain achingbrain merged commit a142bb6 into libp2p:main Sep 10, 2024
21 checks passed
@achingbrain
Copy link
Member

Fair enough. This implementation might get removed in the future since we could just use a cuckoo filter for filtering relay servers but hopefully the bloom-filters module will ship it's v4 at some point soon.

@dozyio dozyio deleted the perf-bloom-filter branch September 10, 2024 11:40
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