Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(allocator)!: replace bumpalo with bump-scope #6668

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e8206c4
refactor!: replaced dependency `bumpalo` with `bump-scope`
bluurryy Oct 18, 2024
d3e64cb
refactor: update `bump-scope` to version `0.10.4`
bluurryy Oct 18, 2024
27d43bb
refactor: create a wrapper type for bump allocated strings
bluurryy Oct 19, 2024
a8c4b08
refactor(allocator): implement `Vec` using `bump_scope::BumpVec`
bluurryy Oct 19, 2024
3575bef
fix(allocator): serde support for string
bluurryy Oct 19, 2024
15d2be8
doc(allocator): replace `bumpalo` with `bump-scope`
bluurryy Oct 19, 2024
425ee29
feat(allocator): complete `Vec` abstraction
bluurryy Oct 19, 2024
0497f13
refactor(allocator): use the `from_iter_in` provided by `bump-scope`
bluurryy Oct 19, 2024
315153e
chore: merge branch 'main' into replace-bumpalo-with-bump-scope
bluurryy Oct 19, 2024
e2001d5
chore: actually merge main
bluurryy Oct 19, 2024
9f3dfd0
[autofix.ci] apply automated fixes
autofix-ci[bot] Oct 20, 2024
f76c21e
feat(allocator): `IntoIter` newtype, `pub mod vec`, more docs
bluurryy Oct 23, 2024
f8208dc
feat(allocator): add vec methods, fix clippy and docs
bluurryy Oct 24, 2024
c069844
fix(allocator): implement serialize without derive for string
bluurryy Oct 24, 2024
1f5e4f8
fix(allocator): stacked borrow violation due to missing `as(_mut)_ptr…
bluurryy Oct 24, 2024
06ad007
merge: branch 'main'
bluurryy Oct 24, 2024
91262a7
fix: method rename due to merge
bluurryy Oct 24, 2024
546a7cd
refactor(allocator): `bump-scope` generic arguments as constants
bluurryy Oct 24, 2024
b942f71
feat(allocator): removed `Deref` and `From` implementations
bluurryy Oct 24, 2024
5e96313
feat(allocator): use generic parameter for other `*Impl` types
bluurryy Oct 24, 2024
2e3d33b
feat(allocator): `inline(always)` what wasn't already
bluurryy Oct 24, 2024
0d40d3c
refactor(allocator): `DrainImpl` alias
bluurryy Oct 24, 2024
5fd3622
feat(allocator): rename `into_str` back to `into_bump_str` to keep di…
bluurryy Oct 24, 2024
864c53f
feat(allocator): switch back to bumping upwards
bluurryy Oct 25, 2024
93103be
merge: branch 'main'
bluurryy Oct 25, 2024
badb66c
fix: UB when splicing by updating `bump-scope` to `0.10.6`
bluurryy Oct 26, 2024
fe9db0f
feat(allocator): added `from_iter_exact_in` and use it for `CloneIn f…
bluurryy Oct 26, 2024
f7e47f4
refactor!(allocator): implement `Vec` using `allocator_api2::vec::Vec…
bluurryy Oct 26, 2024
3f81ffb
merge: branch 'main'
bluurryy Oct 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ base64 = "0.22.1"
base64-simd = "0.8"
bitflags = "2.6.0"
bpaf = "0.9.15"
bumpalo = "3.16.0"
bump-scope = "0.10.6"
cfg-if = "1.0.0"
compact_str = "0.8.0"
console = "0.15.8"
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_allocator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ doctest = false

[dependencies]
allocator-api2 = { workspace = true }
bumpalo = { workspace = true, features = ["allocator-api2", "collections"] }
bump-scope = { workspace = true }

serde = { workspace = true, optional = true }

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_allocator/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl<'alloc, T> Box<'alloc, T> {
/// let in_arena: Box<i32> = Box::new_in(5, &arena);
/// ```
pub fn new_in(value: T, allocator: &Allocator) -> Self {
Self(NonNull::from(allocator.alloc(value)), PhantomData)
Self(allocator.alloc(value).into_raw(), PhantomData)
}

/// Create a fake [`Box`] with a dangling pointer.
Expand Down Expand Up @@ -176,7 +176,7 @@ mod test {
let allocator = Allocator::default();
let mut b = Box::new_in("x", &allocator);
let b = &mut *b;
*b = allocator.alloc("v");
*b = allocator.bump.alloc("v").into_mut();
assert_eq!(*b, "v");
}

Expand Down
20 changes: 16 additions & 4 deletions crates/oxc_allocator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,26 @@ use std::{
ops::{Deref, DerefMut},
};

pub use bumpalo::collections::String;
use bumpalo::Bump;

mod address;
mod boxed;
mod clone_in;
mod convert;
mod string;
mod vec;

pub use address::{Address, GetAddress};
use allocator_api2::alloc::Global;
pub use boxed::Box;
pub use clone_in::CloneIn;
pub use convert::{FromIn, IntoIn};
pub use string::String;
pub use vec::Vec;

const BUMP_UPWARDS: bool = false;
const MINIMUM_ALIGNMENT: usize = 1;

type Bump = bump_scope::Bump<Global, MINIMUM_ALIGNMENT, BUMP_UPWARDS>;

/// A bump-allocated memory arena based on [bumpalo].
///
/// ## No `Drop`s
Expand All @@ -71,6 +76,13 @@ pub struct Allocator {
bump: Bump,
}

impl Allocator {
/// Allocate a `str`.
pub fn alloc_str(&self, src: &str) -> &mut str {
self.bump.alloc_str(src).into_mut()
}
}

impl From<Bump> for Allocator {
fn from(bump: Bump) -> Self {
Self { bump }
Expand All @@ -95,7 +107,7 @@ impl DerefMut for Allocator {
mod test {
use std::ops::Deref;

use bumpalo::Bump;
use bump_scope::Bump;

use crate::Allocator;

Expand Down
156 changes: 156 additions & 0 deletions crates/oxc_allocator/src/string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
use std::{
mem,
ops::{Deref, DerefMut},
ptr, str,
};

use crate::{Allocator, Vec};

/// A bump-allocated string.
pub struct String<'a> {
vec: Vec<'a, u8>,
}

impl<'a> String<'a> {
/// Constructs a new empty `String`.
#[inline]
pub fn new_in(allocator: &'a Allocator) -> Self {
Self { vec: Vec::new_in(allocator) }
}

/// Constructs a `String` from a `&str`.
#[inline]
pub fn from_str_in(s: &str, allocator: &'a Allocator) -> Self {
// code taken from `bumpalo::collections::String::from_str_in`
let len = s.len();
let mut t = String::with_capacity_in(len, allocator);
// SAFETY:
// * `src` is valid for reads of `s.len()` bytes by virtue of being an allocated `&str`.
// * `dst` is valid for writes of `s.len()` bytes as `String::with_capacity_in(s.len(), bump)`
// above guarantees that.
// * Alignment is not relevant as `u8` has no alignment requirements.
// * Source and destination ranges cannot overlap as we just reserved the destination
// range from the bump.
unsafe { ptr::copy_nonoverlapping(s.as_ptr(), t.vec.as_mut_ptr(), len) };
// SAFETY: We reserved sufficent capacity for the string above.

Check warning on line 35 in crates/oxc_allocator/src/string.rs

View workflow job for this annotation

GitHub Actions / Spell Check

"sufficent" should be "sufficient".
// The elements at `0..len` were initialized by `copy_nonoverlapping` above.
unsafe { t.vec.set_len(len) };
t
}

/// Constructs a new empty `String` with the specified capacity.
#[inline]
pub fn with_capacity_in(capacity: usize, allocator: &'a Allocator) -> Self {
Self { vec: Vec::with_capacity_in(capacity, allocator) }
}

/// Converts a `String` into a `&str`.
#[inline]
pub fn into_bump_str(self) -> &'a str {
#![allow(
clippy::undocumented_unsafe_blocks,
clippy::missing_transmute_annotations,
clippy::forget_non_drop
)]
// code taken from `bumpalo::collections::String::into_bump_str`
let s = unsafe {
let s = self.as_str();
mem::transmute(s)
};
mem::forget(self);
s
}

/// Appends a given string slice to the end of this string.
#[inline]
pub fn push_str(&mut self, string: &str) {
self.vec.extend_from_slice_copy(string.as_bytes());
}

/// Appends a given `char` to the end of this string.
#[inline]
pub fn push(&mut self, ch: char) {
match ch.len_utf8() {
1 => self.vec.push(ch as u8),
_ => self.vec.extend_from_slice(ch.encode_utf8(&mut [0; 4]).as_bytes()),
}
}

/// Extracts a string slice containing the entire `String`.
#[inline]
pub fn as_str(&self) -> &str {
self
}
}

impl Deref for String<'_> {
type Target = str;

#[inline]
fn deref(&self) -> &Self::Target {
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
str::from_utf8_unchecked(&self.vec)
}
}
}

impl DerefMut for String<'_> {
#[inline]
fn deref_mut(&mut self) -> &mut Self::Target {
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
str::from_utf8_unchecked_mut(&mut self.vec)
}
}
}

// code taken from `bumpalo::collections::Vec`
impl<'alloc> Vec<'alloc, u8> {
/// Copies all elements in the slice `other` and appends them to the `Vec`.
///
/// Note that this function is same as [`extend_from_slice`] except that it is optimized for
/// slices of types that implement the `Copy` trait. If and when Rust gets specialization
/// this function will likely be deprecated (but still available).
pub fn extend_from_slice_copy(&mut self, other: &[u8]) {
// Reserve space in the Vec for the values to be added
self.reserve(other.len());

// Copy values into the space that was just reserved
// SAFETY:
// * `self` has enough capacity to store `other.len()` more items as `self.reserve(other.len())`
// above guarantees that.
// * Source and destination data ranges cannot overlap as we just reserved the destination
// range from the bump.
unsafe {
self.extend_from_slice_copy_unchecked(other);
}
}

/// Helper method to copy all of the items in `other` and append them to the end of `self`.
///
/// SAFETY:
/// * The caller is responsible for:
/// * calling [`reserve`](Self::reserve) beforehand to guarantee that there is enough
/// capacity to store `other.len()` more items.
/// * guaranteeing that `self` and `other` do not overlap.
unsafe fn extend_from_slice_copy_unchecked(&mut self, other: &[u8]) {
let old_len = self.len();
debug_assert!(old_len + other.len() <= self.capacity());

// SAFETY:
// * `src` is valid for reads of `other.len()` values by virtue of being a `&[T]`.
// * `dst` is valid for writes of `other.len()` bytes because the caller of this
// method is required to `reserve` capacity to store at least `other.len()` items
// beforehand.
// * Because `src` is a `&[T]` and dst is a `&[T]` within the `Vec<T>`,
// `copy_nonoverlapping`'s alignment requirements are met.
// * Caller is required to guarantee that the source and destination ranges cannot overlap
unsafe {
let src = other.as_ptr();
let dst = self.as_mut_ptr().add(old_len);
ptr::copy_nonoverlapping(src, dst, other.len());
self.set_len(old_len + other.len());
}
}
}
5 changes: 2 additions & 3 deletions crates/oxc_allocator/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ use std::{
};

use allocator_api2::vec;
use bumpalo::Bump;
#[cfg(any(feature = "serialize", test))]
use serde::{ser::SerializeSeq, Serialize, Serializer};

use crate::{Allocator, Box};
use crate::{Allocator, Box, Bump};

/// A `Vec` without [`Drop`], which stores its data in the arena allocator.
///
Expand All @@ -29,7 +28,7 @@ use crate::{Allocator, Box};
/// Note: This is not a soundness issue, as Rust does not support relying on `drop`
/// being called to guarantee soundness.
#[derive(PartialEq, Eq)]
pub struct Vec<'alloc, T>(ManuallyDrop<vec::Vec<T, &'alloc Bump>>);
pub struct Vec<'alloc, T>(pub(crate) ManuallyDrop<vec::Vec<T, &'alloc Bump>>);

impl<'alloc, T> Vec<'alloc, T> {
/// Constructs a new, empty `Vec<T>`.
Expand Down
Loading