RFC: Revisiting ShenandoahOptimize*Finals flags

[Roman Kennke]

Hello all,
I have an outstanding patch to improve ShenandoahOptimizeStableFinals,
but I am not sure if it's worth pushing this, or if we might want to rip
those flags out altogether. While I like those optimizations, I am not
sure if they are actually legal wrt the JMM anymore.

It's obviously easy once an object is constructed: the field/array
element doesn't change, and initializers are employing a mem-fence and
therefore doesn't need a read-barrier.

The tricky part is when an object is under initialization. Consider this
code:

public class A {
  private final X field;
  public A() {
    field = makeX();
    if (field == foo) {
      ..
    } else {
      ..
    }
  }
}

let's assume makeX() generates one of two instances of X (e.g. depending
on some outside condition), and the following if-else does different
actions based on which X we got. the assignment to field used a WB
because we don't elide them. However, the  getfield of field in the if
statement would have its RB elided, and therefore might see an
uninitialized value (null) and go the wrong way in the if-else.

This example would most likely actually work, because the getfield would
likely use the WB'ed object. However, 1. we cannot prove this 2. the
actual code might be more complex, e.g. the 'this' might escape the
initializer and some other code does the if-else, etc.

So far we assumed those optimizations are dipping into undefined
behaviour of the JMM, but unless I get proven wrong, the above example
makes me think it might lead to JMM violating behaviour instead.

Also, we enable ShenandoahOptimizeStaticFinals by default, but not
ShOptInstanceFinals and ShOptStableFinals. How is StaticFinals any
better than InstanceFinals? Exchange <init> with <cinit> and the
instance with the Class object, it's pretty much the same problem, and
not under our control at all. The only case that I can see somewhat ok
is StableFinals because this is under our control: if we ever see bad
behaviour like described above, we could fix it.

OTOH, static finals would often compile to constants anyway, and we
elide barriers on constants altogether. Even instance and stable finals
might compile to constants through constant propagation. I suspect this
is a reason why those flags don't have much of an effect in most benchmarks.

What should we do? Get rid of those flags altogether?

WDYT?

Cheers,
Roman