Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
gh-115103: Delay reuse of mimalloc pages that store PyObjects#115435
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b811432
to99bd660
Compare!buildbot nogil |
bedevere-bot commentedMar 5, 2024
🤖 New build scheduled with the buildbot fleet by@colesbury for commit99bd660 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
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.
Looks good to me, just a few nits really...
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit: Seems like we could consistently use_PyMem_mi_page_clear_qsbr
?
Objects/obmalloc.c Outdated
return; | ||
} | ||
page->qsbr_goal = 0; |
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.
_PyMem_mi_page_clear_qsbr
?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 inmi_segment_page_clear
and_PyMem_mi_page_maybe_free
99bd660
to1645d3a
CompareThere 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.
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_reclaimed
logic 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_free
to only handle reclaimed pages, instead of also_PyMem_mi_page_maybe_free
case. Theqsbr_goal == 0
has different implications in the two cases. When reclaiming an abandoned page,qsbr_goal == 0
means that we can free it immediately, while inmi_heap_page_collect
it means that we haven't started the QSBR process. _mi_page_retire
should clear the previous QSBR goal, if one exists (since we just freed a block).
!buildbot nogil |
bedevere-bot commentedMar 5, 2024
🤖 New build scheduled with the buildbot fleet by@colesbury for commit1645d3a 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
!buildbot nogil |
bedevere-bot commentedMar 5, 2024
🤖 New build scheduled with the buildbot fleet by@colesbury for commit3453eb9 🤖 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 |
…ython#115435)This implements the delayed reuse of mimalloc pages that contain Pythonobjects in the free-threaded build.Allocations of the same size class are grouped in data structures calledpages. These are different from operating system pages. For thread-safety, wewant to ensure that memory used to store PyObjects remains valid as long asthere may be concurrent lock-free readers; we want to delay using it forother size classes, in other heaps, or returning it to the operating system.When a mimalloc page becomes empty, instead of immediately freeing it, we tagit with a QSBR goal and insert it into a per-thread state linked list ofpages to be freed. When mimalloc needs a fresh page, we process the queue andfree any still empty pages that are now deemed safe to be freed. Pageswaiting to be freed are still available for allocations of the same sizeclass and allocating from a page prevent it from being freed. There isadditional logic to handle abandoned pages when threads exit.
…ython#115435)This implements the delayed reuse of mimalloc pages that contain Pythonobjects in the free-threaded build.Allocations of the same size class are grouped in data structures calledpages. These are different from operating system pages. For thread-safety, wewant to ensure that memory used to store PyObjects remains valid as long asthere may be concurrent lock-free readers; we want to delay using it forother size classes, in other heaps, or returning it to the operating system.When a mimalloc page becomes empty, instead of immediately freeing it, we tagit with a QSBR goal and insert it into a per-thread state linked list ofpages to be freed. When mimalloc needs a fresh page, we process the queue andfree any still empty pages that are now deemed safe to be freed. Pageswaiting to be freed are still available for allocations of the same sizeclass and allocating from a page prevent it from being freed. There isadditional logic to handle abandoned pages when threads exit.
Uh oh!
There was an error while loading.Please reload this page.
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.
Seehttps://peps.python.org/pep-0703/#mimalloc-page-reuse