-
Notifications
You must be signed in to change notification settings - Fork 175
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
Async/subscription benches #372
Conversation
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.
Good stuff, thank you so much. :)
}) | ||
}) | ||
}); | ||
} | ||
|
||
/// Benchmark http batch requests over batch sizes of 2, 5, 10, 50 and 100 RPCs in each batch. | ||
fn run_round_trip_with_batch(rt: &TokioRuntime, crit: &mut Criterion, client: Arc<impl Client>, name: &str) { | ||
fn run_sub_round_trip(rt: &TokioRuntime, crit: &mut Criterion, client: Arc<impl SubscriptionClient>, name: &str) { |
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.
a follow could be to subscribe to the "same subscription (by method name)" many times to benchmark how costly it is to access the Mutex
when the number of subscribers grows. This Mutex
should only accessed when creating a new subscription or dropping an existing one however so maybe not that interesting anymore.
currently we have one Arc<Mutex>
per registered subscription (by method name)
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.
I think it'd be more right to write more specific benches for that matter. Like, ones that do not include real server and client, and thus impact of mutex will be more clear.
// Subscription will be closed inside of the drop impl. | ||
// Actually, it just sends a notification about object being closed, | ||
// but it's still important to know that drop impl is not too expensive. | ||
drop(black_box(sub)); |
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.
I like this
}) | ||
}, | ||
|mut sub| { | ||
rt.block_on(async { black_box(sub.next().await.unwrap()) }); |
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.
ok, and you can't create the subscription outside the bench function because fn next
needs &mut self, correct?
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.
I can, but in that case, it'd be quite difficult to create a representative benchmark.
bench
function is called many times, thus if we will try to re-use the subscription made outside of the bench, then the subscription function will have to yield multiple values. If it'd be done without timeouts, it may eat RAM pretty quickly (sender is unbounded). If we will use timeouts, it may affect benchmark results.
In theory, we can try to synchronize somehow received/sent messages using some kind of semaphore, but once again this communication and overhead caused by it will affect the benchmark.
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.
fair enough sounds complicated make sense.
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.
thanks, good job again
@dvdplm I know that you had some runtime issues with this PR, do you want to merge it? I think we could try to use a shared global |
Yeah, macos has ridiculously low |
* master: (21 commits) New proc macro (#387) Streaming RpcParams parsing (#401) Set allowed Host header values (#399) Synchronization-less async connections in ws-server (#388) [ws server]: terminate already established connection(s) when the server is stopped (#396) feat: customizable JSON-RPC error codes via new enum variant on `CallErrror` (#394) [ci]: test each individual crate's manifest (#392) Add a way to stop servers (#386) [jsonrpsee types]: unify a couple of types + more tests (#389) Update roadmap link in readme (#390) Cross-origin protection (#375) Method aliases + RpcModule: Clone (#383) Use criterion's async bencher (#385) Async/subscription benches (#372) send text (#374) Fix link to ws server in README.md (#373) Concat -> simple push (#370) Add missing `rt` feature (#369) Release prep for v0.2 (#368) chore(scripts): publish script (#354) ...
* Add benches for async methods * Benches for subscriptions
This PR implements some basic benches for async methods and subscriptions.
Benches results on my machine (Macbook pro 2020 m1/8gb)