refactor(allocator): add Arena::grow_fixed_size_chunk method#22123
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Pull request overview
Adds a new internal hook for fixed-size arenas to attempt in-place growth of the current chunk, and wires that hook into the allocation slow path. This is preparatory scaffolding for a future Windows-specific implementation.
Changes:
- Introduce
Arena::grow_fixed_size_chunkas a per-platform implementation stub (currently always returnsNone). - Call
grow_fixed_size_chunkfromtry_alloc_layout_slow_implwhen allocating from a fixed-size chunk, before failing the allocation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/oxc_allocator/src/arena/fixed_size/windows.rs | Adds Windows stub for grow_fixed_size_chunk (currently no-op). |
| crates/oxc_allocator/src/arena/fixed_size/macos.rs | Adds macOS stub for grow_fixed_size_chunk (currently no-op). |
| crates/oxc_allocator/src/arena/fixed_size/linux.rs | Adds non-macOS/non-Windows stub for grow_fixed_size_chunk (currently no-op). |
| crates/oxc_allocator/src/arena/alloc_impl.rs | Invokes grow_fixed_size_chunk in fixed-size allocation slow path before failing. |
91be1c5 to
2098199
Compare
8ad4c56 to
a654d30
Compare
2098199 to
01d8cdd
Compare
b6c9c93 to
d5b9c75
Compare
01d8cdd to
cdc43b7
Compare
cdc43b7 to
9e53a66
Compare
d5b9c75 to
f4cbe3d
Compare
9e53a66 to
c08e350
Compare
I need to investigate if this is spurious or not. |
c08e350 to
dd5ead0
Compare
Pushed again, and the "regression" entirely disappeared. I think previous result was just a freak. Codspeed noted that the CI run with the poor perf was run on a machine with a different CPU - assume that was the cause. |
Merge activity
|
Add a private method `Arena::grow_fixed_size_chunk` which attempts to grow a fixed-size `Arena` chunk in place. Similar to #22122, there's multiple implementations for different platforms, but all are currently the same - all return `None` to indicate "could not grow". This is preparatory work for #22124, which adds a Windows-specific implementation.
f4cbe3d to
5086ddc
Compare
dd5ead0 to
e216a84
Compare
…22124) Fixes #19395 - OOM errors in Oxlint JS plugins on Windows. ### The problem Previously, on all platforms each fixed-size (raw transfer-enabled) allocator obtained a large chunk of memory from the `System` allocator (6 GiB in the case of Windows). This is fine on MacOS and most Linux distros, as they use overcommit allocation accounting, so these allocations consume only virtual memory (address space), not actual physical memory. The available address space is enormous (~128 TiB), so it's hard to exhaust it. Physical memory is only consumed when pages of the allocation are actually touched, which the vast majority never are. Windows behaves differently. Allocations obtained from `System` allocator immediately count towards the memory limit. So, on memory-constrained machines, allocating 6 GiB can immediately fail - OOM just from linting the first file. ### This PR [#22088](#22088), #22122, and #22123 introduced platform-specific code for handling allocation and deallocation of fixed-size `Arena`s, which are used in raw transfer/Oxlint JS plugins. This PR switches to a different allocation method on Windows for fixed-size `Arena`s. It switches to using [VirtualAlloc](https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualalloc). This Windows-only API allows reserving address space and committing pages of that reservation as separate operations (similar to `mmap`). It works like this: - Fixed-size allocators now still reserve 6 GiB, but that only consumes address space, and does not count towards max memory limit. - Of that 6 GiB reservation, a 2 GiB "container", aligned on a 4 GiB boundary, will be used as the active part of the memory for storing the AST and other data. - Initially only the last 16 KiB of the container is committed (made into usable memory, with physical pages backing it). - If AST does not fit in the committed region, the region grows downwards, committing further pages to accomodate allocation requests, up to the maximum of 2 GiB. Essentially, this replicates something similar to Linux-style allocation, but on Windows. The only difference is that on Windows it's necessary to explicitly commit pages before touching them, whereas Linux/MacOS commit on demand automatically. ### Implementation details and testing This PR uses the battle-tested [windows-sys](https://crates.io/crates/windows-sys) crate for `VirtualAlloc` and `VirtualFree`. The allocation implementation is written from scratch, but while I was researching to validate that I'd not missed any edge cases, I discovered that it's almost identical to [wasmtime's implementation](https://github.com/bytecodealliance/wasmtime/blob/386a3280dee61f5c4120ce7cde621c1039e383d5/crates/wasmtime/src/runtime/vm/sys/windows/mmap.rs) which uses the same APIs. Comments in the code go over in some detail considerations of Rust's memory model and pointer provenance. As far as I can see, the implementation is fairly watertight. There are tests for the essential functionality and edge cases, and #22121 made these tests run under Miri on Windows. However, I don't have access to a Windows machine to fully test this by running Oxlint on a massive repo. I am hoping members of the community who do can put this through its paces a bit before we merge. ### What's missing In enormous projects, when linting with both JS plugins and `import` plugin, it's possible to end up with a huge number of ASTs in memory simultaneously. With each AST consuming 6 GiB of address space, it's possible to exhaust the entire 128 TiB usable address space, and get OOM on _virtual_ memory. At present we have a workaround to prevent too many fixed-size allocators existing concurrently, so avoiding OOM, but the workaround is a drag on performance. Future work will aim to address this.

Add a private method
Arena::grow_fixed_size_chunkwhich attempts to grow a fixed-sizeArenachunk in place.Similar to #22122, there's multiple implementations for different platforms, but all are currently the same - all return
Noneto indicate "could not grow". This is preparatory work for #22124, which adds a Windows-specific implementation.