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

Release GIL during cipher update #11900

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Release GIL during cipher update #11900

wants to merge 1 commit into from

Conversation

kcking
Copy link

@kcking kcking commented Nov 5, 2024

Related to discussion in #11585.

@alex
Copy link
Member

alex commented Nov 5, 2024

Can you share anything about the performance impact of this?

I'm modestly nervous about performance overhead for very small chunks, but what can you do 🙃

@kcking
Copy link
Author

kcking commented Nov 5, 2024

modestly nervous about performance overhead

Releasing the GIL should be order of microseconds. I could put together a benchmark for small blocks to see if the overhead is meaningful

Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can share some performance numbers, we can get this merged. Thanks!

@alex
Copy link
Member

alex commented Nov 5, 2024

Ooops, crossed streams :-)

Do you have any numbers for the impact of this on your application?

@kcking
Copy link
Author

kcking commented Nov 5, 2024

I can get some for you hopefully tonight, thanks for the quick review!

@alex
Copy link
Member

alex commented Nov 5, 2024

Sounds great, thanks for the contribution

@reaperhulk
Copy link
Member

Just a quick ping re: perf numbers 😄

@reaperhulk
Copy link
Member

I ran some tests on an M1 Max (10 cores, 10 threads) and it appears GIL release is slower than single threading up to ~23kB update calls. I'd be curious if the same is true on other CPUs.

Some numbers:

Payload Size Main PR
64 bytes (100MB per thread) 16.6s 91s
23 kB (1024MB per thread) 4.7s 4.5s
1 MB (1024MB per thread) 4.3s 0.55s

These were derived using the following script and varying the size of DATA and TOTAL (100 for the first test, 1024 for the remaining).

import time
import concurrent.futures
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms

TOTAL = 1024 * 1024 * 100
DATA = bytes(64)
ITERATIONS = TOTAL // len(DATA)
KEY = bytes(32)

start = time.time()

def encrypt(num):
    buf = bytearray(len(DATA))
    c = Cipher(algorithms.ChaCha20(KEY, b"0" * 16), None)
    encryptor = c.encryptor()
    for _ in range(ITERATIONS):
        encryptor.update_into(DATA, buf)
    print(f"{num} thread complete. {ITERATIONS * len(DATA) / 1024 / 1024} MiB encrypted")


with concurrent.futures.ThreadPoolExecutor() as executor:
    executor.map(encrypt, range(0, 10))

print(time.time() - start)

@alex alex added this to the Forty Fifth Release milestone Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants