Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
110 changes: 79 additions & 31 deletions src/css_jsc/color_js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,48 @@ fn zero_if_none(component: f32) -> f32 {
if component.is_nan() { 0.0 } else { component }
}

/// Run `f` with a borrow of a reused per-thread scratch arena, returning
/// whatever it produces. Used for both halves of `Bun.color` on a string —
/// the CSS parse and the CSS-string serialization.
///
/// Neither half allocates into the arena on the common path: the tokenizer
/// threads `&'a Arena` through `ParserInput` but idents/numbers/percentages are
/// borrowed sub-slices of the input (see `Tokenizer::consume_name`/
/// `consume_numeric`), and the printer's `scratchbuf`/`indentation_buf` stay
/// empty for a bare `CssColor` (which serializes straight into a global-heap
/// `Vec<u8>`). Only an escape sequence / NUL / non-ASCII byte inside an ident
/// forces a copy-on-write `ArenaVec`, deliberately abandoned into the arena
/// (`CopyOnWriteStr`'s `into_bump_slice`), relying on a bulk free.
///
/// A fresh `Arena::new()` per call pays a `mi_heap_new()` + `mi_heap_destroy()`
/// round-trip — which profiling attributes ~900ns per site to, dwarfing the
/// work itself — so reuse one warm heap per thread instead and reset it before
/// each use. `reset_retain_with_limit` keeps the heap while its footprint is
/// small (the common case allocates zero, a cheap retain) and only falls back
/// to `mi_heap_destroy` + `mi_heap_new` once abandoned buffers exceed the cap —
/// bounding steady-state memory without leaking. Unlike `borrowing_default()`,
/// any stray allocation lands in this arena and is reclaimed by the next reset
/// rather than leaking into `mi_heap_main()`.
///
/// The arena borrow is confined to `f` via the `RefCell` guard, so there is no
/// `unsafe`: `f`'s return value carries no arena lifetime, and `Bun.color` runs
/// on the JS thread where neither the tokenizer nor the printer re-enters this
/// — a re-entry would be a safe `RefCell` double-borrow panic, not UB. The two
/// uses within one call are sequential (parse, then serialize), so they take
/// the borrow one after another, never nested.
fn with_color_arena<R>(f: impl FnOnce(&Arena) -> R) -> R {
use std::cell::RefCell;

thread_local! {
static ARENA: RefCell<Arena> = RefCell::new(Arena::new());
}

ARENA.with_borrow_mut(|arena| {
arena.reset_retain_with_limit(64 * 1024);
f(arena)
})
}

pub fn js_function_color(global: &JSGlobalObject, frame: &CallFrame) -> JsResult<JSValue> {
use bun_ast::symbol::Map as SymbolMap;
use bun_core::ZigStringSlice;
Expand Down Expand Up @@ -324,17 +366,18 @@ pub fn js_function_color(global: &JSGlobalObject, frame: &CallFrame) -> JsResult

input = args[0].to_slice(global)?;

// MimallocArena::new() calls mi_heap_new(), so defer creation to the
// paths that actually allocate.
let arena = Arena::new();
let mut parser_input = css::ParserInput::new(input.slice(), &arena);
let mut parser = css::Parser::new(
&mut parser_input,
None,
css::css_parser::ParserOpts::default(),
None,
);
break 'brk CssColor::parse(&mut parser);
// Borrow the per-thread scratch arena (see `with_color_arena`); the
// parsed `CssColor` is fully owned, so it outlives the borrow.
break 'brk with_color_arena(|arena| {
let mut parser_input = css::ParserInput::new(input.slice(), arena);
let mut parser = css::Parser::new(
&mut parser_input,
None,
css::css_parser::ParserOpts::default(),
None,
);
CssColor::parse(&mut parser)
});
};

match parsed_color {
Expand Down Expand Up @@ -598,27 +641,32 @@ pub fn js_function_color(global: &JSGlobalObject, frame: &CallFrame) -> JsResult
return str.transfer_to_js(global);
}

// Fallback to CSS string output
let arena = Arena::new();
let mut dest: Vec<u8> = Vec::new();

let symbols = SymbolMap::init_list(Default::default());
let mut printer = css::Printer::new(
&arena,
bun_alloc::ArenaVec::<u8>::new_in(&arena),
&mut dest,
&css::PrinterOptions::default(),
None,
None,
&symbols,
);

if let Err(err) = result.to_css(&mut printer) {
return Err(global.throw(format_args!("color() internal error: {}", err.name())));
}
drop(printer);
// Fallback to CSS string output. Borrow the per-thread scratch arena
// (see `with_color_arena`) rather than `borrowing_default()` so any
// stray printer allocation is reclaimed instead of leaking.
return with_color_arena(|arena| {
let mut dest: Vec<u8> = Vec::new();

let symbols = SymbolMap::init_list(Default::default());
let mut printer = css::Printer::new(
arena,
bun_alloc::ArenaVec::<u8>::new_in(arena),
&mut dest,
&css::PrinterOptions::default(),
None,
None,
&symbols,
);

if let Err(err) = result.to_css(&mut printer) {
return Err(
global.throw(format_args!("color() internal error: {}", err.name()))
);
}
drop(printer);

return bun_jsc::bun_string_jsc::create_utf8_for_js(global, &dest);
bun_jsc::bun_string_jsc::create_utf8_for_js(global, &dest)
});
}
}
}
38 changes: 38 additions & 0 deletions test/js/bun/css/color.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,3 +536,41 @@ describe("input forms", () => {
expect(color("#f00", "[rgb]")).toEqual([255, 0, 0]);
});
});

// The string parser reuses one per-thread scratch arena across calls instead
// of creating a fresh mimalloc heap each time. Most color literals allocate
// nothing into it, but a CSS escape sequence (`\72\65\64` == "red") or a
// function token (`url(...)`) forces the tokenizer to allocate copy-on-write
// buffers there. This interleaves those arena-allocating inputs with cheap ones
// and invalid ones in a tight loop: if the arena reset/reuse leaked state
// between calls, an escaped-ident or function-token parse would corrupt a later
// call's result. Every call must return exactly what it returns in isolation.
test("reused parse arena stays correct across interleaved calls", () => {
// [input, expected `css` output] — covers escapes (arena copy-on-write),
// plain literals (no arena alloc), and invalid/function tokens.
const cases: Array<[string, string | null]> = [
["\\72\\65\\64", "red"], // "red" via hex escapes -> arena copy-on-write
["#f00", "red"], // plain literal -> no arena allocation
["re\\64", "red"], // "red" with a single escaped 'd'
["rgb(0, 0, 255)", "#00f"], // plain literal
["\\62\\6c\\75\\65", "#00f"], // "blue" via escapes -> arena copy-on-write
["hsl(0, 100%, 50%)", "red"], // plain literal
["url(#bad)", null], // function token -> arena, invalid -> null
["bad color input", null], // invalid
];

// Sanity: each result in isolation matches the expectation.
for (const [input, expected] of cases) {
expect(color(input, "css")).toBe(expected);
}

withoutAggressiveGC(() => {
for (let i = 0; i < 10_000; i++) {
for (const [input, expected] of cases) {
if (color(input, "css") !== expected) {
throw new Error(`color(${JSON.stringify(input)}, "css") !== ${JSON.stringify(expected)} on iteration ${i}`);
}
}
}
});
});
Loading