Simplify MMTk--VM glue code

Moving all unscheduled issues to 3.1. Please close or retarget to a different fix target as appropriate.

Moving to 3.2

Hi Daniel,

Looking through ControllerCollectorContext.java I noticed some unsafe optimisations with respect to the Java Memory Model, the issues that caught my eye are highlighted by a diff below

Kind regards
Laurence

diff --git a/work/MMTk/src/org/mmtk/plan/ControllerCollectorContext.java b/work/MMTk/src/org/mmtk/plan/ControllerCollectorContext.java
index fd0f1de..3821695 100644
— a/work/MMTk/src/org/mmtk/plan/ControllerCollectorContext.java
+++ b/work/MMTk/src/org/mmtk/plan/ControllerCollectorContext.java
@@ -39,7 +39,8 @@ public class ControllerCollectorContext extends CollectorContext {
private int lastRequestCount = -1;

/** Is there concurrent collection activity */

• private boolean concurrentCollection = false;
  + private volatile boolean concurrentCollection = false; // Can be changed mid way through run() via workers.triggerCycle(),
  + // compiler might cache stale value

/**

• Create a controller context.
  @@ -140,9 +141,9 @@ public class ControllerCollectorContext extends CollectorContext {
• Request a collection.
  */
  public void request() {

• if (requestFlag) { - return; - }

  + // if (requestFlag)

  { // unsafe optimisation, requestFlag must be volatile or only read with lock held, otherwise thread might + // return; // see stale value + // }

  lock.lock();
  if (!requestFlag) {
  requestFlag = true;

I am happy to do some work to clean up this code, but I don't think you have pointed to any 'real' problems (though I have to admit I have not really paged back in this code)

1. For concurrentCollection, there is synchronization between the controller and gc threads, and the value isn't allowed to be cached across the lock/unlock sequences.

2. For requestFlag, the check is safe because there is stronger synchronization required for the value change back from true to false---any thread that is out of date will see the stale (false) value but find the real value after acquiring the lock which is safe.

One thing we rely on here for both cases is that the compilers treat the lock/unlock calls as barriers for code motion. If lock() did not prevent the read of requestFlag from moving up before the lock() then things could get quite nasty!