Skip to content

[3.14] GH-133136: Revise QSBR to reduce excess memory held (gh-135473) #135912

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

Open
wants to merge 5 commits into
base: 3.14
Choose a base branch
from

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Jun 25, 2025

The free threading build uses QSBR to delay the freeing of dictionary
keys and list arrays when the objects are accessed by multiple threads
in order to allow concurrent reads to proceed with holding the object
lock. The requests are processed in batches to reduce execution
overhead, but for large memory blocks this can lead to excess memory
usage.

Take into account the size of the memory block when deciding when to
process QSBR requests.

Also track the amount of memory being held by QSBR for mimalloc pages. Advance the write sequence if this memory exceeds a limit. Advancing the sequence will allow it to be freed more quickly.

Process the held QSBR items from the "eval breaker", rather than from _PyMem_FreeDelayed(). This gives a higher chance that the global read sequence has advanced enough so that items can be freed.
(cherry picked from commit 113de85)

Co-authored-by: Neil Schemenauer [email protected]
Co-authored-by: Sam Gross [email protected]

…35473)

The free threading build uses QSBR to delay the freeing of dictionary
keys and list arrays when the objects are accessed by multiple threads
in order to allow concurrent reads to proceed with holding the object
lock. The requests are processed in batches to reduce execution
overhead, but for large memory blocks this can lead to excess memory
usage.

Take into account the size of the memory block when deciding when to
process QSBR requests.

Also track the amount of memory being held by QSBR for mimalloc pages.  Advance the write sequence if this memory exceeds a limit.  Advancing the sequence will allow it to be freed more quickly.

Process the held QSBR items from the "eval breaker", rather than from `_PyMem_FreeDelayed()`.  This gives a higher chance that the global read sequence has advanced enough so that items can be freed.
(cherry picked from commit 113de85)

Co-authored-by: Neil Schemenauer <[email protected]>
Co-authored-by: Sam Gross <[email protected]>
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @nascheme for commit d7d6412 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135912%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 25, 2025
@nascheme nascheme requested a review from hugovk June 26, 2025 02:58
@nascheme
Copy link
Member

@hugovk I understand this needs review by the release manager due to the potential ABI change. I believe this change is okay. The _PyThreadStateImpl structure, which has been changed, is not exposed to extensions.

@hugovk hugovk requested a review from encukou as a code owner July 7, 2025 10:15
@hugovk
Copy link
Member

hugovk commented Jul 7, 2025

Yes, that's fine, I've updated the ABI file from the CI.

Let's also retrigger the buildbots because there was a failure with wasm32-wasi Non-Debug before: https://buildbot.python.org/#/builders/1373/builds/547

@hugovk
Copy link
Member

hugovk commented Jul 7, 2025

Ah, now it conflicts. Reverted and updated from 3.14 to regenerate on CI.

@hugovk hugovk added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 7, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @hugovk for commit 7320121 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135912%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 7, 2025
@nascheme
Copy link
Member

nascheme commented Jul 8, 2025

The Windows refleaks failure looks to be a known flaky test: GH-136186.

For the Arch ASAN buildbot, I'm not sure what's going on. Looks like test_sumprod_stress timing out is the trigger.

test_sumprod_stress (test.test_math.MathTests.test_sumprod_stress) ... Timeout (0:35:00)!
Thread 0x00007d6d0481a800 [python] (most recent call first):
  File "/buildbot/buildarea/pull_request.pablogsal-arch-x86_64.asan_debug/build/Lib/abc.py", line 119 in __instancecheck__
=================================================================
==758963==ERROR: AddressSanitizer: stack-use-after-return on address 0x7d6d02d2d90a at pc 0x60307d27b9c1 bp 0x7d6d00f5fba0 sp 0x7d6d00f5fb90
READ of size 1 at 0x7d6d02d2d90a thread T1
    #0 0x60307d27b9c0 in dump_traceback Python/traceback.c:1059
    #1 0x60307d27bba2 in _Py_DumpTracebackThreads Python/traceback.c:1218
    #2 0x60307d2a944c in faulthandler_thread Modules/faulthandler.c:667
    #3 0x60307d27556d in pythread_wrapper Python/thread_pthread.h:242
    #4 0x7d6d04e19ec9 in asan_thread_start /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:234
    #5 0x7d6d04b68709  (/usr/lib/libc.so.6+0x95709) (BuildId: 0b707b217b15b106c25fe51df3724b25848310c0)
    #6 0x7d6d04becaab  (/usr/lib/libc.so.6+0x119aab) (BuildId: 0b707b217b15b106c25fe51df3724b25848310c0)

Address 0x7d6d02d2d90a is located in stack of thread T0 at offset 2314 in frame
    #0 0x60307d0f9773 in _PyEval_EvalFrameDefault Python/ceval.c:997

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants