Skip to content

cranelift-jit: add bump memory allocator, make configurable#10512

Merged
alexcrichton merged 6 commits intobytecodealliance:mainfrom
Mrmaxmeier:cranelift-jit-pluggable-memory-provider
Apr 9, 2025
Merged

cranelift-jit: add bump memory allocator, make configurable#10512
alexcrichton merged 6 commits intobytecodealliance:mainfrom
Mrmaxmeier:cranelift-jit-pluggable-memory-provider

Conversation

@Mrmaxmeier
Copy link
Copy Markdown
Contributor

@Mrmaxmeier Mrmaxmeier commented Apr 2, 2025

Hey,
this is a stab at #4000 (and #4986) to resolve an issue with x86 relocation range that comes up in multi-threaded JITs.
I've added an option to swap out the memory allocator in cranelift-jit and have added an arena-based option that works with a pre-allocated PROT_NONE region.

I've marked this as draft because I could also see updating the PR to either

  • just add the trait, have users provide their own option when in doubt?
  • update the existing allocator to allow reserving an area up-front?
  • split the changes into multiple commits if helpful

Let me know if this seems reasonable. Thanks! 🙂

Comment thread cranelift/jit/src/memory/arena.rs
Comment thread cranelift/jit/src/memory/arena.rs Outdated
@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label Apr 2, 2025
This adds a new JIT memory allocator which operates on an address range
that is reserved up-front. This is useful in order to side-step issues
where allocation behavior affect relocations, i.e. on x86 where we run
into issues when the system allocator returns memory that is more than
4 GiB apart.

The main drawback of this allocator option is that the memory range has
to be reserved up-front, so allocation will fail if this space is
exhausted.

This commit:
- adds a 'JITMemoryProvider' trait
- moves the previous on-demand allocator to this trait
- adds a new 'Arena' allocator
@Mrmaxmeier Mrmaxmeier force-pushed the cranelift-jit-pluggable-memory-provider branch from f508348 to 0621d5b Compare April 3, 2025 12:17
@Mrmaxmeier Mrmaxmeier marked this pull request as ready for review April 3, 2025 12:17
@Mrmaxmeier Mrmaxmeier requested a review from a team as a code owner April 3, 2025 12:17
@Mrmaxmeier Mrmaxmeier requested review from abrown and removed request for a team April 3, 2025 12:17
Copy link
Copy Markdown
Member

@abrown abrown left a comment

Choose a reason for hiding this comment

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

I think this overall makes sense to me; I didn't look too, too closely at all the arithmetic so it would be useful to have some tests around this (if there aren't any already). Plus, @bjorn3 are you a +1 on this? I believe you may be the one most interested in this. (I checked and apparently there are some other dependents of this so it isn't just @bjorn3).

Comment thread cranelift/jit/src/memory/arena.rs
Comment thread cranelift/jit/src/memory/arena.rs Outdated
@Mrmaxmeier
Copy link
Copy Markdown
Contributor Author

I think this overall makes sense to me; I didn't look too, too closely at all the arithmetic so it would be useful to have some tests around this (if there aren't any already). Plus, @bjorn3 are you a +1 on this? I believe you may be the one most interested in this. (I checked and apparently there are some other dependents of this so it isn't just @bjorn3).

I've added some simple tests and have cleaned up the PR a bit.
Bjorn3's sub-1hr review is just luck of the draw. 🙂

Comment thread cranelift/jit/src/memory/system.rs
Comment thread cranelift/jit/src/lib.rs Outdated
@alexcrichton alexcrichton added this pull request to the merge queue Apr 8, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 8, 2025
@Mrmaxmeier
Copy link
Copy Markdown
Contributor Author

Whoops, sorry. Didn't realize that only a subset of the CI jobs runs on posted PRs ^^

@alexcrichton alexcrichton added this pull request to the merge queue Apr 8, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Apr 8, 2025
* cranelift-jit: add bump memory allocator, make configurable

This adds a new JIT memory allocator which operates on an address range
that is reserved up-front. This is useful in order to side-step issues
where allocation behavior affect relocations, i.e. on x86 where we run
into issues when the system allocator returns memory that is more than
4 GiB apart.

The main drawback of this allocator option is that the memory range has
to be reserved up-front, so allocation will fail if this space is
exhausted.

This commit:
- adds a 'JITMemoryProvider' trait
- moves the previous on-demand allocator to this trait
- adds a new 'Arena' allocator

* ArenaMemoryProvider: add tests, simplify alignment logic

* ArenaMemoryProvider: add asserts

* Move pipeline flush back out of `set_readable_and_executable`

* Fix aarch64 build
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 8, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Apr 9, 2025
Merged via the queue into bytecodealliance:main with commit 56148ad Apr 9, 2025
41 checks passed
@Mrmaxmeier Mrmaxmeier deleted the cranelift-jit-pluggable-memory-provider branch April 9, 2025 08:39
honzasp added a commit to KeyrockEU/wasmtime that referenced this pull request Oct 1, 2025
Send was implemented for JITModule in bytecodealliance#8718, but this was (likely
intentionally) broken in bytecodealliance#10512. This commit implements Send again and
adds a test to guard against any future regressions.
github-merge-queue Bot pushed a commit that referenced this pull request Oct 1, 2025
Send was implemented for JITModule in #8718, but this was (likely
intentionally) broken in #10512. This commit implements Send again and
adds a test to guard against any future regressions.
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
Send was implemented for JITModule in bytecodealliance#8718, but this was (likely
intentionally) broken in bytecodealliance#10512. This commit implements Send again and
adds a test to guard against any future regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants