-
Notifications
You must be signed in to change notification settings - Fork 37
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
Introduce restate_benchmarks package with sequential and parallel throughput benchmark #561
Conversation
@slinkydeveloper it would be great if you could try to create the flamegraphs on your machine. I am not 100% whether the |
Using the Java based throughput/parallel throughput/sequential |
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.
Looks good to me, just left a couple of comment with some ideas on benchmarks we can add in future. I'll now try to run it locally now.
async fn send_sequential_counter_requests( | ||
mut counter_client: CounterClient<tonic::transport::Channel>, | ||
num_requests: u64, | ||
) { | ||
for _ in 0..num_requests { | ||
counter_client | ||
.get_and_add(CounterAddRequest { | ||
counter_name: "single".into(), | ||
value: 10, | ||
}) | ||
.await | ||
.expect("counter.Counter::get_and_add should not fail"); | ||
} | ||
} |
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.
An interesting variant of this benchmark could be to send many requests in parallel (always with the same key), and then await on them all at once. Perhaps this might be interesting to test if there is some contention going on in the ingress_grpc? (e.g. for the registry?)
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.
True. Maybe this could also be a more component targeted benchmark that only tests the ingress_grpc
component.
.await | ||
.expect("counter.Counter::get_and_add should not fail"); | ||
} | ||
} |
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.
Another bench variant could be to send requests using connect/http, rather than grpc, to see whether json encoding/decoding has some effective cost on the requests
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.
True. This could also be a more component targeted benchmark.
Using the java counter as well:
Wondering why it's so low my sequential time?! |
I did not managed to get flamegraphs on my machine working :( I suggest we go ahead and merge this PR, and then i'll take the issue of figuring out why it doesn't work on my machine |
I guess it's also worth figuring out why on my machine the performance of the sequential case is so terrible |
These numbers are indeed quite low. Not sure what is happening on your machine. This deserves indeed some further investigation.
Alright, sounds good to me. |
Thanks for the review @slinkydeveloper. I've addressed your comments. Rebasing and then merging this PR. |
This fixes #560. Add exception for inferno library which is under CDDL-1.0 We can use the inferno library (CDDL-1.0) because we don't modify its source code. Any such modifications would have to be made source code available under CDDL-1.0 as well.
Builders are generated via the derive_builder crate which creates a builder via a procedural macro.
This resolves a clash between different transitive dependencies. Add exception for duplicate regex-syntax dependency The regex-syntax dependency is a transitive dependency of tracing-subscriber and datafusion. Add exception for duplicate quick-xml dependency The dependency is pulled in by datafusion and pprof which is being used for profiling our benchmarks.
This PR introduces the
restate_benchmarks
package with the sequential and parallel throughput benchmark.The sequential throughput benchmark invokes
counter.Counter/GetAndAdd
sequentially with the same key.The parallel throughput benchmark invokes
counter.Counter/GetAndAdd
concurrently with random keys.The benchmarks are set up to support profiling via
pprof
. You can run them viacargo bench --bench throughput_parallel -- --profile-time=30
. This will generate a flamegraph undertarget/criterion/throughput/parallel/profile/flamegraph.svg
.This PR also includes a couple of version upgrades to resolve dependency conflicts.
This PR fixes #560.