Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
colesbury merged 3 commits intopython:mainfromcolesbury:gh-115103-mimalloc-qsbr
Mar 6, 2024

Conversation

colesbury
Copy link
Contributor

@colesburycolesbury commentedFeb 13, 2024
edited
Loading

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

@colesbury
Copy link
ContributorAuthor

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@colesbury for commit99bd660 🤖

The command will test the builders whose names match following regular expression:nogil

The builders matched are:

  • AMD64 Ubuntu NoGIL Refleaks PR
  • x86-64 MacOS Intel ASAN NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • ARM64 MacOS M1 NoGIL PR

Copy link
Contributor

@DinoVDinoV left a 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...

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;
Copy link
Contributor

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?

colesbury reacted with thumbs up emoji
return;
}

page->qsbr_goal = 0;
Copy link
Contributor

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?

colesbury reacted with thumbs up emoji
if (!_Py_qbsr_goal_reached(tstate->qsbr, page->qsbr_goal)) {
return false;
}
_PyMem_mi_page_clear_qsbr(page);
Copy link
Contributor

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

colesbury reacted with thumbs up emoji
@colesburycolesburyforce-pushed thegh-115103-mimalloc-qsbr branch from99bd660 to1645d3aCompareMarch 5, 2024 23:03
Copy link
ContributorAuthor

@colesburycolesbury left a 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 (seemi_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).

DinoV reacted with thumbs up emoji
@colesbury
Copy link
ContributorAuthor

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@colesbury for commit1645d3a 🤖

The command will test the builders whose names match following regular expression:nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • AMD64 Windows Server 2022 NoGIL PR

@colesbury
Copy link
ContributorAuthor

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@colesbury for commit3453eb9 🤖

The command will test the builders whose names match following regular expression:nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • AMD64 Windows Server 2022 NoGIL PR

@colesbury
Copy link
ContributorAuthor

The failed buildbot looks unrelated to this PR.

@colesburycolesbury merged commitc012c8a intopython:mainMar 6, 2024
@colesburycolesbury deleted the gh-115103-mimalloc-qsbr branchMarch 6, 2024 14:42
@ericsnowcurrently
Copy link
Member

CC@daanx

adorilson pushed a commit to adorilson/cpython that referenced this pull requestMar 25, 2024
…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.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull requestApr 17, 2024
…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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@DinoVDinoVDinoV approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@colesbury@bedevere-bot@ericsnowcurrently@DinoV

[8]ページ先頭

©2009-2025 Movatter.jp