GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-135239: simpler use of mutex in hashlib & co by picnixz · Pull Request #135267 · python/cpython

Conversation

@picnixz

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
picnixz added 5 commits June 8, 2025 16:24
@picnixz
add common object head for hashlib/hmac objects
5d8c093
@picnixz
simplify digest computation
81e3046
@picnixz
refactor update logic
7f9f7b7
@picnixz
refactor alloc() logic
15a4f2f
@picnixz
finalizing touches
5cd828a
@picnixz
picnixz requested review from gpshead and tiran as code owners June 8, 2025 14:48
@bedevere-app bedevere-app Bot added the awaiting core review label Jun 8, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jun 8, 2025
Smarter use of a mutex in incremental HMAC and hash functions #135239
Closed
@picnixz 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
ZeroIntensity reviewed Jun 8, 2025
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/md5module.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
picnixz marked this pull request as draft June 8, 2025 19:37
@bedevere-app bedevere-app Bot removed the awaiting core review label Jun 8, 2025
@picnixz
correct mutex usage
63db1de
@picnixz
picnixz requested a review from ZeroIntensity June 15, 2025 09:40
picnixz added 2 commits June 15, 2025 11:42
@picnixz
Revert 5cd828a
ea033a3
@picnixz
revert some constructor changes
77baa67
@picnixz
picnixz marked this pull request as ready for review June 15, 2025 11:39
@bedevere-app bedevere-app Bot added the awaiting core review label Jun 15, 2025
@picnixz
unconditionally lock when performing HASH updates
902759f
@picnixz
picnixz marked this pull request as draft June 16, 2025 10:16
@bedevere-app bedevere-app Bot removed the awaiting core review label Jun 16, 2025
picnixz added 3 commits June 16, 2025 12:19
@picnixz
Merge remote-tracking branch 'upstream/main' into perf/hashlib/mutex-…
dde68c4 
…135239

# Conflicts:
#	Modules/_hashopenssl.c
#	Modules/blake2module.c
@picnixz
post-merge
05c1e66
@picnixz
do not guard against empty buffers for now
db57278
@picnixz
picnixz force-pushed the perf/hashlib/mutex-135239 branch from bb237c2 to db57278 Compare June 16, 2025 10:35
@picnixz
consistency fixes
ead20a1
picnixz added 3 commits June 21, 2025 15:34
@picnixz
make the test suite less slow
c9db0b1
@picnixz
fix test when GIL_MINSIZE is changed
6ffdd1c
@picnixz
defer cosmetics
98ec915
@picnixz
picnixz marked this pull request as ready for review June 21, 2025 13:46
@bedevere-app bedevere-app Bot added the awaiting core review label Jun 21, 2025
@picnixz

This comment was marked as resolved.

Sign in to view
@picnixz

This comment was marked as resolved.

Sign in to view
@bedevere-bot

This comment was marked as resolved.

Sign in to view
picnixz
picnixz commented Jun 21, 2025
View reviewed changes
Comment thread Lib/test/test_hashlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
Update Lib/test/test_hashlib.py
398ddb3
@picnixz

This comment was marked as resolved.

Sign in to view
@bedevere-bot

This comment was marked as resolved.

Sign in to view
@picnixz

This comment was marked as resolved.

Sign in to view
@picnixz

picnixz commented Jun 22, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
improve test
0ae70e9
@picnixz

picnixz commented Jun 22, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity
ZeroIntensity approved these changes Jun 22, 2025
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The locking looks good to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jun 22, 2025
@picnixz
picnixz merged commit e7295a8 into python:main Jun 22, 2025
@picnixz
picnixz deleted the perf/hashlib/mutex-135239 branch June 22, 2025 15:00
@bedevere-app bedevere-app Bot removed the awaiting merge label Jun 22, 2025
@picnixz

picnixz commented Jun 22, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Jun 24, 2025

Copy link
Copy Markdown
Member

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

Why? What is the problem?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz

picnixz commented Jun 24, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
@picnixz @AndPuQing
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
@picnixz @Pranjal095
pythongh-135239: simpler use of mutexes in cryptographic modules (pyt…
2dfda94 
…hon#135267)
@picnixz 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
@picnixz @taegyunkim
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
@picnixz @Agent-Hellboy
pythongh-135239: simpler use of mutexes in cryptographic modules (pyt…
2b23cd6 
…hon#135267)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes
@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner
@tiran tiran Awaiting requested review from tiran
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news type-refactor Code refactoring (with no changes in behavior)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@picnixz @bedevere-bot @ZeroIntensity @vstinner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.