Skip to content

refactor(allocator): per-platform implementation of freeing fixed-size chunks#22122

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks
May 14, 2026
Merged

refactor(allocator): per-platform implementation of freeing fixed-size chunks#22122
graphite-app[bot] merged 1 commit into
mainfrom
om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks

Conversation

@overlookmotel

@overlookmotel overlookmotel commented May 5, 2026

Copy link
Copy Markdown
Member

Add per-platform implementations of dealloc_fixed_size_arena_chunk, and delegate to them when dropping an Arena which has fixed-size chunks.

Currently, all the implementations are identical - they just deallocate the memory with System allocator. But this prepares the way for #22124, which alters the Windows impl to use VirtualFree instead.

overlookmotel commented May 5, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel self-assigned this May 5, 2026
@overlookmotel overlookmotel marked this pull request as ready for review May 5, 2026 01:25
Copilot AI review requested due to automatic review settings May 5, 2026 01:25
@codspeed-hq

codspeed-hq Bot commented May 5, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks (5086ddc) with main (6d42395)2

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (5086ddc) during the generation of this report, so 6d42395 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copilot AI 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.

Pull request overview

Refactors fixed-size arena chunk deallocation to route through per-platform implementations, in preparation for a future Windows-specific change (VirtualFree) while keeping the allocator drop path consistent with the fixed-size pool’s explicit free logic.

Changes:

  • Adds dealloc_fixed_size_arena_chunk per platform (linux/macos/windows) and re-exports it from arena::fixed_size.
  • Updates the fixed-size allocator pool to call dealloc_fixed_size_arena_chunk when freeing a fixed-size allocator.
  • Refactors arena drop deallocation to delegate fixed-size chunk frees to the platform function.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/oxc_allocator/src/pool/fixed_size.rs Switches fixed-size pool deallocation to the new platform-specific dealloc entrypoint.
crates/oxc_allocator/src/arena/mod.rs Makes arena::fixed_size module accessible within the crate to support the new call sites.
crates/oxc_allocator/src/arena/drop.rs Refactors chunk deallocation, delegating fixed-size frees to arena::fixed_size.
crates/oxc_allocator/src/arena/fixed_size/mod.rs Re-exports dealloc_fixed_size_arena_chunk from the active platform module.
crates/oxc_allocator/src/arena/fixed_size/linux.rs Adds Linux implementation of dealloc_fixed_size_arena_chunk.
crates/oxc_allocator/src/arena/fixed_size/macos.rs Adds macOS implementation of dealloc_fixed_size_arena_chunk.
crates/oxc_allocator/src/arena/fixed_size/windows.rs Adds Windows implementation of dealloc_fixed_size_arena_chunk.

Comment thread crates/oxc_allocator/src/arena/drop.rs
Comment thread crates/oxc_allocator/src/arena/fixed_size/linux.rs Outdated
Comment thread crates/oxc_allocator/src/arena/fixed_size/macos.rs Outdated
Comment thread crates/oxc_allocator/src/arena/fixed_size/windows.rs Outdated
@overlookmotel overlookmotel force-pushed the om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks branch from 8ad4c56 to a654d30 Compare May 5, 2026 02:25
@overlookmotel overlookmotel added C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior A-allocator Area - Allocator labels May 5, 2026
@overlookmotel overlookmotel force-pushed the om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows branch from c66882c to d14a4f2 Compare May 12, 2026 22:57
@overlookmotel overlookmotel force-pushed the om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks branch 2 times, most recently from b6c9c93 to d5b9c75 Compare May 13, 2026 21:07
@overlookmotel overlookmotel force-pushed the om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows branch from d14a4f2 to b710538 Compare May 13, 2026 21:07
@overlookmotel overlookmotel force-pushed the om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks branch from d5b9c75 to f4cbe3d Compare May 14, 2026 18:15
@overlookmotel overlookmotel force-pushed the om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows branch from b710538 to 6fe6d15 Compare May 14, 2026 18:15
@graphite-app

graphite-app Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Merge activity

…e chunks (#22122)

Add per-platform implementations of `dealloc_fixed_size_arena_chunk`, and delegate to them when dropping an `Arena` which has fixed-size chunks.

Currently, all the implementations are identical - they just deallocate the memory with `System` allocator. But this prepares the way for #22124, which alters the Windows impl to use `VirtualFree` instead.
@graphite-app graphite-app Bot force-pushed the om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows branch from 6fe6d15 to f5ed139 Compare May 14, 2026 22:27
@graphite-app graphite-app Bot force-pushed the om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks branch from f4cbe3d to 5086ddc Compare May 14, 2026 22:27
graphite-app Bot pushed a commit that referenced this pull request May 14, 2026
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.
graphite-app Bot pushed a commit that referenced this pull request May 14, 2026
…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.
Base automatically changed from om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows to main May 14, 2026 22:32
@graphite-app graphite-app Bot merged commit 5086ddc into main May 14, 2026
40 checks passed
@graphite-app graphite-app Bot deleted the om/05-03-refactor_allocator_per-platform_implementation_of_freeing_fixed-size_chunks branch May 14, 2026 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-allocator Area - Allocator C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants