Skip to content
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

Investigate using s2n-quic as transport #10

Open
rklaehn opened this issue Dec 8, 2022 · 11 comments
Open

Investigate using s2n-quic as transport #10

rklaehn opened this issue Dec 8, 2022 · 11 comments

Comments

@rklaehn
Copy link
Collaborator

rklaehn commented Dec 8, 2022

The quinn throughput numbers are disappointing. s2n-quic seems to have a pretty high level API, so it should not be that hard to wrap it.

https://github.com/aws/s2n-quic

@camshaft
Copy link

Let me know if you run into anything while testing it out. We also have quite a few performance-related tasks scheduled so the results are only going to get better from here.

@rklaehn
Copy link
Collaborator Author

rklaehn commented Dec 16, 2022

@flub this is not really urgent, but if you want to try it out it would be nice.

I sometimes think that quic-rpc would also be a good option for connections between peers, but unlike for the RPC use case where hyper is fine, here we would really want QUIC.

@flub
Copy link
Contributor

flub commented Dec 16, 2022

yeah, had this on my "if i have time" list last weekend already :) but as you can tell... time... but i'd be keen to try it out sometime still

@rklaehn
Copy link
Collaborator Author

rklaehn commented Dec 28, 2022

Here is a first attempt: https://github.com/n0-computer/quic-rpc/tree/s2n-quic

@rklaehn
Copy link
Collaborator Author

rklaehn commented Jan 10, 2023

@camshaft I now got a very ugly but working impl in the above branch.

The API of quic-rpc was originally modeled after quinn, so in order to match the quic-rpc API I have to do so some seriously ugly stuff like call poll_whatever after grabbing a lock, because many of the s2n-quic methods require mutable references.

We might adapt the API later to avoid that, depending on how this experiment goes.

The bench works, however the bidirectional streaming is extremely slow. It seems like some buffer is not sent immediately but only after some fixed delay.

cargo test --release s2n_quic_channel_bench -- --nocapture
RPC seq 8_420 rps                                                               
RPC par 117_260 rps   
bidi seq 37 rps 

For comparison, this is the somewhat similar quinn based impl:

cargo test --release quinn_channel_bench -- --nocapture
RPC seq 16_243 rps                                                              
RPC par 201_385 rps                                                             
bidi seq 173_999 rps  

I looked at this with good old println debugging, and it seems while the server gets the requests almost immediately, the responses only get to the client as a trickle. Revert 98986b4 to see the debug output.

All the ugliness with the locks is done for establishing a connection. Once that is done, the code is extremely similar to the quinn code, so I am not sure this is on my side.

One notable thing is that we are not using the default crypto but rustlis. I guess I could try with the default crypto to make sure that is not the cause...

@rklaehn
Copy link
Collaborator Author

rklaehn commented Jan 10, 2023

Tried out the built in crypto. Does not make a difference - same behaviour.

@rklaehn
Copy link
Collaborator Author

rklaehn commented Feb 9, 2023

I have two branches where I wired up s2n_quic. One where there is questionable stuff like taking a lock to poll futures, and one that is pretty clean.

In both cases I am getting extremely bad performance, in particular for the bidi seq test.

This would require a closer look before we proceed with this. In it's current form it is unusable because it is unbearably slow.

@camshaft
Copy link

camshaft commented Feb 9, 2023

Can you open a draft PR with the clean one and I can add some comments

@camshaft
Copy link

camshaft commented Feb 9, 2023

Also could you share a flamegraph of the benchmarks you're running. There might be something that stands out there

@rklaehn
Copy link
Collaborator Author

rklaehn commented Feb 17, 2023

Will do. Just have to clean it up a bit.

@rklaehn
Copy link
Collaborator Author

rklaehn commented Feb 17, 2023

Here it is: #40

There is basically a smoke test and a perf test for each transport. Here is the output of the perf tests:

❯ cargo test _channel_bench --all-features --release -- --nocapture
...

running 1 test
RPC seq 584_690 rps                                                             ....................
RPC par 642_932 rps                                                             
bidi seq 14_068_053 rps                                                         ....................
test flume_channel_bench ... ok

running 1 test
RPC seq 17_068 rps                                                              
RPC par 83_819 rps                                                              
bidi seq 587_368 rps                      
test hyper_channel_bench ... ok

running 1 test
RPC seq 21_120 rps                                                              
RPC par 190_103 rps                                                             
bidi seq 290_512 rps                                                            
test quinn_channel_bench ... ok

running 1 test
RPC seq 1_618 rps                                                               
RPC par 25_370 rps                                                              
bidi seq 35 rps                                                                 
test s2n_quic_channel_bench ... ok

Maybe something about the config I need to change?

@flub flub removed their assignment Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants