Skip to content
Merged
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
50 changes: 50 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: Rust - Continuous integration

on:
push:
branches: ["main"]
pull_request:
branches: ["main"]

jobs:
lint:
name: Lint
runs-on: ubuntu-latest
steps:
- name: Set environment
# Setting `RUSTFLAGS` overrides any flags set on .cargo/config.toml, so we need to
# set the target flags instead which are cumulative.
# Track https://github.com/rust-lang/cargo/issues/5376
run: |
target=$(rustc -vV | awk '/^host/ { print $2 }' | tr [:lower:] [:upper:] | tr '-' '_')
echo "CARGO_TARGET_${target}_RUSTFLAGS=$FLAGS" >> $GITHUB_ENV

- uses: actions/checkout@v6
- uses: dtolnay/rust-toolchain@stable
with:
components: clippy, rustfmt
- uses: Swatinem/rust-cache@v2
with:
key: lint
- name: Install cargo-workspaces
run: cargo install cargo-workspaces
- name: Check rustfmt
run: cargo fmt --all -- --check
- name: Check clippy
run: cargo ws exec cargo clippy --all-features --all-targets
- name: Check clippy (No features)
run: cargo ws exec cargo clippy --no-default-features --all-targets

docs:
name: Documentation
runs-on: ubuntu-latest
env:
RUSTDOCFLAGS: -D warnings
steps:
- uses: actions/checkout@v6
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@v2
with:
key: docs
- name: Generate documentation
run: cargo doc -v --document-private-items --all-features
11 changes: 7 additions & 4 deletions oscars/src/alloc/arena2/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ impl ErasedHeapItem {
self.buf.cast::<T>()
}

pub fn as_ref<T>(&self) -> &T {
unsafe { self.get().as_ref() }
}

pub fn mark_dropped(&mut self) {
if !self.next.is_tagged() {
self.next.tag()
Expand All @@ -87,6 +83,13 @@ impl ErasedHeapItem {
}
}

impl<T> core::convert::AsRef<T> for ErasedHeapItem {
fn as_ref(&self) -> &T {
// SAFETY: TODO
unsafe { self.get().as_ref() }
}
}

const MASK: usize = 1usize << (usize::BITS as usize - 1usize);

#[derive(Debug, Clone, Copy)]
Expand Down
4 changes: 3 additions & 1 deletion oscars/src/alloc/arena2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use rust_alloc::collections::LinkedList;
mod alloc;

use alloc::Arena;
pub use alloc::{ArenaAllocationData, ArenaHeapItem, ArenaPointer, ErasedArenaPointer, ErasedHeapItem};
pub use alloc::{
ArenaAllocationData, ArenaHeapItem, ArenaPointer, ErasedArenaPointer, ErasedHeapItem,
};

#[cfg(test)]
mod tests;
Expand Down
11 changes: 7 additions & 4 deletions oscars/src/collectors/mark_sweep/internals/ephemeron.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ impl<K: Trace, V: Trace> Ephemeron<K, V> {
// Creates a new [`Ephemeron`] with given key and value
//
// The [`WeakGcBox`] for the key is created internally from the provided [`Gc`] pointer
pub fn new_in(key: &Gc<K>, value: V, collector: &mut MarkSweepGarbageCollector) -> Self
{
pub fn new_in(key: &Gc<K>, value: V, collector: &mut MarkSweepGarbageCollector) -> Self {
let weak_key = WeakGcBox::new(key.inner_ptr);
let value = GcBox::new(value, &collector.state);
let vtable = vtable_of::<K, V>();
Self { key: weak_key, value, vtable }
Self {
key: weak_key,
value,
vtable,
}
}

pub fn key(&self) -> &K {
Expand Down Expand Up @@ -61,7 +64,7 @@ impl<K: Trace, V: Trace> Ephemeron<K, V> {
pub(crate) fn is_reachable_fn(&self) -> EphemeronIsReachableFn {
self.vtable.is_reachable_fn
}

pub(crate) fn finalize_fn(&self) -> EphemeronFinalizeFn {
self.vtable.finalize_fn
}
Expand Down
20 changes: 8 additions & 12 deletions oscars/src/collectors/mark_sweep/internals/gc_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ unsafe impl Trace for NonTraceable {

// NOTE: This may not be the best idea, but let's find out.
//
use crate::alloc::arena2::{ErasedArenaPointer, ArenaHeapItem};
use core::ptr::NonNull;
use crate::alloc::arena2::{ArenaHeapItem, ErasedArenaPointer};
use core::marker::PhantomData;
use core::ptr::NonNull;

#[repr(transparent)]
pub struct WeakGcBox<T: Trace + ?Sized + 'static> {
Expand All @@ -38,7 +38,10 @@ pub struct WeakGcBox<T: Trace + ?Sized + 'static> {

impl<T: Trace + Finalize + ?Sized> WeakGcBox<T> {
pub fn new(inner_ptr: ErasedArenaPointer<'static>) -> Self {
Self { inner_ptr, marker: PhantomData }
Self {
inner_ptr,
marker: PhantomData,
}
}

pub(crate) fn erased_inner_ptr(&self) -> NonNull<GcBox<NonTraceable>> {
Expand All @@ -65,10 +68,6 @@ impl<T: Trace + Finalize + ?Sized> WeakGcBox<T> {
self.inner_ref().is_reachable(color)
}

pub(crate) fn is_rooted(&self) -> bool {
self.inner_ref().is_rooted()
}

pub(crate) fn mark(&self, color: HeaderColor) {
self.inner_ref().header.mark(color);
}
Expand All @@ -80,7 +79,7 @@ impl<T: Trace + Finalize + ?Sized> WeakGcBox<T> {

impl<T: Trace> WeakGcBox<T> {
pub(crate) fn inner_ptr(&self) -> crate::alloc::arena2::ArenaPointer<'static, GcBox<T>> {
// SAFETY: This pointer started out as a `GcBox<T>`, so it's safe to cast
// SAFETY: This pointer started out as a `GcBox<T>`, so it's safe to cast
// it back, the `PhantomData` guarantees that the type `T` is still correct
unsafe { self.inner_ptr.to_typed_arena_pointer::<GcBox<T>>() }
}
Expand Down Expand Up @@ -128,10 +127,7 @@ impl<T: Trace> GcBox<T> {

// TODO (nekevss): What is the best function signature here?
#[inline]
pub(crate) fn new_typed(
value: T,
collection_state: &CollectionState,
) -> Self {
pub(crate) fn new_typed(value: T, collection_state: &CollectionState) -> Self {
//check for color sync issue
let header = match collection_state.color {
TraceColor::White => GcHeader::new_typed::<true>(),
Expand Down
9 changes: 4 additions & 5 deletions oscars/src/collectors/mark_sweep/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ pub(crate) mod trace;

pub mod cell;

#[cfg(test)]
#[cfg(all(test, feature = "mark_sweep"))]
mod tests;

pub(crate) mod internals;

pub use trace::{Trace, Finalize, TraceColor};
pub use trace::{Finalize, Trace, TraceColor};

pub use pointers::{Gc, WeakGc, WeakMap};

Expand Down Expand Up @@ -196,7 +196,7 @@ impl MarkSweepGarbageCollector {
self.run_sweep_phase();
// We've run a collection, so we switch the color.
self.state.color = self.state.color.flip();
// NOTE: It would actually be interesting to reuse the arenas that are dead rather
// NOTE: It would actually be interesting to reuse the arenas that are dead rather
// than drop the page and reallocate when a new page is needed ... TBD

// prune dead entries from each collector owned weak map before freeing
Expand Down Expand Up @@ -259,8 +259,7 @@ impl MarkSweepGarbageCollector {
let color = self.state.color;

// check if the key is reachable via the vtable
let is_reachable =
unsafe { ephemeron.is_reachable_fn()(*node, color) };
let is_reachable = unsafe { ephemeron.is_reachable_fn()(*node, color) };

if !is_reachable {
unsafe { ephemeron.finalize_fn()(*node) };
Expand Down
3 changes: 1 addition & 2 deletions oscars/src/collectors/mark_sweep/pointers/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ pub struct WeakGc<T: Trace + 'static> {
}

impl<T: Trace> WeakGc<T> {
pub fn new_in(value: &super::Gc<T>, collector: &mut MarkSweepGarbageCollector) -> Self
{
pub fn new_in(value: &super::Gc<T>, collector: &mut MarkSweepGarbageCollector) -> Self {
let ephemeron = Ephemeron::new_in(value, (), collector);
let inner_ptr = collector.alloc_epemeron_with_collection(ephemeron);
Self { inner_ptr }
Expand Down
24 changes: 9 additions & 15 deletions oscars/src/collectors/mark_sweep/pointers/weak_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ use hashbrown::HashMap;
use crate::{
alloc::arena2::{ArenaPointer, ErasedHeapItem},
collectors::mark_sweep::{
Finalize, MarkSweepGarbageCollector, TraceColor,
internals::Ephemeron,
trace::Trace,
Finalize, MarkSweepGarbageCollector, TraceColor, internals::Ephemeron, trace::Trace,
},
};

Expand All @@ -25,15 +23,12 @@ struct WeakMapInner<K: Trace + 'static, V: Trace + 'static> {

impl<K: Trace, V: Trace> WeakMapInner<K, V> {
fn new() -> Self {
Self { entries: HashMap::new() }
Self {
entries: HashMap::new(),
}
}

fn insert(
&mut self,
key: &Gc<K>,
value: V,
collector: &mut MarkSweepGarbageCollector,
) {
fn insert(&mut self, key: &Gc<K>, value: V, collector: &mut MarkSweepGarbageCollector) {
let key_addr = key.inner_ptr.as_non_null().as_ptr() as usize;

// Drop the old entry before allocating a new one to prevent the old
Expand Down Expand Up @@ -77,9 +72,7 @@ impl<K: Trace, V: Trace> ErasedWeakMap for WeakMapInner<K, V> {
self.entries.retain(|_, ephemeron_ptr| {
// SAFETY: Memory is valid until next collector step
// We only read the dropped flag
let heap_item = unsafe {
ephemeron_ptr.as_ptr().cast::<ErasedHeapItem>().as_ref()
};
let heap_item = unsafe { ephemeron_ptr.as_ptr().cast::<ErasedHeapItem>().as_ref() };
!heap_item.is_dropped()
});
}
Expand All @@ -90,7 +83,7 @@ impl<K: Trace, V: Trace> ErasedWeakMap for WeakMapInner<K, V> {
// the collector owns the actual data, this is just a thin pointer
// wrapper that stays valid as long as the collector does
pub struct WeakMap<K: Trace + 'static, V: Trace + 'static> {
// raw pointer to collector owned memory
// raw pointer to collector owned memory
inner: *mut WeakMapInner<K, V>,
}

Expand All @@ -102,7 +95,8 @@ impl<K: Trace, V: Trace> WeakMap<K, V> {
// get a raw pointer that stays valid even after the box is moved
let inner: *mut WeakMapInner<K, V> = rust_alloc::boxed::Box::into_raw(boxed);
// SAFETY: we just got this from into_raw, and we are giving ownership to the collector
let erased: rust_alloc::boxed::Box<dyn ErasedWeakMap> = unsafe { rust_alloc::boxed::Box::from_raw(inner) };
let erased: rust_alloc::boxed::Box<dyn ErasedWeakMap> =
unsafe { rust_alloc::boxed::Box::from_raw(inner) };
collector.weak_maps.push(erased);
Self { inner }
}
Expand Down
25 changes: 17 additions & 8 deletions oscars/src/collectors/mark_sweep/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::{Finalize, Trace};

use crate::collectors::mark_sweep::MarkSweepGarbageCollector;
use crate::{Finalize, Trace};

use super::Gc;
use super::WeakMap;
Expand Down Expand Up @@ -60,7 +59,6 @@ fn gc_recursion() {
.with_arena_size(4096)
.with_heap_threshold(8_192);


#[derive(Debug, Finalize, Trace)]
struct S {
i: usize,
Expand All @@ -71,10 +69,13 @@ fn gc_recursion() {

let mut root = Gc::new_in(S { i: 0, next: None }, collector);
for i in 1..COUNT {
root = Gc::new_in(S {
root = Gc::new_in(
S {
i,
next: Some(root),
}, collector);
},
collector,
);
}

drop(root);
Expand All @@ -96,7 +97,7 @@ fn drop_gc() {
drop(gc);
collector.collect();

// TODO: don't drop an active arena
// TODO: don't drop an active arena
assert_eq!(collector.allocator.arenas_len(), 0, "arena not freed");
}

Expand Down Expand Up @@ -237,7 +238,11 @@ fn update_wm() {
drop(key);
collector.collect();

assert_eq!(collector.allocator.arenas_len(), 0, "arena leaked after update");
assert_eq!(
collector.allocator.arenas_len(),
0,
"arena leaked after update"
);
}

#[test]
Expand Down Expand Up @@ -330,7 +335,11 @@ fn remove_then_collect() {
drop(key);
collector.collect();

assert_eq!(collector.allocator.arenas_len(), 0, "ephemeron leaked after remove");
assert_eq!(
collector.allocator.arenas_len(),
0,
"ephemeron leaked after remove"
);
}

#[test]
Expand Down
3 changes: 1 addition & 2 deletions oscars/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ extern crate std;
#[cfg(feature = "mark_sweep")]
pub use crate::collectors::mark_sweep::*;
#[cfg(feature = "mark_sweep")]
pub use oscars_derive::{Trace, Finalize};

pub use oscars_derive::{Finalize, Trace};

pub mod alloc;
pub mod collectors;