mirror of
https://github.com/oxc-project/oxc.git
synced 2026-09-14 19:36:11 +08:00
refactor(allocator): clean up obsolete bumpalo references (#18172)
## 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)
This commit is contained in:
+1
-1
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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::<RawTransferMetadata>();
|
||||
const {
|
||||
assert!(RAW_METADATA_SIZE >= BUMP_ALIGN);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Bumpalo arena allocator, ported from <https://github.com/fitzgen/bumpalo>
|
||||
//! 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 <https://github.com/fitzgen/bumpalo>
|
||||
//! Arena allocator.
|
||||
|
||||
#[doc(hidden)]
|
||||
pub extern crate alloc as core_alloc;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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::<ChunkFooter>();
|
||||
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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};
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -58,7 +58,7 @@ use super::{StackCapacity, StackCommon};
|
||||
/// uses 1 less register, but disadvantage that [`last`] and [`last_mut`] are 2 instructions, not 1.
|
||||
/// <https://godbolt.org/z/xnx7YP5de>
|
||||
///
|
||||
/// 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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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::<RawTransferMetadata>();
|
||||
const {
|
||||
assert!(RAW_METADATA_SIZE >= BUMP_ALIGN);
|
||||
|
||||
@@ -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));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user