feat(benchmarks): Write better benchmark for testing the aggregator #3839
Conversation
relay-metrics/benches/benches.rs
Outdated
| ]; | ||
|
|
||
| for input in &inputs { | ||
| for (input_name, input) in &inputs { | ||
| c.bench_with_input( |
There was a problem hiding this comment.
Can make a criterion group here and specify throughput, compare to here
relay/relay-cardinality/benches/redis_impl.rs
Line 198 in 4d5941a
relay-metrics/benches/benches.rs
Outdated
| }, | ||
| |(mut aggregator, buckets)| { | ||
| for (project_key, bucket) in buckets { | ||
| aggregator.merge(project_key, bucket, None).unwrap(); | ||
| #[allow(clippy::unit_arg)] | ||
| black_box(aggregator.merge(project_key, bucket, None).unwrap()); |
There was a problem hiding this comment.
I think the black box should be on the aggregator after the loop.
There was a problem hiding this comment.
I thought that black_box is handled specifically by the compiler just by wrapping an expression with it. Maybe I need to better read the docs.
There was a problem hiding this comment.
I read the docs, it's indeed doing some stuff with the result value, treating it pessimistically.
There was a problem hiding this comment.
The result value here is just the unit type. I am not sure what the compiler is allowed to optimize here, but for me it makes more sense on the "result".
This PR reworks the benchmarks for the aggregator by adding more fine-grained control over certain parameters.
Closes: #3844
#skip-changelog