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

Speed up packet dedup and fix benches #22592

Merged
merged 3 commits into from Jan 20, 2022

Conversation

jstarry
Copy link
Member

@jstarry jstarry commented Jan 20, 2022

Problems

  • Packet deduping is slow for larger packets but we always add the full 1232 data buffer to the bloom filter
  • It's slow to iterate over the bloom filter keys twice (once for contains and once for add)

Summary of Changes

  • Only add the used data buffer portion of a packet to the bloom filter
  • Modify AtomicBloom:add to return whether the item was newly added or not

Before

test bench_dedup_diff_big_packets   ... bench:  20,010,316 ns/iter (+/- 1,623,356)
test bench_dedup_diff_small_packets ... bench:  19,772,433 ns/iter (+/- 1,470,336)
test bench_dedup_same_big_packets   ... bench:  17,869,979 ns/iter (+/- 1,158,488)
test bench_dedup_same_small_packets ... bench:  18,181,612 ns/iter (+/- 2,449,875)

After

test bench_dedup_diff_big_packets   ... bench:  15,266,433 ns/iter (+/- 1,096,914)
test bench_dedup_diff_small_packets ... bench:   1,943,839 ns/iter (+/- 239,305)
test bench_dedup_same_big_packets   ... bench:  16,299,237 ns/iter (+/- 570,619)
test bench_dedup_same_small_packets ... bench:   3,363,462 ns/iter (+/- 2,007,400)

Fixes #

Copy link
Member

@sakridge sakridge left a comment

Choose a reason for hiding this comment

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

lgtm

@jstarry jstarry added the automerge Merge this Pull Request automatically once CI passes label Jan 20, 2022
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Jan 20, 2022
@mergify
Copy link
Contributor

mergify bot commented Jan 20, 2022

automerge label removed due to a CI failure

@codecov
Copy link

codecov bot commented Jan 20, 2022

Codecov Report

Merging #22592 (4460c2c) into master (6edeed8) will increase coverage by 0.4%.
The diff coverage is 91.7%.

@@            Coverage Diff            @@
##           master   #22592     +/-   ##
=========================================
+ Coverage    81.1%    81.5%   +0.4%     
=========================================
  Files         560      555      -5     
  Lines      151206   149654   -1552     
=========================================
- Hits       122633   122086    -547     
+ Misses      28573    27568   -1005     

Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

nice!

@t-nelson t-nelson merged commit a2d251c into solana-labs:master Jan 20, 2022
mergify bot pushed a commit that referenced this pull request Jan 20, 2022
* Speed up packet dedup and fix benches

* fix tests

* allow int arithmetic in bench

(cherry picked from commit a2d251c)

# Conflicts:
#	perf/src/sigverify.rs
mergify bot pushed a commit that referenced this pull request Jan 20, 2022
* Speed up packet dedup and fix benches

* fix tests

* allow int arithmetic in bench

(cherry picked from commit a2d251c)
mergify bot added a commit that referenced this pull request Jan 20, 2022
* Speed up packet dedup and fix benches

* fix tests

* allow int arithmetic in bench

(cherry picked from commit a2d251c)

Co-authored-by: Justin Starry <justin@solana.com>
@jstarry jstarry deleted the speed-up-dedupe branch January 21, 2022 00:26
mergify bot added a commit that referenced this pull request Jan 21, 2022
* Speed up packet dedup and fix benches (#22592)

* Speed up packet dedup and fix benches

* fix tests

* allow int arithmetic in bench

(cherry picked from commit a2d251c)

# Conflicts:
#	perf/src/sigverify.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>
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.

None yet

3 participants