Skip to content

ci(miri): run oxc_allocator tests with Miri on Windows#22121

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows
May 14, 2026
Merged

ci(miri): run oxc_allocator tests with Miri on Windows#22121
graphite-app[bot] merged 1 commit into
mainfrom
om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows

Conversation

@overlookmotel

@overlookmotel overlookmotel commented May 5, 2026

Copy link
Copy Markdown
Member

Run the tests for oxc_allocator under Miri on Windows on CI.

Set to run on all PRs which touch oxc_allocator's code, but not any other crates.

This is preparation for Windows-specific changes to fixed-size allocators, which contain complex unsafe code.

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:22
Copilot AI review requested due to automatic review settings May 5, 2026 01:22

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

This PR extends the existing Miri CI workflow to also run oxc_allocator tests under Miri on a GitHub-hosted Windows runner, targeting Windows-specific fixed-size allocator code paths.

Changes:

  • Add a new miri-windows job on windows-latest that runs cargo +nightly miri test for oxc_allocator.
  • Reuse the existing check-changes composite action to decide whether to execute the Windows Miri steps based on oxc_allocator changes.

Comment thread .github/workflows/miri.yml Outdated
Comment thread .github/workflows/miri.yml Outdated
@overlookmotel overlookmotel added C-test Category - Testing. Code is missing test cases, or a PR is adding them A-allocator Area - Allocator labels May 5, 2026
Comment thread .github/workflows/miri.yml Outdated
@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 changed the base branch from main to graphite-base/22121 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 changed the base branch from graphite-base/22121 to om/05-13-fix_allocator_fix_segfault_on_linux_musl_with_fixed-size_allocators May 13, 2026 21:07
@overlookmotel overlookmotel force-pushed the om/05-13-fix_allocator_fix_segfault_on_linux_musl_with_fixed-size_allocators branch from 28567af to d3d2249 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
@overlookmotel overlookmotel requested review from camc314 and Copilot May 14, 2026 18:20

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@camc314 camc314 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.

in future, we could consider making this a separate job on miri.yml (or perhaps use a matrix

@graphite-app

graphite-app Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Merge activity

Run the tests for `oxc_allocator` under Miri on Windows on CI.

Set to run on all PRs which touch `oxc_allocator`'s code, but not any other crates.

This is preparation for Windows-specific changes to fixed-size allocators, which contain complex unsafe code.
@graphite-app graphite-app Bot force-pushed the om/05-13-fix_allocator_fix_segfault_on_linux_musl_with_fixed-size_allocators branch from d3d2249 to 66d77eb Compare May 14, 2026 22:26
@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 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-13-fix_allocator_fix_segfault_on_linux_musl_with_fixed-size_allocators to main May 14, 2026 22:31
@graphite-app graphite-app Bot merged commit f5ed139 into main May 14, 2026
29 checks passed
@graphite-app graphite-app Bot deleted the om/05-05-ci_miri_run_oxc_allocator_tests_with_miri_on_windows branch May 14, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-allocator Area - Allocator C-test Category - Testing. Code is missing test cases, or a PR is adding them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants