Fix stop-the-world race.

[wks]

With the associated commit in the mmtk-openjdk repository, stop-the-world and start-the-world requests no longer have to be made in the MMTk coordinator thread.

NOTE: This change will break the current mmtk-openjdk binding, because the VM will raise errors if the thread that starts the world is not the thread that stopped the world. So mmtk-openjdk should only update its dependency to this revision at the same time it applied its corresponding PR for the stop-the-world race fix.

[qinsoon]

Is the change necessary for the OpenJDK binding change? I feel it probably would work fine without this PR. Though it may not be necessary to stop/resume from the MMTk coordinator thread for OpenJDK, this is still a nice property to have for mmtk-core, as 1. we do have a designated coordinator thread, 2. other VMs may require something like this.

[wks]

@qinsoon It is not necessary. Without this change, the mmtk-openjdk binding will still work, but will introduce one extra context switch.

There is an alternative solution, i.e. making it configurable in mmtk-core (an MMTK_XXXXXX option should be fast enough) whether stop-the-world needs to be triggered by the coordinator thread. By default, it can be done by any thread. In this way, OpenJDK can elide one context switch, and other VMs may configure it differently.

[qinsoon]

There is an alternative solution, i.e. making it configurable in mmtk-core (an MMTK_XXXXXX option should be fast enough) whether stop-the-world needs to be triggered by the coordinator thread. By default, it can be done by any thread. In this way, OpenJDK can elide one context switch, and other VMs may configure it differently.

I think this sounds good. Can you add that in this PR? It probably can be a build-time feature.

[wks]

@qinsoon It could be a compile-time feature (as in "Cargo feature"). However, I think it should be set per-instance. There could be two different VMs in one process, and they have different requirements.

Instead, I added a const COORDINATOR_ONLY_STW: bool = true to VMCollection so that the VM binding can decide. It is still a compile-time constant. It defaults to true, so the behaviour is not changed and existing VM bindings will work like before.

I will not change my PR in mmtk-openjdk before this is merged because mmtk-openjdk/mmtk/Cargo.toml depends on a repo and a commit hash. I tested locally, and the following patch for mmtk-openjdk makes use of this new feature.

diff --git a/mmtk/src/collection.rs b/mmtk/src/collection.rs
index 81b6326..2e3f6f4 100644
--- a/mmtk/src/collection.rs
+++ b/mmtk/src/collection.rs
@@ -20,6 +20,8 @@ extern "C" fn create_mutator_scan_work<E: ProcessEdgesWork<VM = OpenJDK>>(
 }
 
 impl Collection<OpenJDK> for VMCollection {
+    const COORDINATOR_ONLY_STW: bool = false;
+
     fn stop_all_mutators<E: ProcessEdgesWork<VM = OpenJDK>>(tls: VMWorkerThread) {
         let f = {
             if <OpenJDK as VMBinding>::VMScanning::SCAN_MUTATORS_IN_SAFEPOINT {

[qinsoon]

@wks I think this PR is good to merge. If you dont have any other change, I will merge this (first mmtk-core and the openjdk fork, then the openjdk binding).

[wks]

@qinsoon Nope. That's all I am going to change for the three repositories.