| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Fix three issues that caused mimalloc pages to be leaked until the owning thread exited: 1. In _PyMem_mi_page_maybe_free(), move pages out of the full queue when relying on QSBR to defer freeing the page. Pages in the full queue are never searched by mi_page_queue_find_free_ex(), so a page left there is unusable for allocations. 2. Move _PyMem_mi_page_clear_qsbr() from _mi_page_free_collect() to _mi_page_thread_free_collect() where it only fires when all blocks on the page are free (used == 0). The previous placement was too broad: it cleared QSBR state whenever local_free was non-NULL, but _mi_page_free_collect() is called from non-allocation paths (e.g., page visiting in mi_heap_visit_blocks) where the page is not being reused. 3. In _PyMem_mi_page_maybe_free(), use the page's heap tld to find the correct thread state for QSBR list insertion instead of PyThreadState_GET(). During stop-the-world pauses, the function may process pages belonging to other threads, so the current thread state is not necessarily the owner of the page.
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 2516a1a 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145626%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
|
The changes look good to me, though I am not intimately familiar with mimalloc or our modifications to it. The buildbot failure seems like it might be real though. It looks like test.test_tools.test_freeze.TestFreeze.test_freeze_simple_script is consistently timing out on this PR. None of the recent runs on the buildbot failed in the same way. |
Sorry, something went wrong.
|
I don't think the buildbot failure is related. The PR only touches mimalloc and Py_GIL_DISABLED code, which isn't used by that buildbot configuration. There are fairly frequent time outs at random places with that buildbot: https://buildbot.python.org/#/workers/4. The test_freeze_simple_script test is pretty slow and isn't run in GitHub CI. |
Sorry, something went wrong.
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
…ythongh-145626) Fix three issues that caused mimalloc pages to be leaked until the owning thread exited: 1. In _PyMem_mi_page_maybe_free(), move pages out of the full queue when relying on QSBR to defer freeing the page. Pages in the full queue are never searched by mi_page_queue_find_free_ex(), so a page left there is unusable for allocations. 2. Move _PyMem_mi_page_clear_qsbr() from _mi_page_free_collect() to _mi_page_thread_free_collect() where it only fires when all blocks on the page are free (used == 0). The previous placement was too broad: it cleared QSBR state whenever local_free was non-NULL, but _mi_page_free_collect() is called from non-allocation paths (e.g., page visiting in mi_heap_visit_blocks) where the page is not being reused. 3. In _PyMem_mi_page_maybe_free(), use the page's heap tld to find the correct thread state for QSBR list insertion instead of PyThreadState_GET(). During stop-the-world pauses, the function may process pages belonging to other threads, so the current thread state is not necessarily the owner of the page. (cherry picked from commit d76df75) Co-authored-by: Sam Gross <colesbury@gmail.com>
|
GH-145691 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
…h-145626) (#145691) Fix three issues that caused mimalloc pages to be leaked until the owning thread exited: 1. In _PyMem_mi_page_maybe_free(), move pages out of the full queue when relying on QSBR to defer freeing the page. Pages in the full queue are never searched by mi_page_queue_find_free_ex(), so a page left there is unusable for allocations. 2. Move _PyMem_mi_page_clear_qsbr() from _mi_page_free_collect() to _mi_page_thread_free_collect() where it only fires when all blocks on the page are free (used == 0). The previous placement was too broad: it cleared QSBR state whenever local_free was non-NULL, but _mi_page_free_collect() is called from non-allocation paths (e.g., page visiting in mi_heap_visit_blocks) where the page is not being reused. 3. In _PyMem_mi_page_maybe_free(), use the page's heap tld to find the correct thread state for QSBR list insertion instead of PyThreadState_GET(). During stop-the-world pauses, the function may process pages belonging to other threads, so the current thread state is not necessarily the owner of the page. (cherry picked from commit d76df75) Co-authored-by: Sam Gross <colesbury@gmail.com>
|
(Late to the party but...) should the changes to mimalloc source be upstreamed? |
Sorry, something went wrong.
|
No, the PR is modifications to our mimalloc modifications / obmalloc.c code. The bugs were in code I wrote, not in upstream mimalloc. |
Sorry, something went wrong.
|
I'll put a note on the issue to make that more clear. |
Sorry, something went wrong.
…ythongh-145626) Fix three issues that caused mimalloc pages to be leaked until the owning thread exited: 1. In _PyMem_mi_page_maybe_free(), move pages out of the full queue when relying on QSBR to defer freeing the page. Pages in the full queue are never searched by mi_page_queue_find_free_ex(), so a page left there is unusable for allocations. 2. Move _PyMem_mi_page_clear_qsbr() from _mi_page_free_collect() to _mi_page_thread_free_collect() where it only fires when all blocks on the page are free (used == 0). The previous placement was too broad: it cleared QSBR state whenever local_free was non-NULL, but _mi_page_free_collect() is called from non-allocation paths (e.g., page visiting in mi_heap_visit_blocks) where the page is not being reused. 3. In _PyMem_mi_page_maybe_free(), use the page's heap tld to find the correct thread state for QSBR list insertion instead of PyThreadState_GET(). During stop-the-world pauses, the function may process pages belonging to other threads, so the current thread state is not necessarily the owner of the page.
| Back | FazBrowse Home | New Git URL |
Fix three issues that caused mimalloc pages to be leaked until the owning thread exited:
In _PyMem_mi_page_maybe_free(), move pages out of the full queue when relying on QSBR to defer freeing the page. Pages in the full queue are never searched by mi_page_queue_find_free_ex(), so a page left there is unusable for allocations.
Move _PyMem_mi_page_clear_qsbr() from _mi_page_free_collect() to _mi_page_thread_free_collect() where it only fires when all blocks on the page are free (used == 0). The previous placement was too broad: it cleared QSBR state whenever local_free was non-NULL, but _mi_page_free_collect() is called from non-allocation paths (e.g., page visiting in mi_heap_visit_blocks) where the page is not being reused.
In _PyMem_mi_page_maybe_free(), use the page's heap tld to find the correct thread state for QSBR list insertion instead of PyThreadState_GET(). During stop-the-world pauses, the function may process pages belonging to other threads, so the current thread state is not necessarily the owner of the page.