Skip to content

refactor(allocator): clean up obsolete bumpalo references#18172

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

refactor(allocator): clean up obsolete bumpalo references#18172
graphite-app[bot] merged 1 commit intomainfrom
refactor/cleanup-bumpalo-references

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented 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

  • cargo check -p oxc_allocator passes
  • cargo test -p oxc_allocator passes
  • Changes are documentation/comment only - no functional changes

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings January 18, 2026 07:54
@github-actions github-actions Bot added A-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic A-cli Area - CLI A-ast-tools Area - AST tools C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels 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 cleans up documentation and comments that reference bumpalo after it was inlined into oxc_allocator (PR #18168). All changes are documentation/comment-only with no functional code modifications.

Changes:

  • Update module documentation from "ported from bumpalo" to "originally derived from bumpalo" for proper attribution
  • Remove obsolete comments about bumpalo version pinning requirements
  • Replace references to "bumpalo" with "arena allocator" or "oxc_allocator" throughout the codebase
  • Remove outdated TODOs about replacing bumpalo with a custom allocator

Reviewed changes

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

Show a summary per file
File Description
tasks/ast_tools/src/generators/raw_transfer.rs Removed reference to Bumpalo in malloc metadata comment
napi/parser/src/raw_transfer.rs Changed "bumpalo" to "the arena allocator" in alignment comment
crates/oxc_semantic/src/node/mod.rs Updated comment from "bumpalo memory arena" to "memory arena"
crates/oxc_parser/src/lib.rs Changed documentation link from bumpalo to oxc_allocator
crates/oxc_data_structures/src/stack/non_empty.rs Changed "bumpalo allocator" to "a bump allocator"
crates/oxc_allocator/src/vec2/raw_vec.rs Updated header from "copied from" to "originally derived from"
crates/oxc_allocator/src/vec2/mod.rs Updated header from "copied from" to "originally derived from"
crates/oxc_allocator/src/tracking.rs Removed obsolete TODO about replacing bumpalo
crates/oxc_allocator/src/pool/fixed_size.rs Removed references to "Bumpalo's ChunkFooter" and "requirements of Bumpalo"
crates/oxc_allocator/src/from_raw_parts.rs Removed extensive warnings about bumpalo version pinning and updated various comments
crates/oxc_allocator/src/bumpalo_alloc.rs Updated module docs from "ported from bumpalo" to "originally derived from bumpalo"
crates/oxc_allocator/src/bump.rs Updated module docs from "ported from bumpalo" to "originally derived from bumpalo"
crates/oxc_allocator/src/allocator.rs Changed inline comment rationale from "delegates to bumpalo" to "it's a small function"
apps/oxlint/src/js_plugins/raw_fs.rs Updated TODO comment to remove reference to replacing bumpalo
apps/oxlint/src/js_plugins/parse.rs Changed "bumpalo" to "the arena allocator" in alignment comment
apps/oxlint/src/js_plugins/external_linter.rs Removed obsolete TODO about replacing bumpalo
ARCHITECTURE.md Changed reference from bumpalo crate to oxc_allocator

💡 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/cleanup-bumpalo-references (e041bb8) with main (2c6966d)

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

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 graphite-app Bot force-pushed the refactor/cleanup-bumpalo-references branch from 0400982 to b553053 Compare January 18, 2026 08:19
@Boshen Boshen marked this pull request as draft January 18, 2026 08:19
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
@Boshen Boshen force-pushed the refactor/cleanup-bumpalo-references branch from b553053 to 8e577de Compare January 18, 2026 08:22
@Boshen Boshen marked this pull request as ready for review January 18, 2026 08:23
@Boshen Boshen force-pushed the refactor/cleanup-bumpalo-references branch from 8e577de to e041bb8 Compare January 18, 2026 08:28
Copy link
Copy Markdown
Member Author

Boshen commented Jan 18, 2026


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 hot fixes, skip the queue and merge this PR next

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.

@graphite-app graphite-app Bot added the 0-merge Merge with Graphite Merge Queue label 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 graphite-app Bot force-pushed the refactor/cleanup-bumpalo-references branch from e041bb8 to 08da7bd Compare January 18, 2026 08:47
@graphite-app graphite-app Bot merged commit 08da7bd into main Jan 18, 2026
22 checks passed
@graphite-app graphite-app Bot deleted the refactor/cleanup-bumpalo-references branch January 18, 2026 08:53
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
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
Follow on after #18172.

That PR made changes to comments to remove references to `bumpalo` from the codebase. Correct a few of those comments which were incorrect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast-tools Area - AST tools A-cli Area - CLI A-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic 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