[bench] Replace AWK tx/s script with b.ReportMetric#515
Conversation
Add test.ReportTxPerSecond(b) helper and call it from all benchmarks. Cap batch benchmarks to exactly b.N transactions for accurate reporting. Remove bench-tx-per-sec.awk and Makefile AWK pipes. Signed-off-by: Senthilnathan <cendhu@gmail.com>
cendhu
force-pushed
the
bench-report-metrics
branch
from
aa121ca to
a8cbc73
Compare
liran-funaro
left a comment
liran-funaro
left a comment
There was a problem hiding this comment.
A major issue (personal preference) is that benchmark metrics in the standard output (like ns/op, B/op, or custom metrics) are not formatted with commas.
This makes the output difficult to read.
I suggest using the report metrics method as this PR suggest, but also having an optional post script to add the commas.
BenchmarkMapBlockSize() reports incorrectly because of the block size.
b.N indicated the number of iterations to test to ensure confidence in the results.
We usually use b.N as the number of TXs as the benchmark runs the same operation on each TX, which is what b.N indicates.
In BenchmarkMapBlockSize(), the block is the minimal tested unit as we test how the block size affects the performance.
Thus, we should use b.N as the number of blocks in this case.
However, I still think we should report TX/s as this is our actual unit of measurement we care about.
I suggest allowing test.ReportTxPerSecond() reporting TX/s with a different number of TXs than b.N.
Unrelated to this PR: it seems that the notifier and dependency graph benchmarks hangs. And the deliver benchmark have a error.
These should be fixed in a different PR.
| }) | ||
|
|
||
| txPoll := workload.GenerateTransactions(b, p, max(b.N*3, batchSize*3)) | ||
| txPoll := workload.GenerateTransactions(b, p, max(b.N, batchSize)) |
There was a problem hiding this comment.
This benchmark is designed such that the dependency graph internal queue will not be empty during the benchmark.
This is why we submit up to 3 times the number of TXs.
We stop the benchmark once the manager released b.N TXs.
| # Run signature benchmarks. | ||
| bench-sign: FORCE | ||
| $(go_cmd) test ./utils/signature/... -bench ".*" -run "^$$" | awk -f scripts/bench-tx-per-sec.awk | ||
| $(go_cmd) test ./utils/signature/... -bench ".*" -run "^$$" |
There was a problem hiding this comment.
a: testsig should also call test.ReportTxPerSecond(b).
b: The makefile command should be fixed.
| $(go_cmd) test ./utils/signature/... -bench ".*" -run "^$$" | |
| $(go_cmd) test ./utils/testsig/... -bench ".*" -run "^$$" |
|
|
||
| func BenchmarkLatencyTrackerPortion(b *testing.B) { | ||
| for _, conf := range []SamplerConfig{ | ||
| { /* never */ }, {Portion: 1}, {Portion: 0.1}, {Portion: 0.01}, {Prefix: "0"}, {Prefix: "00"}, |
There was a problem hiding this comment.
Unrelated: please remove { /* never */ }, {Portion: 1}, as they cannot be correctly measured (too simple).
| } | ||
|
|
||
| func BenchmarkGenTx(b *testing.B) { | ||
| //nolint:thelper // false positive. |
There was a problem hiding this comment.
Unrelated: please add flogging.ActivateSpec("fatal")
| } | ||
|
|
||
| func BenchmarkGenQuery(b *testing.B) { | ||
| //nolint:thelper // false positive. |
There was a problem hiding this comment.
Unrelated: please add flogging.ActivateSpec("fatal")
| var result float64 | ||
|
|
||
| // printResult is used to prevent compiler optimizations. | ||
| func printResult() { | ||
| fmt.Printf("Result: %v\n", result) | ||
| } |
There was a problem hiding this comment.
unrelated: we don't use this anymore. please remove
| # Run dependency detector benchmarks. | ||
| bench-dep: FORCE | ||
| $(go_cmd) test ./service/coordinator/dependencygraph/... -timeout 60m -bench "BenchmarkDependencyGraph.*" -run="^$$" | awk -f scripts/bench-tx-per-sec.awk | ||
| $(go_cmd) test ./service/coordinator/dependencygraph/... -timeout 60m -bench "BenchmarkDependencyGraph.*" -run="^$$" |
There was a problem hiding this comment.
unrelated
| $(go_cmd) test ./service/coordinator/dependencygraph/... -timeout 60m -bench "BenchmarkDependencyGraph.*" -run="^$$" | |
| $(go_cmd) test ./service/coordinator/dependencygraph/... -timeout 60m -bench "Benchmark.*" -run="^$$" |
| } | ||
|
|
||
| func BenchmarkUnwrapEnvelope(b *testing.B) { | ||
| envelopes := loadgenEnvelopes(b, b.N) |
There was a problem hiding this comment.
unrelated: please add flogging.ActivateSpec("fatal")
| } | ||
|
|
||
| func BenchmarkUnwrapEnvelopeLite(b *testing.B) { | ||
| envelopes := loadgenEnvelopes(b, b.N) |
There was a problem hiding this comment.
unrelated: please add flogging.ActivateSpec("fatal")
3 participants
Type of change
Description
Add test.ReportTxPerSecond(b) helper and call it from all benchmarks.
Cap batch benchmarks to exactly b.N transactions for accurate reporting.
Remove bench-tx-per-sec.awk and Makefile AWK pipes.
Related issues