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: add sync-v2 benchmark to CI #1083

Merged
merged 1 commit into from
Aug 6, 2024
Merged

feat: add sync-v2 benchmark to CI #1083

merged 1 commit into from
Aug 6, 2024

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Jul 12, 2024

Motivation

This PR implements the basic infrastructure for running benchmarks in our CI through GitHub Actions and the Bencher tool, which automatically tracks changes in performance across PRs (though it is more flexible than that). Bencher is open source and provides their cloud to run the Bencher service, which we're using through a free tier plan. We can consider upgrading our plan or running our own self-hosted service if necessary, but I don't see any reasons for now. This means our benchmarking results are public on the Bencher Console.

We setup a benchmark for tracking sync-v2 performance. It works by:

  1. Setting up a temporary node connected to testnet that exits after downloading 20k blocks.
  2. Running a server node in the background, connected only on localhost, and using the database with the previously downloaded blocks.
  3. Running a Hyperfine benchmark integrated with Bencher, which starts a new full node that syncs 20k blocks with the server node, and exits.

By doing this we isolate the network and test only the node performance itself. Of course there may be unpredictable variations in GitHub machines, but we'll only understand this impact by observing the metrics over time.

The job described above runs in two workflows, one for every push on master, which sets the baseline for our metrics, and one for each PR to master, which compares the metric value to the baseline and triggers a CI error if it's above a certain threshold. This is configured though the Bencher console directly. I configured both upper and lower thresholds of 10% (it's useful to set a lower threshold to track performance improvements, as an unpredicted drop in time may indicate new bugs).

There are improvements to be made in the future:

  • Instead of using a node connected to testnet and syncing 20k blocks every time, we could have a pre-downloaded database and save it in S3, for example. Currently, this downloading step is way slower than the benchmark itself (to be clear: it does not affect the benchmark time, but it does affect the CI job duration).
  • We currently only sync 20k blocks from testnet, and that doesn't include any transactions. We could test the improvement above with different database sizes and find a tradeoff to include transactions. We could even create a fake database with specific blocks/transactions ratios and benchmark them.
  • We can add other benchmarks such as node initialization time, vertex (de)serialization time, API response times, etc.
  • We could add this to other projects, not only hathor-core.

Acceptance Criteria

  • Create a new reusable action setup-hathor-env, used by our main CI and the new benchmarking workflows.
  • Add new Continuous Benchmarking base branch and Continuous Benchmarking PRs jobs.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Jul 12, 2024
@glevco glevco marked this pull request as ready for review July 12, 2024 16:37
@glevco glevco requested review from jansegre and msbrogli as code owners July 12, 2024 16:37
@glevco glevco force-pushed the feat/benchmarking branch from 134ef8a to ff8030d Compare July 12, 2024 17:02
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.02%. Comparing base (dcb876c) to head (7cee0f6).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1083   +/-   ##
=======================================
  Coverage   85.01%   85.02%           
=======================================
  Files         314      314           
  Lines       23957    23957           
  Branches     3621     3621           
=======================================
+ Hits        20367    20369    +2     
+ Misses       2879     2878    -1     
+ Partials      711      710    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glevco glevco force-pushed the feat/benchmarking branch 2 times, most recently from 68650b7 to 4278024 Compare July 12, 2024 18:28
@HathorNetwork HathorNetwork deleted a comment from github-actions bot Jul 12, 2024
@glevco glevco force-pushed the feat/benchmarking branch from 4278024 to 555df74 Compare July 12, 2024 19:10
Copy link

github-actions bot commented Jul 12, 2024

🐰Bencher

ReportMon, August 5, 2024 at 15:41:06 UTC
Projecthathor-core
Branchfeat/benchmarking
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)
Latency Lower Boundary
nanoseconds (ns) | (%)
Latency Upper Boundary
nanoseconds (ns) | (%)
sync-v2 (up to 20000 blocks)✅ (view plot)161,709,658,426.00 (0.00%)145,538,692,583.40 (90.00%)177,880,624,268.60 (90.91%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

jansegre
jansegre previously approved these changes Jul 17, 2024
Copy link
Member

@jansegre jansegre left a comment

Choose a reason for hiding this comment

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

Looks good to me.

msbrogli
msbrogli previously approved these changes Jul 30, 2024
extras/benchmarking/benchmark_sync_v2.sh Show resolved Hide resolved
@glevco glevco dismissed stale reviews from msbrogli and jansegre via 7cee0f6 August 5, 2024 15:40
@glevco glevco force-pushed the feat/benchmarking branch from 555df74 to 7cee0f6 Compare August 5, 2024 15:40
@glevco glevco merged commit f3481ab into master Aug 6, 2024
13 checks passed
@glevco glevco deleted the feat/benchmarking branch August 6, 2024 15:13
@jansegre jansegre mentioned this pull request Oct 4, 2024
2 tasks
@jansegre jansegre mentioned this pull request Dec 11, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants