Skip to content

Commit

Permalink
[simple] Used relaxed atomic for should_collect checks at a safepoint
Browse files Browse the repository at this point in the history
This should be easier for a JIT to implement since it doesn't require
any fences. See issue #12
As of this writing, cranelift doesn't even support atomic fences.

It's also could be faster on ARM architectures. Seems important
since it's such a hot-path.....
  • Loading branch information
Techcable committed Jun 17, 2020
1 parent 9ad529d commit b6677ef
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
2 changes: 1 addition & 1 deletion libs/simple/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ unsafe impl GcContext for SimpleCollectorContext {
debug_assert_eq!(self.raw.state.get(), ContextState::Active);
let dyn_ptr = (*self.raw.shadow_stack.get())
.push(value);
if self.raw.collector.should_collect() {
if self.raw.collector.should_collect_relaxed() {
self.raw.trigger_safepoint();
}
debug_assert_eq!(self.raw.state.get(), ContextState::Active);
Expand Down
25 changes: 16 additions & 9 deletions libs/simple/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,20 @@ impl GcHeap {
}
}
#[inline]
fn should_collect(&self) -> bool {
fn should_collect_relaxed(&self) -> bool {
/*
* Going with relaxed ordering because it's not essential
* that we see updates immediately.
* Eventual consistency should be enough to eventually
* trigger a collection.
*
* This is much cheaper on ARM (since it avoids a fence)
* and is much easier to use with a JIT.
* A JIT will insert these very often, so it's important to make
* them fast!
*/
self.allocator.allocated_size.load(Ordering::Acquire)
>= self.threshold.load(Ordering::Acquire)
self.allocator.allocated_size.load(Ordering::Relaxed)
>= self.threshold.load(Ordering::Relaxed)
}
}

Expand Down Expand Up @@ -520,14 +525,16 @@ struct RawSimpleCollector {
}
impl RawSimpleCollector {
#[inline]
fn should_collect(&self) -> bool {
fn should_collect_relaxed(&self) -> bool {
/*
* TODO: Consider relaxed ordering
* It'd be cheaper on ARM but potentially
* delay collection.....
* Use relaxed ordering, just like `should_collect`
*
* This is faster on ARM since it avoids a memory fence.
* More importantly this is easier for a JIT to implement inline!!!
* As of this writing cranelift doesn't even seem to support fences :o
*/
self.heap.should_collect() || self.collecting
.load(Ordering::Acquire)
self.heap.should_collect_relaxed() || self.collecting
.load(Ordering::Relaxed)
}
#[cold]
#[inline(never)]
Expand Down

0 comments on commit b6677ef

Please sign in to comment.