-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add benchmark with ci #365
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
Conversation
Adds a `JAMBenchmarks` executable using `swift-benchmark` to test the performance of erasure coding, shuffling, and STF application. This change includes necessary helpers, public API adjustments, and VS Code launch configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes introduce a benchmark suite, which is a great addition. However, the implementation of the asynchronous benchmarks uses a custom, unsafe helper to block on async operations. This helper is unnecessary as the benchmarking library natively supports async/await, and its use should be replaced to improve code safety and simplicity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes introduce a new benchmark suite, which is a great addition. The implementation is mostly solid, but there's a potential for crashes in the benchmark executable due to force-unwrapping (try!) during test data decoding. This should be addressed to make the benchmarks more robust.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #365 +/- ##
==========================================
+ Coverage 81.64% 81.75% +0.10%
==========================================
Files 381 381
Lines 33572 33572
==========================================
+ Hits 27410 27446 +36
+ Misses 6162 6126 -36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes introduce a new benchmark suite, which is a great addition. However, there are a few issues to address. The primary issue is that one of the new benchmarks incorrectly includes data decoding time in its measurement, which will lead to inaccurate results. Additionally, there's a minor improvement to be made in the Swift package dependency declaration and a best-practice adjustment for a GitHub Action version.
Summary of Changes:
ordo-one/package-benchmarklibrary.JAMBenchmarksexecutable target to theJAMTestspackage to house the benchmarks.BenchSupport.swifthelper to expose test data from theJAMTestsmodule to the external benchmark executable.launch.jsonwith configurations for running and debugging the benchmark suite.public,Sendable) inJamTestnet.swiftto allow its use in the benchmark target.Highlights of Changed Code:
JAMTests/Benchmarks/JAMBenchmarks/JAMBenchmarks.swift: New file containing all benchmark definitions.JAMTests/Package.swift: Addition of thepackage-benchmarkdependency and the newJAMBenchmarksexecutable target.JAMTests/Sources/JAMTests/BenchSupport.swift: New public helperJAMBenchSupportfor loading test data.JAMTests/Sources/JAMTests/JamTestnet.swift:runSTFis nowpublic, andJamTestnetTestcaseand related types areSendable..vscode/launch.json: Added "Debug JAMBenchmarks" and "Release JAMBenchmarks" configurations.Blockchain/Sources/Blockchain/RuntimeProtocols/Accumulation.swift: Minor stylistic refactor from closure-basedflatMapto key-path expressions.Additional Information:
swift benchmark) or via the newly added VS Code launch configurations.