mmtk / mmtk-core

Memory Management ToolKit
https://www.mmtk.io
Other
374 stars 68 forks source link

Provide callbacks into VM binding for mutator prepare/release? #1017

Open k-sareen opened 11 months ago

k-sareen commented 11 months ago

We've recently added the BumpPointer struct so that bindings can embed just the cursor and limit into their TLS instead of embedding the very large Mutator struct (which may not always be possible). However, there is a subtle pitfall if we use this naively. The pitfall is described in detail here. The tl;dr is that all mutators' cached cursor and limit need to be reset, not just the mutator which failed the allocation.

This begs the question if we should provide an explicit API that is called for mutator prepare/release so that the VM can update internal state? We used to have VMCollection::prepare_mutator to allow for VM-specific behaviour, however its semantics were unclear and it was removed in https://github.com/mmtk/mmtk-core/pull/875.

qinsoon commented 11 months ago

If we add back the API prepare_mutator, we should see if the code in JikesRVM works fine with the API: https://github.com/mmtk/mmtk-jikesrvm/blob/c08d421ead1ce734c90ba9d80ae2c74f13eeec41/mmtk/src/collection.rs#L30

I remembered that in https://github.com/mmtk/mmtk-core/pull/875, I attempted to move the call VMCollection::prepare_mutator to the PrepareMutator packet, and it breaks JikesRVM (the only binding that uses prepare_mutator) immediately. https://github.com/mmtk/mmtk-core/blob/d253d97d96766b31fd0014b6c98ec34728cb62ee/src/scheduler/gc_work.rs#L91

k-sareen commented 11 months ago

One subtle thing is that we want to call rt_release_mutator on the runtime's mutator thread as opposed to MMTk's Mutator struct (as the embedded bump pointer is inside the VMMutatorThread). More concretely, we want:

fn rt_release_mutator(tls: VMMutatorThread);

instead of

fn rt_release_mutator<VM>(tls: &mut Mutator<VM>);