Skip to content

Commit 54223ca

Browse files
committed
Actually allocate an empty vec instead of dangling
Dangling pointers don't have a valid `GcVecHeader`, so its not possible to fetch the length/capacity. The allocated vector is a singleton (assuming proper alignment), so we still wont waste much memory.
1 parent b9d9c0e commit 54223ca

File tree

4 files changed

+95
-36
lines changed

4 files changed

+95
-36
lines changed

libs/context/src/collector.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,10 @@ pub unsafe trait RawSimpleAlloc: RawCollectorImpl {
350350
fn alloc<'gc, T: GcSafe + 'gc>(context: &'gc CollectorContext<Self>, value: T) -> Gc<'gc, T, CollectorId<Self>>;
351351
unsafe fn alloc_uninit_slice<'gc, T>(context: &'gc CollectorContext<Self>, len: usize) -> (CollectorId<Self>, *mut T)
352352
where T: GcSafe + 'gc;
353-
fn alloc_vec_with_capacity<'gc, T>(context: &'gc CollectorContext<Self>, capacity: usize) -> GcVec<'gc, T, CollectorContext<Self>> where T: GcSafe + 'gc;
353+
fn alloc_vec<'gc, T>(context: &'gc CollectorContext<Self>) -> GcVec<'gc, T, CollectorContext<Self>>
354+
where T: GcSafe + 'gc;
355+
fn alloc_vec_with_capacity<'gc, T>(context: &'gc CollectorContext<Self>, capacity: usize) -> GcVec<'gc, T, CollectorContext<Self>>
356+
where T: GcSafe + 'gc;
354357
}
355358
unsafe impl<C> GcSimpleAlloc for CollectorContext<C>
356359
where C: RawSimpleAlloc {
@@ -368,7 +371,7 @@ unsafe impl<C> GcSimpleAlloc for CollectorContext<C>
368371

369372
#[inline]
370373
fn alloc_vec<'gc, T>(&'gc self) -> GcVec<'gc, T, Self> where T: GcSafe + 'gc {
371-
self.alloc_vec_with_capacity(0)
374+
C::alloc_vec(self)
372375
}
373376

374377
#[inline]

libs/simple/src/layout.rs

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ impl<H> Clone for HeaderLayout<H> {
6565
}
6666

6767
impl<H> HeaderLayout<H> {
68+
#[inline]
69+
pub(crate) const fn value_offset_from_common_header(&self, align: usize) -> usize {
70+
self.value_offset(align) - self.common_header_offset
71+
}
6872
#[inline]
6973
pub(crate) const fn into_unknown(self) -> HeaderLayout<UnknownHeader> {
7074
HeaderLayout {
@@ -272,8 +276,8 @@ impl<T: GcSafe> SimpleVecRepr<T> {
272276
}
273277
}
274278
unsafe impl<T: GcSafe> GcVecRepr for SimpleVecRepr<T> {
275-
/// We do support reallocation, but only for large sized vectors
276-
const SUPPORTS_REALLOC: bool = true;
279+
/// Right now, there is no stable API for in-place re-allocation
280+
const SUPPORTS_REALLOC: bool = false;
277281

278282
#[inline]
279283
fn element_layout(&self) -> Layout {
@@ -296,17 +300,9 @@ unsafe impl<T: GcSafe> GcVecRepr for SimpleVecRepr<T> {
296300
unsafe { (*self.header()).capacity }
297301
}
298302

299-
fn realloc_in_place(&self, new_capacity: usize) -> Result<(), ReallocFailedError> {
300-
if !fits_small_object(Self::layout(new_capacity)) {
301-
// TODO: Use allocator api for realloc
302-
todo!("Big object realloc")
303-
} else {
304-
Err(ReallocFailedError::SizeUnsupported)
305-
}
306-
}
307-
303+
#[inline]
308304
unsafe fn ptr(&self) -> *const c_void {
309-
todo!()
305+
self as *const Self as *const c_void // We are actually just a GC pointer to the value ptr
310306
}
311307
}
312308
unsafe_gc_impl!(
@@ -476,8 +472,7 @@ impl<T: GcSafe> StaticVecType for T {
476472
value_offset_from_common_header: {
477473
// We have same alignment as our members
478474
let align = std::mem::align_of::<T>();
479-
let header_layout = GcVecHeader::LAYOUT;
480-
header_layout.value_offset(align) - header_layout.common_header_offset
475+
GcArrayHeader::LAYOUT.value_offset_from_common_header(align)
481476
},
482477
trace_func: if T::NEEDS_TRACE {
483478
Some({
@@ -517,8 +512,7 @@ impl<T: GcSafe> StaticGcType for [T] {
517512
const STATIC_TYPE: &'static GcType = &GcType {
518513
layout: GcTypeLayout::Array { element_layout: Layout::new::<T>() },
519514
value_offset_from_common_header: {
520-
let header_layout = GcArrayHeader::LAYOUT;
521-
header_layout.value_offset(std::mem::align_of::<T>()) - header_layout.common_header_offset
515+
GcArrayHeader::LAYOUT.value_offset_from_common_header(std::mem::align_of::<T>())
522516
},
523517
trace_func: if <T as Trace>::NEEDS_TRACE {
524518
Some({
@@ -551,7 +545,9 @@ impl<T: GcSafe> StaticGcType for [T] {
551545
impl<T: GcSafe> StaticGcType for T {
552546
const STATIC_TYPE: &'static GcType = &GcType {
553547
layout: GcTypeLayout::Fixed(Layout::new::<T>()),
554-
value_offset_from_common_header: GcHeader::LAYOUT.value_offset(std::mem::align_of::<T>()),
548+
value_offset_from_common_header: {
549+
GcHeader::LAYOUT.value_offset_from_common_header(std::mem::align_of::<T>())
550+
},
555551
trace_func: if <T as Trace>::NEEDS_TRACE {
556552
Some(unsafe { mem::transmute::<_, unsafe fn(*mut c_void, &mut MarkVisitor)>(
557553
<T as DynTrace>::trace as fn(&mut T, &mut MarkVisitor),

libs/simple/src/lib.rs

Lines changed: 77 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,13 @@ use zerogc::{GcSafe, Trace, GcVisitor};
6464
use zerogc_context::utils::{ThreadId, MemorySize};
6565

6666
use crate::alloc::{SmallArenaList, SmallArena};
67-
use crate::layout::{StaticGcType, GcType, SimpleVecRepr, DynamicObj, StaticVecType, SimpleMarkData, SimpleMarkDataSnapshot, GcHeader, BigGcObject, HeaderLayout, GcArrayHeader, GcVecHeader};
67+
use crate::layout::{StaticGcType, GcType, SimpleVecRepr, DynamicObj, StaticVecType, SimpleMarkData, SimpleMarkDataSnapshot, GcHeader, BigGcObject, HeaderLayout, GcArrayHeader, GcVecHeader, GcTypeLayout};
6868

6969
use zerogc_context::collector::{RawSimpleAlloc, RawCollectorImpl};
7070
use zerogc_context::handle::{GcHandleList, RawHandleImpl};
7171
use zerogc_context::{CollectionManager as AbstractCollectionManager, RawContext as AbstractRawContext, CollectorContext};
7272
use zerogc::vec::{GcRawVec};
73+
use std::cell::Cell;
7374

7475
#[cfg(feature = "small-object-arenas")]
7576
mod alloc;
@@ -116,6 +117,9 @@ impl Default for GcConfig {
116117
}
117118
}
118119

120+
/// The alignment of the singleton empty vector
121+
const EMPTY_VEC_ALIGNMENT: usize = std::mem::align_of::<usize>();
122+
119123
#[cfg(feature = "sync")]
120124
type RawContext<C> = zerogc_context::state::sync::RawContext<C>;
121125
#[cfg(feature = "sync")]
@@ -166,20 +170,40 @@ unsafe impl RawSimpleAlloc for RawSimpleCollector {
166170
(context.collector().id(), ptr.cast())
167171
}
168172

173+
#[inline]
174+
fn alloc_vec<'gc, T>(context: &'gc CollectorContext<Self>) -> GcVec<'gc, T> where T: GcSafe + 'gc {
175+
if std::mem::align_of::<T>() > EMPTY_VEC_ALIGNMENT {
176+
// We have to do an actual allocation because we want higher alignment :(
177+
return Self::alloc_vec_with_capacity(context, 0)
178+
}
179+
let header = context.collector().heap.empty_vec();
180+
// NOTE: Assuming header is already initialized
181+
unsafe {
182+
debug_assert_eq!((*header).len.get(), 0);
183+
debug_assert_eq!((*header).capacity, 0);
184+
}
185+
let id = context.collector().id();
186+
let ptr = unsafe { NonNull::new_unchecked((header as *mut u8)
187+
.add(GcVecHeader::LAYOUT.value_offset(EMPTY_VEC_ALIGNMENT))) };
188+
GcVec {
189+
raw: unsafe { GcRawVec::from_repr(Gc::from_raw(id, ptr.cast())) },
190+
context
191+
}
192+
}
193+
169194
fn alloc_vec_with_capacity<'gc, T>(context: &'gc CollectorContext<Self>, capacity: usize) -> GcVec<'gc, T> where T: GcSafe + 'gc {
170-
let ptr = if capacity == 0 {
171-
NonNull::dangling()
172-
} else {
173-
let (header, value_ptr) = context.collector().heap.allocator.alloc_layout(
174-
GcVecHeader::LAYOUT,
175-
SimpleVecRepr::<T>::layout(capacity),
176-
<T as StaticVecType>::STATIC_VEC_TYPE
177-
);
178-
unsafe {
179-
(*header).capacity = capacity;
180-
(*header).len.set(0);
181-
NonNull::new_unchecked(value_ptr as *mut SimpleVecRepr<T>)
182-
}
195+
if capacity == 0 && std::mem::align_of::<T>() <= EMPTY_VEC_ALIGNMENT {
196+
return Self::alloc_vec(context)
197+
}
198+
let (header, value_ptr) = context.collector().heap.allocator.alloc_layout(
199+
GcVecHeader::LAYOUT,
200+
SimpleVecRepr::<T>::layout(capacity),
201+
<T as StaticVecType>::STATIC_VEC_TYPE
202+
);
203+
let ptr = unsafe {
204+
(*header).capacity = capacity;
205+
(*header).len.set(0);
206+
NonNull::new_unchecked(value_ptr as *mut SimpleVecRepr<T>)
183207
};
184208
let id = context.collector().id();
185209
GcVec {
@@ -241,7 +265,8 @@ unsafe impl DynTrace for GcHandleListWrapper {
241265
struct GcHeap {
242266
config: Arc<GcConfig>,
243267
threshold: AtomicUsize,
244-
allocator: SimpleAlloc
268+
allocator: SimpleAlloc,
269+
cached_empty_vec: Cell<Option<*mut GcVecHeader>>
245270
}
246271
impl GcHeap {
247272
fn new(config: Arc<GcConfig>) -> GcHeap {
@@ -252,7 +277,43 @@ impl GcHeap {
252277
config.initial_threshold
253278
}),
254279
allocator: SimpleAlloc::new(),
255-
config
280+
config, cached_empty_vec: Cell::new(None)
281+
}
282+
}
283+
#[inline]
284+
pub fn empty_vec(&self) -> *mut GcVecHeader {
285+
match self.cached_empty_vec.get() {
286+
Some(cached) => cached,
287+
None => {
288+
let res = self.create_empty_vec();
289+
self.cached_empty_vec.set(Some(self.create_empty_vec()));
290+
res
291+
}
292+
}
293+
}
294+
#[cold]
295+
fn create_empty_vec<'gc>(&self) -> *mut GcVecHeader {
296+
const DUMMY_LAYOUT: Layout = unsafe { Layout::from_size_align_unchecked(
297+
0, EMPTY_VEC_ALIGNMENT
298+
) };
299+
const DUMMY_TYPE: GcType = GcType {
300+
layout: GcTypeLayout::Vec {
301+
element_layout: DUMMY_LAYOUT,
302+
},
303+
value_offset_from_common_header: GcVecHeader::LAYOUT
304+
.value_offset_from_common_header(EMPTY_VEC_ALIGNMENT),
305+
drop_func: None,
306+
trace_func: None
307+
};
308+
let (header, _) = self.allocator.alloc_layout(
309+
GcVecHeader::LAYOUT,
310+
DUMMY_LAYOUT,
311+
&DUMMY_TYPE
312+
);
313+
unsafe {
314+
(*header).capacity = 0;
315+
(*header).len.set(0);
316+
header
256317
}
257318
}
258319
#[inline]

libs/simple/tests/arrays.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use zerogc::safepoint;
55
use zerogc_derive::Trace;
66

77
use zerogc_simple::{SimpleCollector, GcArray, GcVec, Gc, CollectorId as SimpleCollectorId, GcConfig};
8-
use zerogc::vec::GcRawVec;
98

109
fn test_collector() -> SimpleCollector {
1110
let mut config = GcConfig::default();

0 commit comments

Comments
 (0)