Repository navigation
Conversation
|
The following commit authors need to sign the Contributor License Agreement: |
Documentation build overview
48 files changed ·
|
|
@daanx can you please sign the CLA? |
Ah yes, but not really for the current PR as that includes the full mimalloc v3 vendored sources and that is a separate project under the MIT license. I can of course sign off for the changes I made on the CPython side. The current PR contains both, maybe we can split it? I think commit cedd47b is the CPython part only, while the other commits are just importing mimalloc v3 as a vendored library. |
daanx
left a comment
There was a problem hiding this comment.
Using MI_WIN_INIT_USE_FLS is not really encouraged for mimalloc. If we can use the default that would be best.
There's a crash on the Windows builds w/o it currently (and it seems to show up basically at startup but the freeze programs make it through it). I was planning on digging into it more later but this is what Claude came up as a fix - I just don't have immediate access to a Windows machine to see where the actual crash is coming from. But that's why it's there for now. Right now I'm just trying to get things to green CI and then I'll clean up the final result. |
|
@corona10 Hey here it is! |
FWIW the docs due already call out mimalloc license: https://docs.python.org/3/license.html#mimalloc although we should probably update the date range w/ the v3 update. |
2x to 1.14x but still higher than mimalloc v2 |
| } | ||
| } | ||
| // Hooks can run under allocator locks, so only consult cached progress. |
There was a problem hiding this comment.
| // Hooks can run under allocator locks, so only consult cached progress. | |
| if (qsbr != NULL) { | |
| // Scan thread sequences like the v2 integration did. | |
| return !_Py_qsbr_poll(qsbr, page->goal); | |
| } | |
| // Hooks can run under allocator locks, so only consult cached progress. |
That was still a 6% regression on memory (but things are headed in the right direction, THP tweak about it from 2x to 1.15, and then the _Py_qsbr_poll fix got us here), here's another run https://github.com/facebookexperimental/free-threading-benchmarking/actions/runs/39476892845 which has commit 3c8be4f that sets |
|
Another run with new options for mi_option_arena_purge_min_size, and mi_option_page_full_retain and a tweaked mi_option_arena_purge_mult: https://github.com/facebookexperimental/free-threading-benchmarking/actions/runs/39497633255 |
|
Down to 3% |
|
This is now neutral: https://github.com/facebookexperimental/free-threading-benchmarking/tree/main/results/bm-20261020-3.16.0a1+dev-a36efb6-NOGIL It includes one more upstream fix which is a36efb6 that re-enables the @daanx Thanks for merging all of the other upstream PRs I sent. I can send another one to re-enable this but I can also throw it under another ifdef if you're concerned about having it on in the common case. But I think this basically brings us to parity where we could merge this. I'd still want to bring back the latest commits you have upstream but my previous run had that as neutral. I also am assuming your dev3 branch isn't yet released? |
Sync Objects/mimalloc and Include/internal/mimalloc with microsoft/mimalloc dev3 (1ff2382, "fix profile timestamps"). Most of the local edits previously carried in the vendored copy have been merged upstream (forceinline declaration order, WASI page size, quoted includes, MSan syscalls, Emscripten builtin allocator, MI_NO_ALIGN_FALLBACK_WARNING, MI_NO_CXX_NEW_HANDLER, __ImageBase, trailing whitespace). The FLS init assertion fix is dropped as CPython no longer uses MI_WIN_INIT_USE_FLS. Changes still carried on top of dev3 (proposed upstream): - Fix build with MI_NO_THP after the MADV_NOHUGEPAGE change - Fix -Wsign-compare warning when reading hpage_pmd_size - Fix data race on the thread-done pthread key at process exit CPython-specific change: - Purge expired ranges on arena free, as in mimalloc v2, to keep the peak memory of short-running processes at v2 levels. Define MI_NO_ALIGN_FALLBACK_WARNING in pycore_mimalloc.h, since the vendored copy now has the upstream opt-out instead of the warning being commented out.
|
Another benchmark run w/ the latest and greatest mimalloc/dev3 branch merged in: https://github.com/facebookexperimental/free-threading-benchmarking/actions/runs/39651451425 We're now down to 4 vendored in changes which I've opened upstream PRs for (the last being the most questionable I think): |
|
🤖 New build scheduled with the buildbot fleet by @DinoV for commit 7c4bdf4 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F159718%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
|
Still memory neutral after merging in the latest updates: https://github.com/facebookexperimental/free-threading-benchmarking/tree/main/results/bm-20261021-3.16.0a1+dev-7c4bdf4-NOGIL |
This upgrades mimalloc to v3 - the initial work here was done by @daanx and this pushes it over the edge to getting all CI passing and conforming we're at parity with the existing implementation. v3 brings with it baked in qsbr support and it has a number of the CPython specific changes upstreamed now.
To maintain parity with v2 on the pyperformance benchmarks we set several options back to their v2 levels in
pycore_mimalloc.h.