Skip to content

Commit 5e40657

Browse files
committed
The arena allocation implementation was accumulating all cons cell
allocations without limit, causing memory to grow during long-running programs (25GB+ in my machine lol)
1 parent 77df584 commit 5e40657

File tree

1 file changed

+86
-10
lines changed

1 file changed

+86
-10
lines changed

src/value.rs

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ use crate::bytecode::Chunk;
1818
//
1919
// Thread-local arena is used so native functions can allocate without
2020
// explicit arena parameter passing.
21+
//
22+
// IMPORTANT: Arena has a size limit to prevent unbounded memory growth in
23+
// long-running programs. When the limit is reached, new allocations fall
24+
// back to Rc allocation. This provides fast allocation for short-lived
25+
// programs while preventing memory exhaustion.
26+
27+
/// Maximum number of objects in arena before falling back to Rc allocation.
28+
/// This prevents unbounded memory growth in long-running programs.
29+
/// 64K objects * ~40 bytes/cons = ~2.5MB max arena size
30+
const ARENA_SIZE_LIMIT: usize = 64 * 1024;
2131

2232
/// Arena for heap object allocation without per-object reference counting.
2333
/// Uses a simple bump allocator with Vec backing storage.
@@ -38,7 +48,20 @@ impl Arena {
3848
}
3949
}
4050

51+
/// Try to allocate a heap object in the arena.
52+
/// Returns Some(index) if successful, None if arena is full.
53+
#[inline]
54+
pub fn try_alloc(&mut self, obj: HeapObject) -> Option<u32> {
55+
if self.objects.len() >= ARENA_SIZE_LIMIT {
56+
return None; // Arena full, caller should use Rc
57+
}
58+
let idx = self.objects.len() as u32;
59+
self.objects.push(obj);
60+
Some(idx)
61+
}
62+
4163
/// Allocate a heap object in the arena, returning its index
64+
/// Note: Use try_alloc() for size-limited allocation
4265
#[inline]
4366
pub fn alloc(&mut self, obj: HeapObject) -> u32 {
4467
let idx = self.objects.len() as u32;
@@ -61,6 +84,12 @@ impl Arena {
6184
self.objects.clear();
6285
}
6386

87+
/// Check if arena is full (at size limit)
88+
#[inline]
89+
pub fn is_full(&self) -> bool {
90+
self.objects.len() >= ARENA_SIZE_LIMIT
91+
}
92+
6493
/// Check if arena is empty (used for testing)
6594
#[allow(dead_code)]
6695
pub fn is_empty(&self) -> bool {
@@ -109,12 +138,6 @@ pub fn arena_size() -> usize {
109138
ARENA.with(|a| a.borrow().len())
110139
}
111140

112-
/// Allocate a heap object in the thread-local arena
113-
#[inline]
114-
fn arena_alloc(obj: HeapObject) -> u32 {
115-
ARENA.with(|a| a.borrow_mut().alloc(obj))
116-
}
117-
118141
/// Get a reference to an arena object (unsafe - caller must ensure index is valid)
119142
#[inline]
120143
unsafe fn arena_get(idx: u32) -> *const HeapObject {
@@ -335,14 +358,16 @@ impl Value {
335358
///
336359
/// OPTIMIZED: Uses arena allocation when enabled for zero-refcount cloning.
337360
/// When arena is enabled, cons cells are allocated in the thread-local arena.
338-
/// When disabled, falls back to Rc allocation for compatibility.
361+
/// When arena is full or disabled, falls back to Rc allocation.
339362
pub fn cons(car: Value, cdr: Value) -> Value {
340-
if arena_enabled() {
363+
if arena_enabled() && !ARENA.with(|a| a.borrow().is_full()) {
341364
// Arena allocation - zero overhead clone/drop
342-
let idx = arena_alloc(HeapObject::Cons(ConsCell { car, cdr }));
365+
let idx = ARENA.with(|a| {
366+
a.borrow_mut().alloc(HeapObject::Cons(ConsCell { car, cdr }))
367+
});
343368
Value(TAG_ARENA | idx as u64)
344369
} else {
345-
// Rc allocation fallback
370+
// Rc allocation fallback (arena disabled or full)
346371
let heap = Rc::new(HeapObject::Cons(ConsCell { car, cdr }));
347372
Value::from_heap(heap)
348373
}
@@ -1297,4 +1322,55 @@ mod tests {
12971322

12981323
super::clear_arena();
12991324
}
1325+
1326+
#[test]
1327+
fn test_arena_size_limit_fallback() {
1328+
super::set_arena_enabled(true);
1329+
super::clear_arena();
1330+
1331+
// Fill the arena to the limit
1332+
let limit = super::ARENA_SIZE_LIMIT;
1333+
for i in 0..limit {
1334+
let cell = Value::cons(Value::int(i as i64), Value::nil());
1335+
if i < limit {
1336+
// Should be arena-allocated until we hit the limit
1337+
assert!(cell.is_arena() || cell.is_ptr());
1338+
}
1339+
}
1340+
1341+
// Arena should now be full
1342+
assert!(super::ARENA.with(|a| a.borrow().is_full()));
1343+
1344+
// Next allocation should fall back to Rc
1345+
let overflow = Value::cons(Value::int(999999), Value::nil());
1346+
assert!(overflow.is_ptr(), "should fall back to Rc when arena is full");
1347+
assert!(!overflow.is_arena());
1348+
1349+
// Verify the Rc-allocated value still works
1350+
assert_eq!(overflow.as_cons().unwrap().car.as_int(), Some(999999));
1351+
1352+
super::clear_arena();
1353+
}
1354+
1355+
#[test]
1356+
fn test_arena_clear_allows_reuse() {
1357+
super::set_arena_enabled(true);
1358+
super::clear_arena();
1359+
1360+
// Fill the arena
1361+
for i in 0..super::ARENA_SIZE_LIMIT {
1362+
let _ = Value::cons(Value::int(i as i64), Value::nil());
1363+
}
1364+
assert!(super::ARENA.with(|a| a.borrow().is_full()));
1365+
1366+
// Clear arena
1367+
super::clear_arena();
1368+
assert_eq!(super::arena_size(), 0);
1369+
1370+
// Should be able to allocate in arena again
1371+
let cell = Value::cons(Value::int(42), Value::nil());
1372+
assert!(cell.is_arena(), "should be arena-allocated after clear");
1373+
1374+
super::clear_arena();
1375+
}
13001376
}

0 commit comments

Comments
 (0)