-
Notifications
You must be signed in to change notification settings - Fork 237
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
Switch "crypto" implementation to Python #457
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #457 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 26 +1
Lines 5113 5178 +65
=========================================
+ Hits 5113 5178 +65 ☔ View full report in Codecov by Sentry. |
Initial testing suggest a 10x slowdown of packet protection compared to the current C code. |
e28fb8a
to
eb3a5f2
Compare
I haven't tried measuring it yet. My prior experience is that cryptography is pretty fast, and I'm wondering if the header protection part might be the slow thing. If it were, it might be possible to do the crypto with cryptography and then do the masking with a tiny bit of C code. |
AFAICT it's the actual crypto code unfortunately that is slower. |
Can you say how you are benchmarking? I'm guessing I'm doing something wrong, but I'm testing by using the example client to pull 20MiB from nginx, and while it is slower it's not too bad, only around 1.074 times the non-cryptography version. Regular aioquic (main from your branch with python-crypto):
Your python-crypto branch:
|
Here is what I use: import binascii
import time
from aioquic.quic.crypto import CryptoPair
from tests.test_crypto import (
LONG_CLIENT_PACKET_NUMBER,
LONG_CLIENT_PLAIN_HEADER,
LONG_CLIENT_PLAIN_PAYLOAD,
PROTOCOL_VERSION,
)
pair = CryptoPair()
pair.setup_initial(
cid=binascii.unhexlify("8394c8f03e515708"),
is_client=True,
version=PROTOCOL_VERSION,
)
start = time.time()
for i in range(100000):
pair.encrypt_packet(
LONG_CLIENT_PLAIN_HEADER,
LONG_CLIENT_PLAIN_PAYLOAD,
LONG_CLIENT_PACKET_NUMBER,
)
elapsed = time.time() - start
print(elapsed) A typical run for me puts the C code at about 70ms and the Python code at around 800ms. Notes:
|
Yeah, I discovered the need to purge while I was benchmarking. The timings I reported are after making sure I was testing in clean setups. |
And of course both of our testing could be accurate, as you're testing just the crypto whereas I'm testing what the overall effect in typical use is. |
I wish As things currently stand, the build of the C code has broken again through no change of our own: https://github.com/jlaine/aioquic/actions/runs/9150448224 |
157e70c
to
427ddf2
Compare
This simplifies the build process and avoids needing to ship a vendor'd OpenSSL.
This simplifies the build process and avoids needing to ship a vendor'd OpenSSL.