Skip to content

BUG: Remove thread-local allocator cache implementation#31237

Merged
ngoldbaum merged 5 commits into
numpy:mainfrom
ngoldbaum:rm-alloc-cache-ft
Apr 14, 2026
Merged

BUG: Remove thread-local allocator cache implementation#31237
ngoldbaum merged 5 commits into
numpy:mainfrom
ngoldbaum:rm-alloc-cache-ft

Conversation

@ngoldbaum

@ngoldbaum ngoldbaum commented Apr 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

PR summary

Fixes #31179.

This reverts #30499 and #30673 in favor of the old single global allocator cache on the GIL-enabled build and no cache on the free-threaded build. Since #30499 was merged, CPython has changed PyMy_RawMalloc to use mimalloc instead of the system allocator on the free-threaded build. That means we can rely on mimalloc to cache allocations.

The thread-local cache we have currently is complicated and leads to observed races in our CI (#31179).

Using uv's build of Python, I consistently see better allocation performance on the GIL-enabled build:


| Change   | Before [7a0dfadc] <main>   | After [be63b9f2] <rm-alloc-cache-ft>   |   Ratio | Benchmark (Parameter)                                    |
|----------|----------------------------|----------------------------------------|---------|----------------------------------------------------------|
| -        | 2.89±0.03ms                | 2.76±0.02ms                            |    0.96 | bench_alloc_cache.SmallArrayCreation.time_full_loop(128) |
| -        | 2.59±0.02ms                | 2.50±0.04ms                            |    0.96 | bench_alloc_cache.SmallArrayCreation.time_full_loop(64)  |
| -        | 2.91±0.03ms                | 2.76±0.02ms                            |    0.95 | bench_alloc_cache.SmallArrayCreation.time_full_loop(256) |
| -        | 2.63±0.03ms                | 2.48±0.02ms                            |    0.94 | bench_alloc_cache.SmallArrayCreation.time_full_loop(16)  |
| -        | 2.62±0.02ms                | 2.47±0.03ms                            |    0.94 | bench_alloc_cache.SmallArrayCreation.time_full_loop(4)   |

I see no significant performance change on the free-threaded build.

AI Disclosure

I used Claude to help generate the new benchmark. Happy to leave it out if you don't think it's useful.

@ngoldbaum ngoldbaum added this to the 2.5.0 Release milestone Apr 13, 2026
@ngoldbaum ngoldbaum added 28 - Benchmark 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703) labels Apr 13, 2026
seberg
seberg approved these changes Apr 14, 2026
View reviewed changes

@seberg seberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding benchmarks even. Cool if this just works out for free-threading!

Comment thread numpy/_core/src/multiarray/alloc.c
* CPython uses mimalloc on the free-threaded build, which we trust to cache
* allocations better than we can.
*/
#ifdef Py_GIL_DISABLED

@seberg seberg Apr 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we maybe just do || PYTHON_USING_MIMALLOC (whatever that exact spelling is)?

@ngoldbaum ngoldbaum Apr 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The interpreter only uses mimalloc for the PyMem_Raw on the free-threaded build, so the way I have it spelled here is correct I think.

mhvk
mhvk approved these changes Apr 14, 2026
View reviewed changes

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good. Agreed with @seberg that we might as well also not use our home-brew cache generally if a better method is available. Even better if that is linked to a python version -- if so, it will be more obvious when we can just remove the cache altogether.

@ngoldbaum

ngoldbaum commented Apr 14, 2026

Copy link
Copy Markdown
Member Author

Two approvals so I'm gonna merge this, thanks all! Also thanks @kumaraditya303 for working on this. No shame that we ended up just backing out the two PRs since it led you to making the default allocator mimalloc on the free-threaded build, which benefits all free-threaded applications.

@ngoldbaum ngoldbaum merged commit 4a126b3 into numpy:main Apr 14, 2026
87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seberg seberg seberg approved these changes
@mhvk mhvk mhvk approved these changes
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303

Assignees

No one assigned

Labels

28 - Benchmark 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703)

Projects

None yet

Milestone

2.5.0 Release

Development

Successfully merging this pull request may close these issues.

TSan race in test_structured_advanced_indexing

3 participants

@ngoldbaum @seberg @mhvk