gh-115103: Delay reuse of mimalloc pages that store PyObjects #115435
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
colesbury
force-pushed
the
gh-115103-mimalloc-qsbr
branch
2 times, most recently
from
b811432
to
99bd660
Compare
|
!buildbot nogil |
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 99bd660 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
DinoV
approved these changes
Mar 5, 2024
Objects/obmalloc.c
Outdated
| mi_page_t *page = llist_data(node, mi_page_t, qsbr_node); | ||
| if (!mi_page_all_free(page)) { | ||
| // We allocated from this page some point after the delayed free | ||
| page->qsbr_goal = 0; |
There was a problem hiding this comment.
nit: Seems like we could consistently use _PyMem_mi_page_clear_qsbr?
Objects/obmalloc.c
Outdated
| return; | ||
| } | ||
|
|
||
| page->qsbr_goal = 0; |
Objects/obmalloc.c
Outdated
| if (!_Py_qbsr_goal_reached(tstate->qsbr, page->qsbr_goal)) { | ||
| return false; | ||
| } | ||
| _PyMem_mi_page_clear_qsbr(page); |
There was a problem hiding this comment.
This side effect seems surprising given the name of the function... It seems like it'd be better to call _PyMem_mi_page_clear_qsbr explicitly in mi_segment_page_clear and _PyMem_mi_page_maybe_free
colesbury
force-pushed
the
gh-115103-mimalloc-qsbr
branch
from
99bd660
to
1645d3a
Compare
colesbury
commented
Mar 5, 2024
There was a problem hiding this comment.
I've rebased now that #116343 landed.
- I fixed a bug with reclaiming segments that led to the buildbot failures (see
mi_segment_reclaim). Large pages only contain a single block. Prior to this PR, they were either empty and freed immediately on reclamation or full and so didn't setright_page_reclaimed. Theright_page_reclaimedlogic doesn't handle large pages properly because large pages use a different allocation code path than small and medium pages. This means we have slightly worse re-use than we'd like when reclaiming large pages, but at least it's correct now. - I changed
_PyMem_mi_page_is_safe_to_freeto only handle reclaimed pages, instead of also_PyMem_mi_page_maybe_freecase. Theqsbr_goal == 0has different implications in the two cases. When reclaiming an abandoned page,qsbr_goal == 0means that we can free it immediately, while inmi_heap_page_collectit means that we haven't started the QSBR process. _mi_page_retireshould clear the previous QSBR goal, if one exists (since we just freed a block).
|
!buildbot nogil |
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 1645d3a 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
|
!buildbot nogil |
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 3453eb9 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
|
The failed buildbot looks unrelated to this PR. |
|
CC @daanx |
adorilson
pushed a commit
to adorilson/cpython
that referenced
this pull request
Mar 25, 2024
…ython#115435) This implements the delayed reuse of mimalloc pages that contain Python objects in the free-threaded build. Allocations of the same size class are grouped in data structures called pages. These are different from operating system pages. For thread-safety, we want to ensure that memory used to store PyObjects remains valid as long as there may be concurrent lock-free readers; we want to delay using it for other size classes, in other heaps, or returning it to the operating system. When a mimalloc page becomes empty, instead of immediately freeing it, we tag it with a QSBR goal and insert it into a per-thread state linked list of pages to be freed. When mimalloc needs a fresh page, we process the queue and free any still empty pages that are now deemed safe to be freed. Pages waiting to be freed are still available for allocations of the same size class and allocating from a page prevent it from being freed. There is additional logic to handle abandoned pages when threads exit.
diegorusso
pushed a commit
to diegorusso/cpython
that referenced
this pull request
Apr 17, 2024
…ython#115435) This implements the delayed reuse of mimalloc pages that contain Python objects in the free-threaded build. Allocations of the same size class are grouped in data structures called pages. These are different from operating system pages. For thread-safety, we want to ensure that memory used to store PyObjects remains valid as long as there may be concurrent lock-free readers; we want to delay using it for other size classes, in other heaps, or returning it to the operating system. When a mimalloc page becomes empty, instead of immediately freeing it, we tag it with a QSBR goal and insert it into a per-thread state linked list of pages to be freed. When mimalloc needs a fresh page, we process the queue and free any still empty pages that are now deemed safe to be freed. Pages waiting to be freed are still available for allocations of the same size class and allocating from a page prevent it from being freed. There is additional logic to handle abandoned pages when threads exit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This implements the delayed reuse of mimalloc pages that contain Python objects in the free-threaded build.
Allocations of the same size class are grouped in data structures called pages. These are different from operating system pages. For thread-safety, we want to ensure that memory used to store PyObjects remains valid as long as there may be concurrent lock-free readers; we want to delay using it for other size classes, in other heaps, or returning it to the operating system.
When a mimalloc page becomes empty, instead of immediately freeing it, we tag it with a QSBR goal and insert it into a per-thread state linked list of pages to be freed. When mimalloc needs a fresh page, we process the queue and free any still empty pages that are now deemed safe to be freed. Pages waiting to be freed are still available for allocations of the same size class and allocating from a page prevent it from being freed. There is additional logic to handle abandoned pages when threads exit.
See https://peps.python.org/pep-0703/#mimalloc-page-reuse