Skip to content

refactor(allocator): inline bumpalo into oxc_allocator#18168

Merged
graphite-app[bot] merged 1 commit intomainfrom
refactor/inline-bumpalo
Jan 18, 2026
Merged

refactor(allocator): inline bumpalo into oxc_allocator#18168
graphite-app[bot] merged 1 commit intomainfrom
refactor/inline-bumpalo

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented Jan 18, 2026

Summary

  • Port bumpalo v3.19.0 directly into oxc_allocator as internal modules
  • Add bump.rs with the Bump allocator implementation
  • Add bumpalo_alloc.rs with the Alloc trait and related types
  • Update all imports to use internal crate::bump::Bump
  • Remove bumpalo dependency from workspace and crate

This gives us full control over the arena allocator implementation for future optimizations. The ported code is kept exactly as-is from bumpalo with lint allows added to preserve the original implementation.

Test plan

  • cargo test -p oxc_allocator - all 26 unit tests pass
  • cargo clippy -p oxc_allocator --all-features - no warnings

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings January 18, 2026 06:34
@Boshen Boshen requested a review from overlookmotel as a code owner January 18, 2026 06:34
@github-actions github-actions bot added the C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior label Jan 18, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 inlines the bumpalo v3.19.0 bump allocator directly into oxc_allocator to gain full control over the arena allocator implementation for future optimizations. The ported code is kept exactly as-is with lint allows added to preserve the original implementation.

Changes:

  • Adds bump.rs and bumpalo_alloc.rs modules with the complete bump allocator implementation from bumpalo v3.19.0
  • Updates all imports across the crate to use crate::bump::Bump instead of bumpalo::Bump
  • Removes the bumpalo dependency from workspace and crate-level Cargo.toml files

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/oxc_allocator/src/bump.rs Ports the main Bump allocator implementation with ~2067 lines including tests
crates/oxc_allocator/src/bumpalo_alloc.rs Ports the Alloc trait and related memory allocation types (~800 lines)
crates/oxc_allocator/src/lib.rs Declares the new internal modules as pub(crate)
crates/oxc_allocator/src/allocator.rs Updates import from external bumpalo to internal bump module
crates/oxc_allocator/src/alloc.rs Updates import and uses qualified method calls to avoid ambiguity
crates/oxc_allocator/src/allocator_api2.rs Updates comments and uses qualified method calls for clarity
crates/oxc_allocator/src/vec.rs Updates import to use internal bump module
crates/oxc_allocator/src/tracking.rs Updates import to use internal bump module
crates/oxc_allocator/src/hash_map.rs Updates import to use internal bump module
crates/oxc_allocator/src/hash_set.rs Updates import to use internal bump module
crates/oxc_allocator/src/from_raw_parts.rs Updates import and replaces Bump::<1>::with_min_align() with Bump::new()
crates/oxc_allocator/src/vec2/raw_vec.rs Updates test import to use internal bump module
crates/oxc_allocator/Cargo.toml Removes bumpalo dependency
Cargo.toml Removes bumpalo dependency with explanatory comment
Cargo.lock Updates dependency tree (bumpalo still present for wasm-bindgen)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jan 18, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing refactor/inline-bumpalo (5e07770) with main (38e4b53)

Summary

✅ 42 untouched benchmarks
⏩ 3 skipped benchmarks1

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.

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
Copy link
Copy Markdown
Member Author

Boshen commented Jan 18, 2026

Merge activity

## Summary

- Port bumpalo v3.19.0 directly into `oxc_allocator` as internal modules
- Add `bump.rs` with the Bump allocator implementation
- Add `bumpalo_alloc.rs` with the Alloc trait and related types
- Update all imports to use internal `crate::bump::Bump`
- Remove bumpalo dependency from workspace and crate

This gives us full control over the arena allocator implementation for future optimizations. The ported code is kept exactly as-is from bumpalo with lint allows added to preserve the original implementation.

## Test plan

- [x] `cargo test -p oxc_allocator` - all 26 unit tests pass
- [x] `cargo clippy -p oxc_allocator --all-features` - no warnings

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the refactor/inline-bumpalo branch from 5e07770 to 16d5906 Compare January 18, 2026 07:21
@graphite-app graphite-app bot merged commit 16d5906 into main Jan 18, 2026
23 checks passed
@graphite-app graphite-app bot deleted the refactor/inline-bumpalo branch January 18, 2026 07:27
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
graphite-app bot pushed a commit that referenced this pull request Jan 18, 2026
## Summary

Now that bumpalo has been inlined into oxc_allocator (#18168), this PR cleans up comments and documentation that still reference bumpalo as an external dependency.

- Update module docs in `bump.rs` and `bumpalo_alloc.rs` to say "originally derived from" instead of "ported from"
- Remove obsolete comments about bumpalo version pinning in `from_raw_parts.rs`
- Update inline comments in `allocator.rs` from "delegates to bumpalo" to "it's a small function"
- Remove outdated TODO about replacing bumpalo in `tracking.rs`
- Update `ARCHITECTURE.md` and parser docs to reference `oxc_allocator` instead of bumpalo
- Clean up bumpalo references in other crates (`oxc_semantic`, `oxc_data_structures`, `apps/oxlint`, `napi/parser`, `tasks/ast_tools`)

## Test plan

- [x] `cargo check -p oxc_allocator` passes
- [x] `cargo test -p oxc_allocator` passes
- [x] Changes are documentation/comment only - no functional changes

🤖 Generated with [Claude Code](https://claude.ai/code)
graphite-app bot pushed a commit that referenced this pull request Jan 18, 2026
## Summary

Now that bumpalo has been inlined into oxc_allocator (#18168), this PR cleans up comments and documentation that still reference bumpalo as an external dependency.

- Update module docs in `bump.rs` and `bumpalo_alloc.rs` to say "originally derived from" instead of "ported from"
- Remove obsolete comments about bumpalo version pinning in `from_raw_parts.rs`
- Update inline comments in `allocator.rs` from "delegates to bumpalo" to "it's a small function"
- Remove outdated TODO about replacing bumpalo in `tracking.rs`
- Update `ARCHITECTURE.md` and parser docs to reference `oxc_allocator` instead of bumpalo
- Clean up bumpalo references in other crates (`oxc_semantic`, `oxc_data_structures`, `apps/oxlint`, `napi/parser`, `tasks/ast_tools`)

## Test plan

- [x] `cargo check -p oxc_allocator` passes
- [x] `cargo test -p oxc_allocator` passes
- [x] Changes are documentation/comment only - no functional changes

🤖 Generated with [Claude Code](https://claude.ai/code)
overlookmotel added a commit that referenced this pull request Apr 2, 2026
#18168 copied `bumpalo`'s code for `Bump` into `oxc_allocator` crate.
#18172, #18181, and #18234 made a few improvements to the
implementation.

However, #18168 removed various things, and altered others. Notably, it
removed the`MIN_ALIGN` generic param, which we definitely want to keep.

I'm going to be working on the allocator, and would like to start with a
clean slate, beginning with the "known good" implementation of
`bumpalo`.

This PR removes the previous modified `bumpalo` implementation, and
copies latest version of `bumpalo` from main branch into the repo. It
then makes the absolute minimum changes necessary just to make CI pass.

Note: Unlike #18168, this PR keeps all the doctests for `Bump`, using a
trick of adding the crate to it's own `dev-dependencies` with `testing`
feature enabled, and exposing `Bump` only with that feature.

This PR is broken into multiple commits, so we have an exact "trail of
breadcrumbs" of how we've diverged from `bumpalo`. I intend to manually
merge this PR, so that record remains on Github (instead of Graphite
squashing all the commits).

Later PRs in this stack reapply the changes made in #18172, #18181, and
#18234 on top of the fresh base implementation. This PR is a small perf
regression, but that will be won back once those other changes are
reapplied.
graphite-app bot pushed a commit that referenced this pull request Apr 2, 2026
…set` (#20964)

Partial revert of #18168.

#18168 made a small change to `get_current_chunk_footer_field_offset` (used by `Allocator::from_raw_parts`).

That change was correct in the context of the other changes made to `Bump` in that PR, but now that #20963 has rolled back most of those changes, this change needs to be reverted too as it's a potential perf regression with current version of `Bump`.
graphite-app bot pushed a commit that referenced this pull request Apr 2, 2026
…ment (#20965)

Partial revert of #18168.

`mod bumpalo_alloc;` does not need to be prefixed with `pub(crate)`, as it's defined in crate root. Remove `pub(crate)`, just because it's a bit confusing one that one module has the prefix and none of the others do.
graphite-app bot pushed a commit that referenced this pull request Apr 2, 2026
Follow-on after #18168.

With the removal of `bumpalo` dependency, the examples in doc comments in `vec2::Vec` would no longer compile. That PR marked them all as `text` to prevent them running as doctests. Change them to `ignore` instead. That's preferable as it at least makes sure they're valid Rust syntax.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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