Skip to content

Commit 06237f1

Browse files
authored
Generational plans set next_gc_full_heap in end_of_gc (#714)
This PR adds `Plan.end_of_gc()`. Our generational plans compute whether we should do full heap GC at the end of `Plan.release()` which is incorrect. There could be other parallel release work when we execute `Plan.release()`. This PR adds `Plan.end_of_gc()`, and we compute `next_gc_full_heap` in `end_of_gc()`.
1 parent 4bb37f8 commit 06237f1

File tree

4 files changed

+17
-13
lines changed

4 files changed

+17
-13
lines changed

src/plan/generational/copying/global.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,9 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {
123123
if full_heap {
124124
self.fromspace().release();
125125
}
126+
}
126127

127-
// TODO: Refactor so that we set the next_gc_full_heap in gen.release(). Currently have to fight with Rust borrow checker
128-
// NOTE: We have to take care that the `Gen::should_next_gc_be_full_heap()` function is
129-
// called _after_ all spaces have been released (including ones in `gen`) as otherwise we
130-
// may get incorrect results since the function uses values such as available pages that
131-
// will change dependant on which spaces have been released
128+
fn end_of_gc(&mut self, _tls: VMWorkerThread) {
132129
self.gen
133130
.set_next_gc_full_heap(Gen::should_next_gc_be_full_heap(self));
134131
}

src/plan/generational/immix/global.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,9 @@ impl<VM: VMBinding> Plan for GenImmix<VM> {
165165
}
166166
self.last_gc_was_full_heap
167167
.store(full_heap, Ordering::Relaxed);
168+
}
168169

169-
// TODO: Refactor so that we set the next_gc_full_heap in gen.release(). Currently have to fight with Rust borrow checker
170-
// NOTE: We have to take care that the `Gen::should_next_gc_be_full_heap()` function is
171-
// called _after_ all spaces have been released (including ones in `gen`) as otherwise we
172-
// may get incorrect results since the function uses values such as available pages that
173-
// will change dependant on which spaces have been released
170+
fn end_of_gc(&mut self, _tls: VMWorkerThread) {
174171
self.gen
175172
.set_next_gc_full_heap(Gen::should_next_gc_be_full_heap(self));
176173
}

src/plan/global.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,22 @@ pub trait Plan: 'static + Sync + Downcast {
221221
}
222222

223223
/// Prepare the plan before a GC. This is invoked in an initial step in the GC.
224-
/// This is invoked once per GC by one worker thread. 'tls' is the worker thread that executes this method.
224+
/// This is invoked once per GC by one worker thread. `tls` is the worker thread that executes this method.
225225
fn prepare(&mut self, tls: VMWorkerThread);
226226

227227
/// Prepare a worker for a GC. Each worker has its own prepare method. This hook is for plan-specific
228228
/// per-worker preparation. This method is invoked once per worker by the worker thread passed as the argument.
229229
fn prepare_worker(&self, _worker: &mut GCWorker<Self::VM>) {}
230230

231-
/// Release the plan after a GC. This is invoked at the end of a GC when most GC work is finished.
232-
/// This is invoked once per GC by one worker thread. 'tls' is the worker thread that executes this method.
231+
/// Release the plan after transitive closure. A plan can implement this method to call each policy's release,
232+
/// or create any work packet that should be done in release.
233+
/// This is invoked once per GC by one worker thread. `tls` is the worker thread that executes this method.
233234
fn release(&mut self, tls: VMWorkerThread);
234235

236+
/// Inform the plan about the end of a GC. It is guaranteed that there is no further work for this GC.
237+
/// This is invoked once per GC by one worker thread. `tls` is the worker thread that executes this method.
238+
fn end_of_gc(&mut self, _tls: VMWorkerThread) {}
239+
235240
/// Ask the plan if they would trigger a GC. If MMTk is in charge of triggering GCs, this method is called
236241
/// periodically during allocation. However, MMTk may delegate the GC triggering decision to the runtime,
237242
/// in which case, this method may not be called. This method returns true to trigger a collection.

src/scheduler/gc_work.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@ impl<VM: VMBinding> GCWork<VM> for EndOfGC {
223223
fn do_work(&mut self, worker: &mut GCWorker<VM>, mmtk: &'static MMTK<VM>) {
224224
info!("End of GC");
225225

226+
// We assume this is the only running work packet that accesses plan at the point of execution
227+
#[allow(clippy::cast_ref_to_mut)]
228+
let plan_mut: &mut dyn Plan<VM = VM> = unsafe { &mut *(&*mmtk.plan as *const _ as *mut _) };
229+
plan_mut.end_of_gc(worker.tls);
230+
226231
#[cfg(feature = "extreme_assertions")]
227232
if crate::util::edge_logger::should_check_duplicate_edges(&*mmtk.plan) {
228233
// reset the logging info at the end of each GC

0 commit comments

Comments
 (0)