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-90815: Add mimalloc memory allocator#109914

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
vstinner merged 24 commits intopython:mainfromDinoV:nogil/mimalloc_212
Oct 30, 2023

Conversation

DinoV
Copy link
Contributor

@DinoVDinoV commentedSep 26, 2023
edited
Loading

This adds mimalloc as an optional (but preferred when available) allocator to CPython. This is a bit of a mashup of the work from#109914 and the work of@colesbury to use mimalloc for no-gil and various updates to bring it up to current CPython.

The configuration logic added by@tiran is re-used and we keep pymalloc support unlike in the version from@colesbury. mimalloc is updated to 2.12 and along with a few changes@colesbury made to it.

This has run into some issues with subinterpreter support in that the allocator's are now stored in thread state and are per-thread. Sub interpreters in some scenarios will create a thread state on one thread and run that on another thread. Most of these are documented in the code base as being known issues. I've modified these so that we will find the right thread based upon the current thread ID and switch to it rather than getting the head thread. This seems pretty reasonable but looks pretty weird when we need to do it at interpreter shutdown.

vasily-v-ryabov reacted with thumbs up emoji
@DinoVDinoVforce-pushed thenogil/mimalloc_212 branch 7 times, most recently from8b3ec52 to06e86d7CompareSeptember 27, 2023 00:25
@DinoVDinoV changed the titleNogil/mimalloc 212gh-90815: Add mimalloc memory allocatorSep 27, 2023
@DinoVDinoVforce-pushed thenogil/mimalloc_212 branch 4 times, most recently frome8c4f01 to2dd8675CompareSeptember 27, 2023 15:59
@ericsnowcurrently
Copy link
Member

FTR,@tiran'sgh-31164 is a similar change and has a bunch of discussion.

CC@daanx

@DinoV
Copy link
ContributorAuthor

@ericsnowcurrently Yep, and much of the autoconf stuff is taken from there... I just finally filled in the description with a little more background on where things are coming from :)

@ericsnowcurrently
Copy link
Member

This has run into some issues with subinterpreter support in that the allocator's are now stored in thread state and are per-thread.

I expect that most of this PR is a relatively vanilla use of mimalloc in CPython, like@tiran's PR is. Would it be much trouble to keep the whole PR vanilla and have a follow-up PR that applies the customizations to mimalloc (e.g. per-thread allocator state)? That distinction would help, possibly a lot, when reviewing.

DinoV reacted with thumbs up emoji

@DinoVDinoVforce-pushed thenogil/mimalloc_212 branch 7 times, most recently from7bda1e4 to35d1ebdCompareSeptember 28, 2023 19:52
@DinoVDinoV marked this pull request as ready for reviewSeptember 28, 2023 20:21
@DinoV
Copy link
ContributorAuthor

Okay, the default is now pymalloc, for nowPYTHONMALLOC=mimalloc ./python will pick up mimalloc. When we start to get nogil integration with it we'll need to make it the default/required in that config.

@thesamesam
Copy link
Contributor

thesamesam commentedOct 30, 2023
edited
Loading

Is it possible for mimalloc?

That's not possible because we will have changes to mimalloc.

Is there any chance of those changes getting upstreamed? There's advantages for cpython then too -- way less work to rebase the internal copy and no real risk of bugs being cpython introduced.

Any thoughts? I think this got missed earlier in the review flurry. I've not seen any substantive discussion about why bundling is a good idea, which feels like it's worth proper consideration, given it's short-term easier but long-term expensive.

I understand upstreaming it initially is going to take some time, but it is going to be worthwhile for cpython upstream maintenance long-term too, as you can avoid:

  • having cpython being stuck on outdated versions in perpetuity, or
  • figuring out how to rebase (and include the local changes)
  • backporting fixes for various platforms/new compilers/etc to increasingly old versions of mimalloc

This also, of course, has the advantage of forcing one to document what the changes are which I think should be done either way.

Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

See also issue#111499: PYTHONMALLOCSTATS=1 fails with fatal error at Python exit. Using mimalloc, Python is not affected :-)

@vstinner
Copy link
Member

@DinoV: I pushed a few cleanup changes. I fixed the outdated doc which still said that mimalloc is the default.

@vstinnervstinnerenabled auto-merge (squash)October 30, 2023 15:25
@vstinner
Copy link
Member

I cleaned the commit message to only keep most important information.

The configuration logic added by@tiran is re-used

I added@tiran as a co-author in the commit message.

@ericsnowcurrently
Copy link
Member

Is there any chance of those changes getting upstreamed? There's advantages for cpython then too -- way less work to rebase the internal copy and no real risk of bugs being cpython introduced.

Any thoughts?

It's worth further discussion.

CC@daanx

@vstinnervstinner merged commit05f2f0a intopython:mainOct 30, 2023
@vstinner
Copy link
Member

Congrats@DinoV, I merged your PR :-) Do you want to propose a follow-up PR to enable it by default when--disable-gil is used?

DinoV reacted with heart emoji

@ericsnowcurrently
Copy link
Member

Looks like this broke the WASM buildbot:https://buildbot.python.org/all/#/builders/1046/builds/3371.

@brettcannon
Copy link
Member

Looks like this broke the WASM buildbot:https://buildbot.python.org/all/#/builders/1046/builds/3371.

Yep, looks like there's an implicit function definition:

In file included from ../../Objects/obmalloc.c:15:In file included from ../../Objects/mimalloc/static.c:37:In file included from ../../Objects/mimalloc/prim/prim.c:19:../../Objects/mimalloc/prim/wasi/prim.c:44:15: error: implicit declaration of function 'sbrk' is invalid in C99 [-Werror,-Wimplicit-function-declaration]    void* p = sbrk(size);              ^1 error generated.make: *** [Makefile:2723: Objects/obmalloc.o] Error 1

@vstinner
Copy link
Member

../../Objects/mimalloc/prim/wasi/prim.c:44:15: error: implicit declaration of function 'sbrk' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

I wrote PR#111524 to fix WASI build.

DinoV reacted with thumbs up emojibrettcannon reacted with heart emoji

@vstinner
Copy link
Member

I wrote 4 follow-up PRs to fix different issues:

While mimalloc C code is built on Windows, Python cannot currently use mimalloc on Windows: see PR#111528.

stonebig reacted with heart emoji

FullteaR pushed a commit to FullteaR/cpython that referenced this pull requestNov 3, 2023
* Add mimalloc v2.12Modified src/alloc.c to remove include of alloc-override.c and notcompile new handler.Did not include the following files: - include/mimalloc-new-delete.h - include/mimalloc-override.h - src/alloc-override-osx.c - src/alloc-override.c - src/static.c - src/region.cmimalloc is thread safe and shares a single heap across all runtimes,therefore finalization and getting global allocated blocks across allruntimes is different.* mimalloc: minimal changes for use in Python: - remove debug spam for freeing large allocations - use same bytes (0xDD) for freed allocations in CPython and mimalloc   This is important for the test_capi debug memory tests* Don't export mimalloc symbol in libpython.* Enable mimalloc as Python allocator option.* Add mimalloc MIT license.* Log mimalloc in Lib/test/pythoninfo.py.* Document new mimalloc support.* Use macro defs for exports as done in:python#31164Co-authored-by: Sam Gross <colesbury@gmail.com>Co-authored-by: Christian Heimes <christian@python.org>Co-authored-by: Victor Stinner <vstinner@python.org>
aisk pushed a commit to aisk/cpython that referenced this pull requestFeb 11, 2024
* Add mimalloc v2.12Modified src/alloc.c to remove include of alloc-override.c and notcompile new handler.Did not include the following files: - include/mimalloc-new-delete.h - include/mimalloc-override.h - src/alloc-override-osx.c - src/alloc-override.c - src/static.c - src/region.cmimalloc is thread safe and shares a single heap across all runtimes,therefore finalization and getting global allocated blocks across allruntimes is different.* mimalloc: minimal changes for use in Python: - remove debug spam for freeing large allocations - use same bytes (0xDD) for freed allocations in CPython and mimalloc   This is important for the test_capi debug memory tests* Don't export mimalloc symbol in libpython.* Enable mimalloc as Python allocator option.* Add mimalloc MIT license.* Log mimalloc in Lib/test/pythoninfo.py.* Document new mimalloc support.* Use macro defs for exports as done in:python#31164Co-authored-by: Sam Gross <colesbury@gmail.com>Co-authored-by: Christian Heimes <christian@python.org>Co-authored-by: Victor Stinner <vstinner@python.org>
@DinoVDinoV deleted the nogil/mimalloc_212 branchMay 31, 2024 18:23
Glyphack pushed a commit to Glyphack/cpython that referenced this pull requestSep 2, 2024
* Add mimalloc v2.12Modified src/alloc.c to remove include of alloc-override.c and notcompile new handler.Did not include the following files: - include/mimalloc-new-delete.h - include/mimalloc-override.h - src/alloc-override-osx.c - src/alloc-override.c - src/static.c - src/region.cmimalloc is thread safe and shares a single heap across all runtimes,therefore finalization and getting global allocated blocks across allruntimes is different.* mimalloc: minimal changes for use in Python: - remove debug spam for freeing large allocations - use same bytes (0xDD) for freed allocations in CPython and mimalloc   This is important for the test_capi debug memory tests* Don't export mimalloc symbol in libpython.* Enable mimalloc as Python allocator option.* Add mimalloc MIT license.* Log mimalloc in Lib/test/pythoninfo.py.* Document new mimalloc support.* Use macro defs for exports as done in:python#31164Co-authored-by: Sam Gross <colesbury@gmail.com>Co-authored-by: Christian Heimes <christian@python.org>Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@vstinnervstinnervstinner left review comments

@corona10corona10corona10 left review comments

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@colesburycolesburyAwaiting requested review from colesbury

@erlend-aaslanderlend-aaslandAwaiting requested review from erlend-aaslanderlend-aasland is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@DinoV@ericsnowcurrently@vstinner@itamaro@colesbury@thesamesam@brettcannon@corona10

[8]ページ先頭

©2009-2025 Movatter.jp