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

Replace unbounded with bounded as single channel is used #2646

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

ksolana
Copy link

@ksolana ksolana commented Aug 17, 2024

We need the receiver to hold just one message so there is no need to have an unbounded channel.

@ksolana ksolana force-pushed the bounded branch 2 times, most recently from 92cc4f0 to 6dd94d5 Compare August 19, 2024 14:40
@ksolana ksolana requested a review from apfitzge August 20, 2024 18:14
@apfitzge
Copy link

Can you add a description to the PR so there's some record of what this does aside from code?

@ksolana ksolana force-pushed the bounded branch 2 times, most recently from 73bb6af to 9b37d1c Compare August 21, 2024 17:29
@apfitzge
Copy link

@ksolana Is there any affect on performance shown by benchmark from this change?

@ksolana
Copy link
Author

ksolana commented Aug 22, 2024

@ksolana Is there any affect on performance shown by benchmark from this change?

Not really

base

running 6 tests
test bench_arc_mutex_poh_batched_hash            5,614,227.10 ns/iter (+/- 147,405.37)
test bench_arc_mutex_poh_hash                    5,748,522.90 ns/iter (+/- 118,654.13)
test bench_poh_hash                              5,577,250.00 ns/iter (+/- 90,381.88)
test bench_poh_lock_time_per_batch                  11,961.97 ns/iter (+/- 174.94)
test bench_poh_recorder_record_transaction_index     1,310.42 ns/iter (+/- 203.48)
test bench_poh_recorder_set_bank                 5,468,741.60 ns/iter (+/- 100.84)
test bench_poh_verify_ticks                     11,094,366.60 ns/iter (+/- 2,023,844.24)
test bench_poh_verify_transaction_entries       11,051,016.70 ns/iter (+/- 1,010,783.40)

diff

test bench_arc_mutex_poh_batched_hash            5,618,408.35 ns/iter (+/- 78,767.67)
test bench_arc_mutex_poh_hash                    5,761,056.30 ns/iter (+/- 1,493,131.64)
test bench_poh_hash                              5,604,585.40 ns/iter (+/- 81,242.69)
test bench_poh_lock_time_per_batch                  11,455.62 ns/iter (+/- 209.70)
test bench_poh_recorder_record_transaction_index     1,216.27 ns/iter (+/- 47.17)
test bench_poh_recorder_set_bank                 5,468,741.80 ns/iter (+/- 191.19)
test bench_poh_verify_ticks                     11,017,170.80 ns/iter (+/- 976,551.66)
test bench_poh_verify_transaction_entries       10,998,991.60 ns/iter (+/- 1,041,723.92)

We need the receiver to hold just one message so there is no need
to have an unbounded channel.
@apfitzge
Copy link

Thanks, wouldn't expect a huge difference, if any. I think previously we would do:

  1. create unbounded with no allocation
  2. send call will allocate space for 1 element

Now we just

  1. create bounded(1) with allocation for 1 element
  2. send call just populates it

@ksolana ksolana merged commit 35051c7 into anza-xyz:master Aug 22, 2024
40 checks passed
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
We need the receiver to hold just one message so there is no need
to have an unbounded channel.
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