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

feat: simple bench target for measuring end to end query times #2433

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

netrome
Copy link
Contributor

@netrome netrome commented Nov 13, 2024

Linked Issues/PRs

Description

This PR adds a benchmark target for measuring end to end query times for transaction queries. The benchmark is a simple binary that submits a bunch of transactions, and later queries these transactions by owner several times. It is intended to be used together with benchmarking tools such as hyperfine.

@netrome netrome linked an issue Nov 13, 2024 that may be closed by this pull request
@netrome
Copy link
Contributor Author

netrome commented Nov 13, 2024

Interesting observation, the current benchmark is slower with the multi-get implementation than with the plain db lookups.

On my build server I get the following results (having saved the end_to_end_query_times_reference and end_to_end_query_times_multi_get binaries from previous runs with cargo bench --bench end_to_end_query_times)

hyperfine /tmp/end_to_end_query_times_reference /tmp/end_to_end_query_times_multi_get
Benchmark 1: /tmp/end_to_end_query_times_reference
  Time (mean ± σ):      7.052 s ±  0.200 s    [User: 7.726 s, System: 0.937 s]
  Range (min … max):    6.768 s …  7.246 s    10 runs

Benchmark 2: /tmp/end_to_end_query_times_multi_get
  Time (mean ± σ):      7.172 s ±  0.119 s    [User: 7.833 s, System: 0.990 s]
  Range (min … max):    6.901 s …  7.288 s    10 runs

Summary
  '/tmp/end_to_end_query_times_reference' ran
    1.02 ± 0.03 times faster than '/tmp/end_to_end_query_times_multi_get'

@netrome netrome force-pushed the 2422-chore-benchmark-multi-get-implementation branch from 31dda4d to c930805 Compare November 14, 2024 08:39
@netrome netrome force-pushed the 2422-chore-benchmark-multi-get-implementation branch from c930805 to 8a2a490 Compare November 14, 2024 09:28
@netrome
Copy link
Contributor Author

netrome commented Nov 14, 2024

Bumped the number of blocks (and therefore number of transactions to query) by a factor 10, but I still can't observe a notable difference between the multi-get implementation and the previous one.

hyperfine /tmp/end_to_end_query_times_long_reference /tmp/end_to_end_query_times_long_multi_get
Benchmark 1: /tmp/end_to_end_query_times_long_reference
  Time (mean ± σ):     68.403 s ±  0.345 s    [User: 77.285 s, System: 8.956 s]
  Range (min … max):   67.802 s … 69.064 s    10 runs

Benchmark 2: /tmp/end_to_end_query_times_long_multi_get
  Time (mean ± σ):     68.726 s ±  0.499 s    [User: 77.652 s, System: 8.834 s]
  Range (min … max):   67.866 s … 69.568 s    10 runs

Summary
  '/tmp/end_to_end_query_times_long_reference' ran
    1.00 ± 0.01 times faster than '/tmp/end_to_end_query_times_long_multi_get'

This could be due to a number of factors.

  1. One loose theory is that most (or all) of the data is still within the RocksDB memtable, and that this makes the speed gain by batching with multi-get negligible.
  2. Another theory is that the overhead of boxing the iterators outweighs the performance gain of the multi-get operation for these workloads.
  3. It could also just be the case that database lookup times isn't the hot path of the current workloads. If so, we could consider writing more isolated benchmarks (though we already have the db_lookup_times), but we should also question the value of this optimization if it's not in the hot path for the graphql queries since that's where it's used right now.
  4. Anything else not considered yet.

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.

chore: Benchmark multi-get implementation
1 participant