Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
7 changes: 1 addition & 6 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,6 @@ pub(crate) fn braces(
) -> JsResult<JSValue> {
let brace_slice = brace_str.to_utf8();

let mut arena = bun_alloc::Arena::new();
let _ = &mut arena;

let mut lexer_output = 'lexer_output: {
if strings::is_all_ascii(brace_slice.slice()) {
break 'lexer_output match Braces::Lexer::tokenize(brace_slice.slice()) {
Expand Down Expand Up @@ -430,7 +427,7 @@ pub(crate) fn braces(
return bun_str.to_js(global);
}
if opts.parse {
let mut parser = Braces::Parser::init(&lexer_output.tokens[..], &arena);
let mut parser = Braces::Parser::init(&lexer_output.tokens[..]);
let ast_node = match parser.parse() {
Ok(v) => v,
Err(err) => {
Expand All @@ -457,15 +454,13 @@ pub(crate) fn braces(
)));
}

// Non-AST crate: result containers use plain Vec (arena is only for Braces::* internals).
let expansion_count = expansion_count as usize;
let mut expanded_strings: Vec<Vec<u8>> = Vec::with_capacity(expansion_count);
for _ in 0..expansion_count {
expanded_strings.push(Vec::new());
}

match Braces::expand(
&arena,
&mut lexer_output.tokens[..],
&mut expanded_strings,
lexer_output.contains_nested,
Expand Down
3 changes: 0 additions & 3 deletions src/runtime/shell/states/Expansion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,7 @@ impl Expansion {
vec![me.current_out.clone()]
} else {
let mut expanded: Vec<Vec<u8>> = (0..count).map(|_| Vec::new()).collect();
let arena = bun_alloc::Arena::new();
if let Err(e) = braces::expand(
&arena,
&mut lexer_output.tokens[..],
&mut expanded[..],
lexer_output.contains_nested,
Expand All @@ -336,7 +334,6 @@ impl Expansion {
// An unexpected token from brace expansion is a parser bug.
panic!("unexpected error from Braces.expand: {e:?}");
}
drop(arena);
expanded
};

Expand Down
126 changes: 89 additions & 37 deletions src/shell_parser/braces.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
use core::ptr;

use bun_alloc::ArenaVecExt as _;
use bun_alloc::{AllocError, Arena as Bump};
use bun_alloc::AllocError;
// `bun.SmallList` lives in `bun_css` (higher tier). Semantically it
// is `smallvec::SmallVec` (inline-N, heap-spill). PORTING.md §Collections.
use self::StringEncoding as Encoding;
use bun_alloc::ArenaVec as BumpVec;
use bun_core::SmolStr;
use smallvec::SmallVec;

Expand Down Expand Up @@ -403,8 +401,8 @@ fn ast_atom_to_json(atom: &ast::Atom, out: &mut Vec<u8>) {
}
ast::Atom::Expansion(exp) => {
out.extend_from_slice(b"{\"expansion\":{\"variants\":[");
// SAFETY: `variants` is a bump-allocated slice live for the parse arena.
let variants = unsafe { &*exp.variants };
// SAFETY: `variants` is a live boxed slice that `exp` owns.
let variants = unsafe { &*exp.variants.as_ptr() };
for (i, g) in variants.iter().enumerate() {
if i > 0 {
out.push(b',');
Expand Down Expand Up @@ -441,8 +439,8 @@ fn ast_group_to_json(group: &ast::Group, out: &mut Vec<u8>) {
}
ast::GroupAtoms::Many(atoms) => {
out.extend_from_slice(b"{\"many\":[");
// SAFETY: bump-allocated slice live for the parse arena.
let atoms = unsafe { &**atoms };
// SAFETY: `atoms` is a live boxed slice that `group` owns.
let atoms = unsafe { &*atoms.as_ptr() };
for (i, a) in atoms.iter().enumerate() {
if i > 0 {
out.push(b',');
Expand All @@ -465,8 +463,7 @@ pub mod ast {

pub enum GroupAtoms {
Single(Atom),
// bump-owned slice; raw because Group has raw backrefs (see bubble_up).
Many(*mut [Atom]),
Many(OwnedSlice<Atom>),
}

pub struct Group {
Expand All @@ -477,9 +474,30 @@ pub mod ast {
}

pub struct Expansion {
// bump-owned mutable slice; raw because expand_nested writes
// bubble_up backrefs into elements while recursing through the parent.
pub(crate) variants: *mut [Group],
pub(crate) variants: OwnedSlice<Group>,
}

/// A `Box<[T]>` held as a raw pointer: `expand_nested` writes `bubble_up`
/// backrefs into the elements while it recurses through the parent that
/// owns them, which a `Box` reached through that parent does not allow.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub struct OwnedSlice<T>(*mut [T]);

impl<T> OwnedSlice<T> {
pub(crate) fn new(items: Vec<T>) -> Self {
OwnedSlice(bun_core::heap::into_raw(items.into_boxed_slice()))
}

pub(crate) fn as_ptr(&self) -> *mut [T] {
self.0
}
}

impl<T> Drop for OwnedSlice<T> {
fn drop(&mut self) {
// SAFETY: `self.0` came from `heap::into_raw` in `new` and is freed
// only here.
unsafe { bun_core::heap::destroy(self.0) };
}
Comment thread
robobun marked this conversation as resolved.
Resolved
}
}

Expand Down Expand Up @@ -524,7 +542,6 @@ pub(crate) type ExpandError = ParserError;

/// `out` is preallocated by using the result from `calculateExpandedAmount`
pub fn expand(
bump: &Bump,
tokens: &mut [Token],
out: &mut [Vec<u8>],
contains_nested: bool,
Expand All @@ -546,16 +563,16 @@ pub fn expand(
);
}

let mut parser = Parser::init(tokens, bump);
let mut parser = Parser::init(tokens);
let mut root_node = parser.parse()?;
// SAFETY: root_node lives on this stack frame for the duration of expand_nested;
// all bubble_up backrefs written during recursion point into bump-owned Groups
// all bubble_up backrefs written during recursion point into Groups it owns
// or back at this root.
unsafe { expand_nested(&raw mut root_node, out, 0, &mut out_key_counter, 0) }
}

// SAFETY contract: `root` must be a valid *mut Group whose `atoms` slices and
// `expansion.variants` slices are bump-owned and outlive this call. The function
// `expansion.variants` slices are owned by the tree and outlive this call. The function
// writes `bubble_up` backrefs (raw pointers) into child Groups and re-enters the
// parent through them; raw-pointer access is used throughout to avoid creating
// overlapping `&mut` borrows.
Expand All @@ -565,7 +582,7 @@ pub fn expand(
// `&(*many)[i_]`, `expansion`) are consumed before recursion — `variants` is
// hoisted to a raw pointer copy ahead of the variant loop, and every arm that
// recurses either tail-returns or returns immediately after its loop. Writes
// during recursion target only `Group` structs (the stack root or bump-owned
// during recursion target only `Group` structs (the stack root or
// `variants` elements), never the `[Atom]` slices a caller frame borrowed
// from, and bubble-up re-entry always resumes past the caller's in-progress
// atom index, so no frame's expansion is mutated while it is on the stack.
Expand All @@ -577,7 +594,7 @@ unsafe fn expand_nested(
start: u32,
) -> Result<(), ExpandError> {
// SAFETY: see fn doc comment —
// bump-owned Groups outlive this call, no overlapping `&mut` borrows are held.
// the tree's Groups outlive this call, no overlapping `&mut` borrows are held.
unsafe {
if let ast::GroupAtoms::Single(_) = (*root).atoms {
if start > 0 {
Expand Down Expand Up @@ -613,7 +630,7 @@ unsafe fn expand_nested(
}
ast::GroupAtoms::Single(ast::Atom::Expansion(expansion)) => {
let length = out[out_key].len();
let variants = expansion.variants;
let variants = expansion.variants.as_ptr();
let variants_len = variants.len();
for j in 0..variants_len {
let group: *mut ast::Group = (*variants).as_mut_ptr().add(j);
Expand Down Expand Up @@ -641,7 +658,7 @@ unsafe fn expand_nested(
}

let many: *mut [ast::Atom] = match &(*root).atoms {
ast::GroupAtoms::Many(m) => *m,
ast::GroupAtoms::Many(m) => m.as_ptr(),
_ => unreachable!(),
};
let many_len = many.len();
Expand All @@ -664,7 +681,7 @@ unsafe fn expand_nested(
}
ast::Atom::Expansion(expansion) => {
let length = out[out_key].len();
let variants = expansion.variants;
let variants = expansion.variants.as_ptr();
let variants_len = variants.len();
for j in 0..variants_len {
let group: *mut ast::Group = (*variants).as_mut_ptr().add(j);
Expand Down Expand Up @@ -774,26 +791,21 @@ fn expand_flat(
}

// FIXME error location
// lifetime on transient parser struct; `tokens`/`bump` borrowed from caller
// lifetime on transient parser struct; `tokens` borrowed from caller
// for the parse() call only — not an AST node.
pub struct Parser<'a> {
current: usize,
tokens: &'a [Token],
bump: &'a Bump,
}

impl<'a> Parser<'a> {
pub fn init(tokens: &'a [Token], bump: &'a Bump) -> Parser<'a> {
Parser {
current: 0,
tokens,
bump,
}
pub fn init(tokens: &'a [Token]) -> Parser<'a> {
Parser { current: 0, tokens }
}

pub fn parse(&mut self) -> Result<ast::Group, ParserError> {
check_brace_group_count(self.tokens)?;
let mut nodes: BumpVec<'a, ast::Atom> = BumpVec::new_in(self.bump);
let mut nodes: Vec<ast::Atom> = Vec::new();
while !self.r#match(TokenTag::Eof) {
match self.parse_atom()? {
Some(atom) => nodes.push(atom),
Expand All @@ -809,11 +821,10 @@ impl<'a> Parser<'a> {
atoms: ast::GroupAtoms::Single(single),
})
} else {
let many = std::ptr::from_mut::<[ast::Atom]>(nodes.into_bump_slice_mut());
Ok(ast::Group {
bubble_up: ptr::null_mut(),
bubble_up_next: None,
atoms: ast::GroupAtoms::Many(many),
atoms: ast::GroupAtoms::Many(ast::OwnedSlice::new(nodes)),
})
}
}
Expand All @@ -831,9 +842,9 @@ impl<'a> Parser<'a> {
}

fn parse_expansion(&mut self) -> Result<ast::Expansion, ParserError> {
let mut variants: BumpVec<'a, ast::Group> = BumpVec::new_in(self.bump);
let mut variants: Vec<ast::Group> = Vec::new();
loop {
let mut group: BumpVec<'a, ast::Atom> = BumpVec::new_in(self.bump);
let mut group: Vec<ast::Atom> = Vec::new();
// Only this inner loop consumes Close/Eof so a trailing empty
// variant (`{a,}`) is pushed before the outer loop exits.
let close = loop {
Expand All @@ -856,11 +867,10 @@ impl<'a> Parser<'a> {
atoms: ast::GroupAtoms::Single(single),
});
} else {
let many = std::ptr::from_mut::<[ast::Atom]>(group.into_bump_slice_mut());
variants.push(ast::Group {
bubble_up: ptr::null_mut(),
bubble_up_next: None,
atoms: ast::GroupAtoms::Many(many),
atoms: ast::GroupAtoms::Many(ast::OwnedSlice::new(group)),
});
}
if close {
Expand All @@ -869,7 +879,7 @@ impl<'a> Parser<'a> {
}

Ok(ast::Expansion {
variants: std::ptr::from_mut::<[ast::Group]>(variants.into_bump_slice_mut()),
variants: ast::OwnedSlice::new(variants),
})
}

Expand Down Expand Up @@ -1297,4 +1307,46 @@ mod tests {
assert_eq!(result.tokens, expected);
}
}

fn expand_all(src: &[u8]) -> Vec<Vec<u8>> {
let mut lexed = Lexer::tokenize(src).unwrap();
let count = calculate_expanded_amount(&lexed.tokens) as usize;
let mut out: Vec<Vec<u8>> = vec![Vec::new(); count];
expand(&mut lexed.tokens, &mut out, lexed.contains_nested).unwrap();
out
}

#[test]
fn expand_flat_groups() {
assert_eq!(
expand_all(b"x{1,2}{a,b}"),
[&b"x1a"[..], b"x1b", b"x2a", b"x2b"]
);
}

// Under Miri this also checks that the nested AST frees what it owns: the
// long atoms are heap-backed `SmolStr`s.
Comment thread
robobun marked this conversation as resolved.
Resolved
#[test]
fn expand_nested_groups() {
assert_eq!(
expand_all(b"a{b,{c,d}e}f{g,h}"),
[
&b"abfg"[..],
b"abfh",
b"acefg",
b"acefh",
b"adefg",
b"adefh"
]
);
assert_eq!(
expand_all(b"{a_text_longer_than_inline,{b,c{d,e}}}-tail_text_longer_than_inline"),
[
&b"a_text_longer_than_inline-tail_text_longer_than_inline"[..],
b"b-tail_text_longer_than_inline",
b"cd-tail_text_longer_than_inline",
b"ce-tail_text_longer_than_inline",
]
);
}
}
34 changes: 33 additions & 1 deletion test/js/bun/shell/brace.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { $ } from "bun";
import { heapStats } from "bun:jsc";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
import { bunEnv, bunExe, expectRssDeltaBelow, isDebug, normalizeBunSnapshot, tempDir } from "harness";

describe("$.braces", () => {
test("no-op", () => {
Expand Down Expand Up @@ -126,6 +127,37 @@ console.log(JSON.stringify(Bun.$.braces("{" + inner)));`,
const result = $.braces(`lol {😂,🫵,🤣}`);
expect(result).toEqual(["lol 😂", "lol 🫵", "lol 🤣"]);
});

test("does not create a mimalloc heap per call", () => {
const heapsCreated = () => heapStats().mimalloc.heaps.total;
const before = heapsCreated();
for (let i = 0; i < 100; i++) {
expect($.braces(`flat${i}{1,2,3}`)).toEqual([`flat${i}1`, `flat${i}2`, `flat${i}3`]);
expect($.braces(`nested${i}{1,{2,3}}`)).toEqual([`nested${i}1`, `nested${i}2`, `nested${i}3`]);
}
// One per call when the expansion builds its AST in an arena of its own.
expect(heapsCreated() - before).toBeLessThan(10);
});

// A debug build lexes 256 KiB in about 100 ms, so this takes 14 s there.
test.skipIf(isDebug).concurrent("a nested expansion frees its atoms", async () => {
// An atom longer than 15 bytes is a heap allocation of its own.
const code = /* js */ `
const atom = Buffer.alloc(256 * 1024, "a").toString();
const pattern = "{" + atom + ",{b,c}}";
for (let i = 0; i < 8; i++) Bun.$.braces(pattern);
Bun.gc(true);
const before = process.memoryUsage.rss();
for (let i = 0; i < 128; i++) {
if (Bun.$.braces(pattern).length !== 3) throw new Error("expected 3 expansions");
}
Bun.gc(true);
console.log(JSON.stringify({ deltaMiB: (process.memoryUsage.rss() - before) / 1024 / 1024 }));
`;

// Unfixed: 37 MiB, one copy of the atom per call. Fixed: under 3 MiB.
await expectRssDeltaBelow(["--smol", "-e", code], { release: 16, debug: 16 });
});
});

// A shell word combining brace + glob (`src/*.{ts,tsx}`, `{src,lib}/*.ts`) was
Expand Down
Loading