From 08da7bd29ad277d108366b1310e76a674842b584 Mon Sep 17 00:00:00 2001 From: Boshen Date: Sun, 18 Jan 2026 08:47:25 +0000 Subject: [PATCH] refactor(allocator): clean up obsolete bumpalo references (#18172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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) --- ARCHITECTURE.md | 2 +- apps/oxlint/src/js_plugins/external_linter.rs | 3 +-- apps/oxlint/src/js_plugins/parse.rs | 2 +- apps/oxlint/src/js_plugins/raw_fs.rs | 6 ++--- crates/oxc_allocator/src/allocator.rs | 10 ++++----- crates/oxc_allocator/src/bump.rs | 6 ++--- crates/oxc_allocator/src/bumpalo_alloc.rs | 4 ++-- crates/oxc_allocator/src/from_raw_parts.rs | 22 +++++-------------- crates/oxc_allocator/src/pool/fixed_size.rs | 6 ++--- crates/oxc_allocator/src/tracking.rs | 3 +-- crates/oxc_allocator/src/vec2/mod.rs | 3 ++- crates/oxc_allocator/src/vec2/raw_vec.rs | 3 ++- .../src/stack/non_empty.rs | 2 +- crates/oxc_parser/src/lib.rs | 2 +- crates/oxc_semantic/src/node/mod.rs | 2 +- napi/parser/src/raw_transfer.rs | 2 +- .../ast_tools/src/generators/raw_transfer.rs | 2 +- 17 files changed, 35 insertions(+), 45 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index aecbdb0541..7aab6e592d 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -227,7 +227,7 @@ Source Text → Arena Allocator → AST Nodes → Visitors → Results #### Parser Performance Implementation -- AST is allocated in a memory arena ([bumpalo](https://crates.io/crates/bumpalo)) for fast AST memory allocation and deallocation +- AST is allocated in a memory arena (oxc_allocator) for fast AST memory allocation and deallocation - Short strings are inlined by [CompactString](https://crates.io/crates/compact_str) - No other heap allocations are done except the above two - Scope binding, symbol resolution and some syntax errors are not done in the parser, they are delegated to the semantic analyzer diff --git a/apps/oxlint/src/js_plugins/external_linter.rs b/apps/oxlint/src/js_plugins/external_linter.rs index cdea682cac..16bfe3c0e1 100644 --- a/apps/oxlint/src/js_plugins/external_linter.rs +++ b/apps/oxlint/src/js_plugins/external_linter.rs @@ -147,8 +147,7 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { // `AllocatorPool::new_fixed_size`, so all `Allocator`s are created via `FixedSizeAllocator`. // This is somewhat sketchy, as we don't have a type-level guarantee of this invariant, // but it does hold at present. - // When we replace `bumpalo` with a custom allocator, we can close this soundness hole. - // TODO: Do that. + // TODO: Close this soundness hole with type-level guarantees. let (buffer_id, buffer) = unsafe { get_buffer(allocator) }; // Send data to JS diff --git a/apps/oxlint/src/js_plugins/parse.rs b/apps/oxlint/src/js_plugins/parse.rs index f8d75d2bfe..36cc58c04c 100644 --- a/apps/oxlint/src/js_plugins/parse.rs +++ b/apps/oxlint/src/js_plugins/parse.rs @@ -118,7 +118,7 @@ unsafe fn parse_raw_impl( // Get offsets and size of data region to be managed by arena allocator. // Leave space for source before it, and space for metadata after it. // Metadata actually only takes 5 bytes, but round everything up to multiple of 16, - // as `bumpalo` requires that alignment. + // as the arena allocator requires that alignment. const RAW_METADATA_SIZE: usize = size_of::(); const { assert!(RAW_METADATA_SIZE >= BUMP_ALIGN); diff --git a/apps/oxlint/src/js_plugins/raw_fs.rs b/apps/oxlint/src/js_plugins/raw_fs.rs index e19887a811..e41fd83072 100644 --- a/apps/oxlint/src/js_plugins/raw_fs.rs +++ b/apps/oxlint/src/js_plugins/raw_fs.rs @@ -17,9 +17,9 @@ use oxc_linter::RuntimeFileSystem; /// Must only be used in conjunction with `AllocatorPool` created with `new_fixed_size`, /// which wraps `Allocator`s with a custom `Drop` impl, which makes `read_to_arena_str` safe. /// -/// This is a temporary solution. When we replace `bumpalo` with our own allocator, all strings -/// will be written at start of the arena, so then `OsFileSystem` will work fine, and we can -/// remove `RawTransferFileSystem`. TODO: Do that! +/// This is a temporary solution. All strings should be written at start of the arena, +/// so then `OsFileSystem` would work fine, and we can remove `RawTransferFileSystem`. +/// TODO: Do that! pub struct RawTransferFileSystem; impl RuntimeFileSystem for RawTransferFileSystem { diff --git a/crates/oxc_allocator/src/allocator.rs b/crates/oxc_allocator/src/allocator.rs index 4ad637c08c..ceb4b83bc0 100644 --- a/crates/oxc_allocator/src/allocator.rs +++ b/crates/oxc_allocator/src/allocator.rs @@ -253,7 +253,7 @@ impl Allocator { /// [`Vec::new_in`]: crate::Vec::new_in /// [`HashMap::new_in`]: crate::HashMap::new_in // - // `#[inline(always)]` because just delegates to `bumpalo` method + // `#[inline(always)]` because it's a small function #[expect(clippy::inline_always)] #[inline(always)] pub fn new() -> Self { @@ -268,7 +268,7 @@ impl Allocator { /// /// See [`Allocator`] docs for more information on efficient use of [`Allocator`]. // - // `#[inline(always)]` because just delegates to `bumpalo` method + // `#[inline(always)]` because it's a small function #[expect(clippy::inline_always)] #[inline(always)] pub fn with_capacity(capacity: usize) -> Self { @@ -506,7 +506,7 @@ impl Allocator { /// } /// ``` // - // `#[inline(always)]` because it just delegates to `bumpalo` + // `#[inline(always)]` because it's a small function #[expect(clippy::inline_always)] #[inline(always)] pub fn reset(&mut self) { @@ -536,7 +536,7 @@ impl Allocator { /// /// [`used_bytes`]: Allocator::used_bytes // - // `#[inline(always)]` because it just delegates to `bumpalo` + // `#[inline(always)]` because it's a small function #[expect(clippy::inline_always)] #[inline(always)] pub fn capacity(&self) -> usize { @@ -616,7 +616,7 @@ impl Allocator { /// Get inner [`Bump`]. /// /// This method is not public. We don't want to expose `Bump` to user. - /// The fact that we're using `bumpalo` is an internal implementation detail. + /// The inner `Bump` is an internal implementation detail. // // `#[inline(always)]` because it's a no-op #[expect(clippy::inline_always)] diff --git a/crates/oxc_allocator/src/bump.rs b/crates/oxc_allocator/src/bump.rs index 7416bc07c0..55ff5ec5ce 100644 --- a/crates/oxc_allocator/src/bump.rs +++ b/crates/oxc_allocator/src/bump.rs @@ -1,6 +1,6 @@ -//! Bumpalo arena allocator, ported from +//! Arena allocator. //! -//! This module is ported exactly from bumpalo v3.19.0 without modifications. +//! This module was originally derived from bumpalo v3.19.0. #![allow( clippy::borrow_as_ptr, @@ -40,7 +40,7 @@ unsafe_op_in_unsafe_fn )] -//! Bumpalo arena allocator, ported from +//! Arena allocator. #[doc(hidden)] pub extern crate alloc as core_alloc; diff --git a/crates/oxc_allocator/src/bumpalo_alloc.rs b/crates/oxc_allocator/src/bumpalo_alloc.rs index b7e8199a67..4e12c32fe6 100644 --- a/crates/oxc_allocator/src/bumpalo_alloc.rs +++ b/crates/oxc_allocator/src/bumpalo_alloc.rs @@ -8,9 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -//! Memory allocation APIs, ported from bumpalo. +//! Memory allocation APIs. //! -//! This module is ported exactly from bumpalo without modifications. +//! This module was originally derived from bumpalo and Rust's allocator APIs. #![allow( clippy::collapsible_if, diff --git a/crates/oxc_allocator/src/from_raw_parts.rs b/crates/oxc_allocator/src/from_raw_parts.rs index 6709ee9fac..9bbf934244 100644 --- a/crates/oxc_allocator/src/from_raw_parts.rs +++ b/crates/oxc_allocator/src/from_raw_parts.rs @@ -17,7 +17,7 @@ use std::{ use crate::{Allocator, bump::Bump}; -/// Minimum alignment for allocator chunks. This is hard-coded on `bumpalo`. +/// Minimum alignment for allocator chunks. const MIN_ALIGN: usize = 16; const CHUNK_FOOTER_SIZE: usize = size_of::(); @@ -35,12 +35,8 @@ impl Allocator { /// Construct a static-sized [`Allocator`] from an existing memory allocation. /// - /// **IMPORTANT: WE MUST NOT CHANGE THE VERSION OF BUMPALO DEPENDENCY**. - /// - /// This code only remains sound as long as the code in version of `bumpalo` we're using matches - /// the duplicate of `bumpalo`'s internals contained in this file. - /// - /// `bumpalo` is pinned to version `=3.19.0` in `Cargo.toml`. + /// This code relies on specific internal layout of the arena allocator. + /// Changes to `Bump` internals may break this code. /// /// The [`Allocator`] which is returned takes ownership of the memory allocation, /// and the allocation will be freed if the `Allocator` is dropped. @@ -48,9 +44,6 @@ impl Allocator { /// /// The [`Allocator`] returned by this function cannot grow. /// - /// This hack is all very inadvisable! - /// Only implemented as a temporary stopgap until we replace `bumpalo` with our own allocator. - /// /// # SAFETY /// /// * `ptr` must be aligned on [`RAW_MIN_ALIGN`]. @@ -61,7 +54,7 @@ impl Allocator { /// /// # Panics /// - /// Panics if cannot determine layout of Bumpalo's `Bump` type, or on a big endian system. + /// Panics if cannot determine layout of `Bump` type, or on a big endian system. /// /// [`RAW_MIN_ALIGN`]: Self::RAW_MIN_ALIGN /// [`RAW_MIN_SIZE`]: Self::RAW_MIN_SIZE @@ -128,7 +121,7 @@ impl Allocator { /// /// Returns a pointer to the start of an uninitialized section of `bytes` bytes. /// - /// Note: [`alloc_layout`] allocates at *end* of the current chunk, because `bumpalo` bumps downwards, + /// Note: [`alloc_layout`] allocates at *end* of the current chunk, because the arena allocator bumps downwards, /// hence the need for this method, to allocate at *start* of current chunk. /// /// This method is dangerous, and should not ordinarily be used. @@ -343,10 +336,7 @@ impl Allocator { /// Allocator chunk footer. /// -/// Copied exactly from `bumpalo` v3.19.0. -/// -/// This type is not exposed by `bumpalo` crate, but the type is `#[repr(C)]`, so we can rely on our -/// duplicate here having the same layout, as long as we don't change the version of `bumpalo` we use. +/// This type must match the layout of `ChunkFooter` in `bump.rs`. #[repr(C)] #[derive(Debug)] struct ChunkFooter { diff --git a/crates/oxc_allocator/src/pool/fixed_size.rs b/crates/oxc_allocator/src/pool/fixed_size.rs index ea39a0b186..a70f524593 100644 --- a/crates/oxc_allocator/src/pool/fixed_size.rs +++ b/crates/oxc_allocator/src/pool/fixed_size.rs @@ -331,7 +331,7 @@ const ALLOC_LAYOUT: Layout = match Layout::from_size_align(ALLOC_SIZE, ALLOC_ALI /// /// ALLOCATOR /// <-----------------------------------------> `Allocator` chunk (`CHUNK_SIZE` bytes) -/// <----> Bumpalo's `ChunkFooter` (aligned on 16) +/// <----> `ChunkFooter` (aligned on 16) /// <-----------------------------------> `Allocator` chunk data storage (for AST) /// /// METADATA @@ -345,8 +345,8 @@ const ALLOC_LAYOUT: Layout = match Layout::from_size_align(ALLOC_SIZE, ALLOC_ALI /// Note that the buffer sent to JS includes both the `Allocator` chunk, and `RawTransferMetadata`, /// but does NOT include `FixedSizeAllocatorMetadata`. /// -/// The end of the region used for `Allocator` chunk must be aligned on `Allocator::RAW_MIN_ALIGN` (16), -/// due to the requirements of Bumpalo. We manage that by: +/// The end of the region used for `Allocator` chunk must be aligned on `Allocator::RAW_MIN_ALIGN` (16). +/// We manage that by: /// * `BLOCK_SIZE` is a multiple of 16. /// * `RawTransferMetadata` is 16 bytes. /// * Size of `FixedSizeAllocatorMetadata` is rounded up to a multiple of 16. diff --git a/crates/oxc_allocator/src/tracking.rs b/crates/oxc_allocator/src/tracking.rs index 2906f62a6f..5a1c470c46 100644 --- a/crates/oxc_allocator/src/tracking.rs +++ b/crates/oxc_allocator/src/tracking.rs @@ -13,8 +13,7 @@ //! The 2nd cargo feature `disable_track_allocations` is to ensure that compiling with `--all-features` //! will not load this module. //! -//! As soon as we replace `bumpalo` with our own arena allocator, we'll remove the hack from `get_stats_ref`, -//! and make this sound. +//! TODO: Remove the hack from `get_stats_ref` and make this sound. use std::{cell::Cell, ptr}; diff --git a/crates/oxc_allocator/src/vec2/mod.rs b/crates/oxc_allocator/src/vec2/mod.rs index 2f481b4112..4135d619f4 100644 --- a/crates/oxc_allocator/src/vec2/mod.rs +++ b/crates/oxc_allocator/src/vec2/mod.rs @@ -1,4 +1,5 @@ -// This file is copied from the [Bumpalo's Vec](https://github.com/fitzgen/bumpalo/blob/1d2fbea9e3d0c2be56367b9ad5382ff33852a188/src/collections/vec.rs) +// This file was originally derived from Bumpalo's Vec: +// https://github.com/fitzgen/bumpalo/blob/1d2fbea9e3d0c2be56367b9ad5382ff33852a188/src/collections/vec.rs // Copyright 2014 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at diff --git a/crates/oxc_allocator/src/vec2/raw_vec.rs b/crates/oxc_allocator/src/vec2/raw_vec.rs index 95b183f016..0e2dc7e9cb 100644 --- a/crates/oxc_allocator/src/vec2/raw_vec.rs +++ b/crates/oxc_allocator/src/vec2/raw_vec.rs @@ -1,4 +1,5 @@ -// This file is copied from the [Bumpalo's Vec](https://github.com/fitzgen/bumpalo/blob/1d2fbea9e3d0c2be56367b9ad5382ff33852a188/src/collections/raw_vec.rs) +// This file was originally derived from Bumpalo's Vec: +// https://github.com/fitzgen/bumpalo/blob/1d2fbea9e3d0c2be56367b9ad5382ff33852a188/src/collections/raw_vec.rs // Copyright 2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at diff --git a/crates/oxc_data_structures/src/stack/non_empty.rs b/crates/oxc_data_structures/src/stack/non_empty.rs index 377d4994cb..8366e6214c 100644 --- a/crates/oxc_data_structures/src/stack/non_empty.rs +++ b/crates/oxc_data_structures/src/stack/non_empty.rs @@ -58,7 +58,7 @@ use super::{StackCapacity, StackCommon}; /// uses 1 less register, but disadvantage that [`last`] and [`last_mut`] are 2 instructions, not 1. /// /// -/// 2. Stack could grow downwards, like `bumpalo` allocator does. This would probably make [`pop`] use +/// 2. Stack could grow downwards, like a bump allocator does. This would probably make [`pop`] use /// 1 less register, but at the cost that: (a) the stack can never grow in place, which would incur /// more memory copies when the stack grows, and (b) [`as_slice`] would have the entries in /// reverse order. diff --git a/crates/oxc_parser/src/lib.rs b/crates/oxc_parser/src/lib.rs index 8da6c8fe10..c3ccfcbd79 100644 --- a/crates/oxc_parser/src/lib.rs +++ b/crates/oxc_parser/src/lib.rs @@ -22,7 +22,7 @@ //! # Performance //! //! The following optimization techniques are used: -//! * AST is allocated in a memory arena ([bumpalo](https://docs.rs/bumpalo)) for fast AST drop +//! * AST is allocated in a memory arena ([oxc_allocator](https://docs.rs/oxc_allocator)) for fast AST drop //! * [`oxc_span::Span`] offsets uses `u32` instead of `usize` //! * Scope binding, symbol resolution and complicated syntax errors are not done in the parser, //! they are delegated to the [semantic analyzer](https://docs.rs/oxc_semantic) diff --git a/crates/oxc_semantic/src/node/mod.rs b/crates/oxc_semantic/src/node/mod.rs index ce3993b617..9ffb982ad7 100644 --- a/crates/oxc_semantic/src/node/mod.rs +++ b/crates/oxc_semantic/src/node/mod.rs @@ -11,7 +11,7 @@ use oxc_syntax::{node::NodeId, scope::ScopeId}; #[derive(Debug, Clone, Copy)] pub struct AstNode<'a> { id: NodeId, - /// A pointer to the ast node, which resides in the `bumpalo` memory arena. + /// A pointer to the ast node, which resides in the memory arena. kind: AstKind<'a>, /// Associated Scope (initialized by binding) diff --git a/napi/parser/src/raw_transfer.rs b/napi/parser/src/raw_transfer.rs index 6b10634693..aff87c3810 100644 --- a/napi/parser/src/raw_transfer.rs +++ b/napi/parser/src/raw_transfer.rs @@ -183,7 +183,7 @@ unsafe fn parse_raw_impl( // Get offsets and size of data region to be managed by arena allocator. // Leave space for source before it, and space for metadata after it. // Metadata actually only takes 5 bytes, but round everything up to multiple of 16, - // as `bumpalo` requires that alignment. + // as the arena allocator requires that alignment. const RAW_METADATA_SIZE: usize = size_of::(); const { assert!(RAW_METADATA_SIZE >= BUMP_ALIGN); diff --git a/tasks/ast_tools/src/generators/raw_transfer.rs b/tasks/ast_tools/src/generators/raw_transfer.rs index 4bde652111..50d9de48a5 100644 --- a/tasks/ast_tools/src/generators/raw_transfer.rs +++ b/tasks/ast_tools/src/generators/raw_transfer.rs @@ -49,7 +49,7 @@ const ALLOCATOR_CHUNK_END_ALIGN: u32 = 16; /// This size includes metadata stored after the `Allocator` chunk which contains AST data. /// /// Must be a multiple of [`ALLOCATOR_CHUNK_END_ALIGN`]. -/// 16 bytes less than 2 GiB, to allow 16 bytes for `malloc` metadata (like Bumpalo does). +/// 16 bytes less than 2 GiB, to allow 16 bytes for `malloc` metadata. const BLOCK_SIZE: u32 = (1 << 31) - MALLOC_RESERVED_SIZE; // 2 GiB - 16 bytes const _: () = assert!(BLOCK_SIZE.is_multiple_of(ALLOCATOR_CHUNK_END_ALIGN));