SSLSocket.read does a GIL round-trip for every 16KB TLS record

[hashbrowncipher]

BPO	37355
Nosy	@warsaw, @pfmoore, @tiran, @tjguk, @benjaminp, @bitdancer, @njsmith, @dimaqq, @zware, @1st1, @zooba, @Safihre, @pablogsal, @hashbrowncipher, @jakirkham
PRs	bpo-37355: For nonblocking sockets, call SSL_read in a loop #25478 gh-81536: For nonblocking sockets, add SSLSocket.eager_recv to call SSL_read in a loop #31492

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-06-20.18:42:49.267>
labels = ['expert-SSL', '3.11', 'performance']
title = 'SSLSocket.read does a GIL round-trip for every 16KB TLS record'
updated_at = <Date 2022-03-14.15:49:20.009>
user = 'https://github.com/hashbrowncipher'

bugs.python.org fields:

activity = <Date 2022-03-14.15:49:20.009>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['SSL']
creation = <Date 2019-06-20.18:42:49.267>
creator = 'josnyder'
dependencies = []
files = []
hgrepos = []
issue_num = 37355
keywords = ['patch']
message_count = 12.0
messages = ['346156', '379904', '391401', '392584', '392585', '392882', '392883', '396780', '413900', '415035', '415094', '415095']
nosy_count = 16.0
nosy_names = ['barry', 'paul.moore', 'christian.heimes', 'tim.golden', 'benjamin.peterson', 'r.david.murray', 'njs', 'Dima.Tisnek', 'zach.ware', 'yselivanov', 'steve.dower', 'Safihre', 'pablogsal', 'Matthew Rocklin', 'josnyder', 'jakirkham']
pr_nums = ['25478', '31492']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue37355'
versions = ['Python 3.11']

Linked PRs

• gh-81536: Avoids repeated GIL acquisition when reading/writing to SSL sockets #102214

[hashbrowncipher]

Background:

SSLSocket.read drops the GIL and performs exactly one successful call to OpenSSL's SSL_read, whose documentation states "At most the contents of one record will be returned". TLS records are at most 16KB, so high throughput (especially multithreaded) TLS reception can become bottlenecked on the GIL.

Proposal:

For non-blocking sockets, call SSL_read in a loop until the user-supplied limit is reached or no bytes are available on the socket. I don't know of a way to safely improve performance for blocking sockets.

Initial testing:

I performed initial testing using 32 threads pinned to 16 cores, downloading and re-assembling a single 140270MB file from a "real world" TLS sender. This resulted in a 4x increase in throughput, a 6.6x reduction in voluntary context switches, a 3.5x reduction in system time. User time did increase by 43%, so the overall reduction in CPU usage was only 2.67x.

                             before     after
 wall clock time (s)    :    29.637     7.116
 user time (s)          :     8.793    12.584
 system time (s)        :   105.118    30.010
 user + system time (s) :   113.911    42.594
 cpu utilization (%)    :       384       599 
 voluntary switches     : 1,653,065   248,484
 speed (MB/s)           :      4733     19712

My git branch (currently a draft) is at https://github.com/hashbrowncipher/cpython/commits/faster_tls

[dimaqq]

Amazing! Would you consider making a pull request out of your branch?

re: PyErr_HasSignals is a notable addition to the cpython API, it may need discussion, and (imo) documentation.

[tiran]

Josh, could you please rebase your branch and create a pull request? The PR process will verify that you have submitted a CLA.

[Safihre]

It would be very beneficial if this gets added. In our application (usenet client) I have wondered for years why we had to limit ourselves to 16k blocks and have such lower speeds compared to non SSL connections.

[tiran]

The PR is still experimental and only applies to one direction. I doubt that it will be ready, reviewed, and approved for feature freeze of 3.10.

[Safihre]

Understandable, as the feature freeze was yesterday :)

Just like to note that we have a 100.000 or so users (which I know is very little compared to overal number of Python users) of our application that this could really help.

We have used a lot of CPU cycles over the past year doing many repeated 'recv(16384)' for SSL-sockets.

Thank you!

[tiran]

You could try to convince Pablo. He is the release manager for 3.10. He can grant exceptions.

I also included Benjamin, Nathaniel, and Victor in the nosy list. They have a deeper understanding of the network I/O layer than me. I mostly take care of the crypto and X.509 part of the module.

[Safihre]

Is anyone available to give feedback on the remaining questions/problems in the PR?

I don't want to be pushy and if it's only changed in 3.11, I understand, but just hoping for some progress :)
Also willing to dive into it myself, but not a network/python-core specialist to really get all the details right away.