On constant folding of final field loads

[John Rose]

On Jul 22, 2015, at 9:14 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
> 
>> That's the sort of thing I would prefer to see, to remove
>> indeterminate behavior.
> Yes, fail-fast approach is very appealing and I like slushy bit idea, but it is more intrusive (from user perspective), unfortunately.
> 
> Though Reflection & MethodHandles can be instrument with slushy bit checks and Unsafe left as-is, what can be done for JNI?

I would prefer to treat JNI (for these use cases) the same as Unsafe, to keep our story simpler.

But it is true that we could add a few more checks to JNI than Unsafe.

> Do you think it is acceptable for JVM to throw exceptions on "illegal" (slushy bit off) final field writes?

Yes, that's the key feature of "fail fast".  The user finds out immediately.

> SetXXXField JNI functions aren't declared to throw any exceptions [1], so it seems like an intrusive change, even with JNI spec adjustments.

In some cases (in the JMM legal framework) we would be within our rights to silently nullify (ignore and discard) illegal stores to final fields (in any API).  Nullification is indistinguishable from the store occurring but never being observed in a future read.  This is possible if either the store is delayed indefinitely, or if all threads (and compiled methods) have previously performed a caching read of the original final value.  Of course, the threads would not have physically done so, but perhaps the JMM could be tortured to allow some sort of OOTA caching read of the original final value.

But silent nullification is a desperation move.  Users deserve an exception or other signal so they can fix their code quickly.

> Thinking more about "slushiness", limiting the time an object is in that state doesn't look like an easy problem to solve, considering there are 3 interacting operations (instantiate, initialize, freeze).

The important thing is to get library writers to "put a lid" on the slushy state by adding an explicit freeze operation.  A mixture of performance and robustness concerns would motivate most of them to get with the program.  But the non-compliant libraries would, as you say, leak slushy objects.

Normally allocated objects should not have the slushy bit set, except (perhaps, but probably not) when they escape during construction.

> VM can do some analysis to ensure slushy objects don't escape, but it looks either too fragile or too complex to implement.

Agree.

> I don't see a big problem with "runaway" objects with slushy bit on. It means JIT will be always conservative when working with them. Or are you mostly concerned about abuse of slushy objects?

A slushy object won't optimize fully (in all cases).  And its surprising mutability provides a little window for bugs to sneak in.  It would be on library writers to fix this.  I suppose we could also add an operation to assert frozen-ness, either inside the JIT or explicitly as a tool for users.  In some cases, a JIT could generate good code optimistically for properly frozen objects, and de-opt when it runs into a stray slushy object.  If the assertion were at the user level, it could fail with an exception; this isn't exactly fail-fast since the bad operation that leaked the slushy object would be long gone.

> It can be mitigated by:
>  (1) requiring a user to perform additional actions to set slushy bit (e.g. calling specialized newInstance() equivalent or marking caller method akin to caller-sensitive methods); in that case it is less likely a user won't freeze previously allocated object;

(Even better than annotations would be split types, a la the verifier.  But the JVM does not have a highly refined type system.)

Any hack that creates a blank instance (Unsafe.newInstance, JNIEnv.AllocObject) without also running a constructor can and should set the slushy bit.  This is something we can control, ultimately.

>  (2) providing VM diagnostics to detect runaway slushy objects;

(Yes, such as a user-visible assertion.)

> In the end, it is expert level API. I don't think many people write their own deserialization frameworks :-) If you bungle it, you are on your own.

And if you own a deserialization framework, you need to read the news and update your code.

> …
> Separate tag has the same shortcoming as high-tag: it is not translated into a single machine instruction. VM needs to inspect the tag before performing a field update, though original setXXX() methods aren't affected.
> 
> Considering fail-fast approach and slushy bits, I'm inclined to leave Unsafe as is (no tag approach). Probably, adding diagnostic mode to VM signalling when a final field is updated using Unsafe.

That sounds pretty good.

One caveat:  Unsafe and automagic behavior (even in debug mode) don't usually go together.

An alternative to automagic mode would be a *separate* Unsafe API to enable library writers to make the check (in their own debug mode).  Something that would decode the (Object,long) pair into a java.lang.reflect.Field (or similar name), which could then be checked for finality.  Plus the aforementioned "assertSlushy" operation.

— John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150722/163880d0/attachment-0001.html>