gh-135239: simpler use of mutex in hashlib & co by picnixz · Pull Request #135267 · python/cpython · GitHub

I've taken the liberty of normalizing code style. I'll do the same in other modules, (SHA and BLAKE2). That way, I'll never need to touch cosmetics again in crypto-modules. Well, if it's too much I can drop the last commit.

Issue: Smarter use of a mutex in incremental HMAC and hash functions #135239

Note that PyMutex_Lock will release the GIL/detach the thread state anyway. You want _PyMutex_LockFlags(m, _Py_LOCK_DONT_DETACH) if you're worried about GIL overhead.

…135239 # Conflicts: # Modules/_hashopenssl.c # Modules/blake2module.c

The benchmarks on an LTO+PGO build (it's actually slower on a DEBUG build but that's because of all the additional assertions) [with OpenSSL 3.5]:

### Large update (chunks size = 2049)

+-----------------------------------+---------+-----------------------+
| Benchmark                         | ref     | new                   |
+===================================+=========+=======================+
| _hashlib.openssl_sha1.update      | 12.4 us | 12.2 us: 1.02x faster |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_sha1.digest      | 126 ns  | 123 ns: 1.02x faster  |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_sha224.update    | 13.7 us | 13.4 us: 1.02x faster |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_sha3_224.digest  | 293 ns  | 284 ns: 1.03x faster  |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_sha3_384.update  | 62.3 us | 59.0 us: 1.06x faster |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_sha3_512.update  | 88.1 us | 85.0 us: 1.04x faster |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_shake_128.update | 38.7 us | 37.1 us: 1.04x faster |
+-----------------------------------+---------+-----------------------+
| _hashlib.openssl_shake_256.update | 46.6 us | 45.8 us: 1.02x faster |
+-----------------------------------+---------+-----------------------+
| _sha1.sha1.update                 | 61.1 us | 59.8 us: 1.02x faster |
+-----------------------------------+---------+-----------------------+
| _sha2.sha224.update               | 69.9 us | 66.3 us: 1.05x faster |
+-----------------------------------+---------+-----------------------+
| _sha2.sha224.digest               | 273 ns  | 266 ns: 1.03x faster  |
+-----------------------------------+---------+-----------------------+
| _sha2.sha256.update               | 71.0 us | 65.7 us: 1.08x faster |
+-----------------------------------+---------+-----------------------+
| _sha2.sha256.digest               | 277 ns  | 266 ns: 1.04x faster  |
+-----------------------------------+---------+-----------------------+
| _sha2.sha384.digest               | 373 ns  | 354 ns: 1.05x faster  |
+-----------------------------------+---------+-----------------------+
| _sha3.sha3_224.update             | 56.9 us | 53.4 us: 1.06x faster |
+-----------------------------------+---------+-----------------------+
| _sha3.sha3_224.digest             | 258 ns  | 250 ns: 1.03x faster  |
+-----------------------------------+---------+-----------------------+
| _sha3.sha3_256.update             | 60.4 us | 56.5 us: 1.07x faster |
+-----------------------------------+---------+-----------------------+
| _sha3.sha3_384.update             | 78.5 us | 74.3 us: 1.06x faster |
+-----------------------------------+---------+-----------------------+
| _sha3.sha3_384.digest             | 260 ns  | 252 ns: 1.03x faster  |
+-----------------------------------+---------+-----------------------+
| _sha3.sha3_512.update             | 112 us  | 107 us: 1.04x faster  |
+-----------------------------------+---------+-----------------------+
| _sha3.shake_128.update            | 49.8 us | 46.3 us: 1.08x faster |
+-----------------------------------+---------+-----------------------+
| _sha3.shake_256.update            | 59.7 us | 56.9 us: 1.05x faster |
+-----------------------------------+---------+-----------------------+
| _blake2.blake2s.update            | 37.0 us | 36.2 us: 1.02x faster |
+-----------------------------------+---------+-----------------------+
| Geometric mean                    | (ref)   | 1.02x faster          |
+-----------------------------------+---------+-----------------------+

### Small update (chunks size = 128)

+----------------------------------------+-----------+-----------------------+
| Benchmark                              | ref-small | new-small             |
+========================================+===========+=======================+
| _hashlib.openssl_md5.update            | 1.94 us   | 1.85 us: 1.05x faster |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_sha1.update           | 961 ns    | 943 ns: 1.02x faster  |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_sha1.digest           | 114 ns    | 124 ns: 1.09x slower  |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_sha224.digest         | 113 ns    | 118 ns: 1.04x slower  |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_sha3_224.digest       | 277 ns    | 284 ns: 1.03x slower  |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_sha3_256.update       | 2.96 us   | 3.20 us: 1.08x slower |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_sha3_384.update       | 4.05 us   | 3.86 us: 1.05x faster |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_shake_128.update      | 2.56 us   | 2.62 us: 1.02x slower |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_shake_256.update      | 3.02 us   | 3.12 us: 1.03x slower |
+----------------------------------------+-----------+-----------------------+
| _hashlib.openssl_shake_256.digest[512] | 804 ns    | 826 ns: 1.03x slower  |
+----------------------------------------+-----------+-----------------------+
| _sha2.sha224.update                    | 4.25 us   | 4.34 us: 1.02x slower |
+----------------------------------------+-----------+-----------------------+
| _sha2.sha224.digest                    | 271 ns    | 264 ns: 1.02x faster  |
+----------------------------------------+-----------+-----------------------+
| _sha2.sha512.update                    | 3.00 us   | 3.04 us: 1.02x slower |
+----------------------------------------+-----------+-----------------------+
| _sha2.sha512.digest                    | 355 ns    | 372 ns: 1.05x slower  |
+----------------------------------------+-----------+-----------------------+
| _sha3.sha3_224.update                  | 3.92 us   | 3.61 us: 1.08x faster |
+----------------------------------------+-----------+-----------------------+
| _sha3.sha3_256.digest                  | 253 ns    | 262 ns: 1.04x slower  |
+----------------------------------------+-----------+-----------------------+
| _sha3.sha3_384.digest                  | 253 ns    | 259 ns: 1.03x slower  |
+----------------------------------------+-----------+-----------------------+
| _sha3.sha3_512.update                  | 6.78 us   | 7.77 us: 1.15x slower |
+----------------------------------------+-----------+-----------------------+
| _sha3.sha3_512.digest                  | 251 ns    | 256 ns: 1.02x slower  |
+----------------------------------------+-----------+-----------------------+
| _sha3.shake_128.update                 | 3.12 us   | 3.22 us: 1.03x slower |
+----------------------------------------+-----------+-----------------------+
| _sha3.shake_256.update                 | 3.76 us   | 4.03 us: 1.07x slower |
+----------------------------------------+-----------+-----------------------+
| _blake2.blake2s.update                 | 2.45 us   | 2.39 us: 1.03x faster |
+----------------------------------------+-----------+-----------------------+
| Geometric mean                         | (ref)     | 1.01x slower          |
+----------------------------------------+-----------+-----------------------+

Benchmark script

import importlib
import os

import pyperf

def update(hasher, blocks):
    for block in blocks:
        hasher.update(block)

def bench_update(runner, args, typename, module, function):
    namespace = importlib.import_module(module)
    hasher = getattr(namespace, function)()
    blocks = [os.urandom(args.chunk_size) for _ in range(args.chunk_loop)]
    runner.bench_func(f"{typename}.update", update, hasher, blocks)

def bench_digest(runner, args, typename, module, function, *, xof=False):
    namespace = importlib.import_module(module)
    hasher = getattr(namespace, function)(os.urandom(1024))
    if xof:
        n = args.xof_length
        runner.bench_func(f"{typename}.digest[{n}]", hasher.digest, n)
    else:
        runner.bench_func(f"{typename}.digest", hasher.digest)

def bench_hash_function(runner, args, module, function, *, xof=False):
    typename = f"{module}.{function}"
    bench_update(runner, args, typename, module, function)
    bench_digest(runner, args, typename, module, function, xof=xof)

def bench_openssl_functions(runner, args, functions, *, xof=False):
    for function in functions:
        function = "openssl_" + function.removeprefix("openssl_")
        bench_hash_function(runner, args, "_hashlib", function, xof=xof)

if __name__ == "__main__":
    runner = pyperf.Runner()
    runner.argparser.add_argument("--chunk-loop", type=int, default=16)
    runner.argparser.add_argument("--chunk-size", type=int, default=2049)
    runner.argparser.add_argument("--xof-length", type=int, default=512)
    args = runner.parse_args()

    bench_openssl_functions(
        runner, args, [
            "md5",
            "sha1",
            "sha224", "sha256", "sha384", "sha512",
            "sha3_224", "sha3_256", "sha3_384", "sha3_512",
        ], xof=False,
    )  # fmt: skip
    bench_openssl_functions(runner, args, ["shake_128", "shake_256"], xof=True)

    for module, functions, xof in [
        ("_md5", ["md5"], False),
        ("_sha1", ["sha1"], False),
        ("_sha2", ["sha224", "sha256", "sha384", "sha512"], False),
        ("_sha3", ["sha3_224", "sha3_256", "sha3_384", "sha3_512"], False),
        ("_sha3", ["shake_128", "shake_256"], True),
        ("_blake2", ["blake2b", "blake2s"], False),
    ]:  # fmt: skip
        for function in functions:
            bench_hash_function(runner, args, module, function, xof=xof)

@ZeroIntensity can you take a look at this PR now? TiA

The locking looks good to me.

Arf, I should have actually run the build bots first.

Arf, I should have actually run the build bots first.

Why? What is the problem?

Why? What is the problem?

Actually, there was none. But when I perform such updates, I like to run some specific build bots (the FIPS only ones) as the hashlib tests are conditioned to the existence of some primitives. And sometimes I forget to add those checks and I need to make a follow-up PR just for that...

…hon#135267)

picnixz added 5 commits June 8, 2025 16:24

add common object head for hashlib/hmac objects

5d8c093

simplify digest computation

81e3046

refactor update logic

7f9f7b7

refactor alloc() logic

15a4f2f

finalizing touches

5cd828a

picnixz requested review from gpshead and tiran as code owners June 8, 2025 14:48

bedevere-app Bot added the awaiting core review label Jun 8, 2025

bedevere-app Bot mentioned this pull request Jun 8, 2025

Smarter use of a mutex in incremental HMAC and hash functions #135239

Closed

picnixz added skip news awaiting core review type-refactor Code refactoring (with no changes in behavior) and removed awaiting core review labels Jun 8, 2025

ZeroIntensity reviewed Jun 8, 2025

View reviewed changes

Comment thread Modules/md5module.c Outdated

picnixz marked this pull request as draft June 8, 2025 19:37

bedevere-app Bot removed the awaiting core review label Jun 8, 2025

correct mutex usage

63db1de

picnixz requested a review from ZeroIntensity June 15, 2025 09:40

picnixz added 2 commits June 15, 2025 11:42

Revert 5cd828a

ea033a3

revert some constructor changes

77baa67

picnixz marked this pull request as ready for review June 15, 2025 11:39

bedevere-app Bot added the awaiting core review label Jun 15, 2025

unconditionally lock when performing HASH updates

902759f

picnixz marked this pull request as draft June 16, 2025 10:16

bedevere-app Bot removed the awaiting core review label Jun 16, 2025

picnixz added 3 commits June 16, 2025 12:19

Merge remote-tracking branch 'upstream/main' into perf/hashlib/mutex-…

dde68c4

…135239 # Conflicts: # Modules/_hashopenssl.c # Modules/blake2module.c

post-merge

05c1e66

do not guard against empty buffers for now

db57278

picnixz force-pushed the perf/hashlib/mutex-135239 branch from bb237c2 to db57278 Compare June 16, 2025 10:35

consistency fixes

ead20a1

picnixz added 3 commits June 21, 2025 15:34

make the test suite less slow

c9db0b1

fix test when GIL_MINSIZE is changed

6ffdd1c

defer cosmetics

98ec915

picnixz marked this pull request as ready for review June 21, 2025 13:46

bedevere-app Bot added the awaiting core review label Jun 21, 2025

This comment was marked as resolved.

Sign in to view

picnixz commented Jun 21, 2025

View reviewed changes

Comment thread Lib/test/test_hashlib.py Outdated

Update Lib/test/test_hashlib.py

398ddb3

This comment was marked as resolved.

Sign in to view

improve test

0ae70e9

ZeroIntensity approved these changes Jun 22, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Jun 22, 2025

picnixz merged commit e7295a8 into python:main Jun 22, 2025

picnixz deleted the perf/hashlib/mutex-135239 branch June 22, 2025 15:00

bedevere-app Bot removed the awaiting merge label Jun 22, 2025

AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025

pythongh-135239 : simpler use of mutexes in cryptographic modules (pyt…

f216c82

…hon#135267)

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025

pythongh-135239 : simpler use of mutexes in cryptographic modules (pyt…

2dfda94

…hon#135267)

picnixz mentioned this pull request Jul 15, 2025

Blake2b/s implementations have minor GIL issues #80700

Closed

taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025

pythongh-135239 : simpler use of mutexes in cryptographic modules (pyt…

db05ae2

…hon#135267)

Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025

pythongh-135239 : simpler use of mutexes in cryptographic modules (pyt…

2b23cd6

…hon#135267)

gh-135239: simpler use of mutex in hashlib & co by picnixz · Pull Request #135267 · python/cpython · GitHub

Repository navigation

gh-135239: simpler use of mutex in hashlib & co - #135267

gh-135239: simpler use of mutex in hashlib & co#135267
picnixz merged 45 commits into
python:mainfrom
picnixz:perf/hashlib/mutex-135239

picnixz commented Jun 8, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

Uh oh!

ZeroIntensity left a comment

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

picnixz commented Jun 22, 2025 •
edited
Loading

Uh oh!

Uh oh!

picnixz commented Jun 22, 2025

Uh oh!

ZeroIntensity left a comment

Uh oh!

picnixz commented Jun 22, 2025

Uh oh!

vstinner commented Jun 24, 2025

Uh oh!

picnixz commented Jun 24, 2025 •
edited
Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-135239: simpler use of mutex in hashlib & co by picnixz · Pull Request #135267 · python/cpython · GitHub

Repository navigation

gh-135239: simpler use of mutex in hashlib & co - #135267

gh-135239: simpler use of mutex in hashlib & co#135267picnixz merged 45 commits intopython:mainpython/cpython:mainfrom picnixz:perf/hashlib/mutex-135239picnixz/cpython:perf/hashlib/mutex-135239Copy head branch name to clipboard

Conversation

picnixz commented Jun 8, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

picnixz commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz commented Jun 22, 2025

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

picnixz commented Jun 22, 2025

Uh oh!

vstinner commented Jun 24, 2025

Uh oh!

picnixz commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-135239: simpler use of mutex in hashlib & co#135267
picnixz merged 45 commits into
python:mainfrom
picnixz:perf/hashlib/mutex-135239

picnixz commented Jun 8, 2025 •
edited by bedevere-app Bot
Loading

picnixz commented Jun 22, 2025 •
edited
Loading

picnixz commented Jun 24, 2025 •
edited
Loading