RFR (S) 8140483: Atomic*FieldUpdaters final fields should be trusted

[Vitaly Davidovich]

FWIW I like (b) as well.  You made a GUARDED_ENTRY change in (a) - is that
a good change to make even if you go with (b)?

sent from my phone
On Oct 27, 2015 7:20 AM, "Aleksey Shipilev" <aleksey.shipilev at oracle.com>
wrote:

> Hi,
>
> I'd like to start a formal review for the change:
>    https://bugs.openjdk.java.net/browse/JDK-8140483
>
> The feedback was overwhelmingly positive so far. Some suggested we use
> compiler control and/or maintain the list of trusted classes. I still
> think it is an overkill for the fix at hand, and we should instead wait
> for the wholesale final field optimizations. Therefore, I suggest we go
> with the explicit exceptions.
>
> Now, it may take two forms:
>
>  a) Accept all non-serializable java/util/concurrent/atomic classes:
>       http://cr.openjdk.java.net/~shade/8140483/webrev.01/
>
>  b) Spell out A*FU symbol names explicitly:
>       http://cr.openjdk.java.net/~shade/8140483/webrev.02/
>
> I am leaning towards (b), because it does not involve hacking
> ciKlass::is_subtype_of, and it appears much safer from the correctness
> standpoint. Both changes pass JPRT, and both changes improve A*FU
> performance and the generated code quality.
>
> Thanks,
> -Aleksey
>
> On 10/26/2015 05:14 PM, Aleksey Shipilev wrote:
> > Hi,
> >
> > I would like to gauge interest in doing a simple VM change that
> > significantly improves the performance of Atomic*FieldUpdaters:
> >  https://bugs.openjdk.java.net/browse/JDK-8140483
> >
> > In short, we are looking into folding away the checks in A*FU:
> >
> >   private final Class<T> tclass;
> >   private final Class<?> cclass;
> >
> >   public final int get(T obj) {
> >     if (obj == null || obj.getClass() != tclass || cclass != null)
> >       fullCheck(obj);
> >     return unsafe.getIntVolatile(obj, offset);
> >   }
> >
> > Since most use cases for A*FU involve putting them into static final
> > fields, the Updater instances are known constants. Now, the internal
> > instance final fields are not trusted by default. This can be "fixed"
> > with -XX:+TrustNonStaticFinalFields -- the bad thing about this option
> > is that it's global, and it may break user code. Vladimir I. has a
> > potential improvement that handles this by tracking the final field
> > writes, but there is no clear ETA for that feature.
> >
> > With this little VM improvement, we can close the gap for A*FU, and
> > eliminate another reason for using Unsafe today:
> >  http://cr.openjdk.java.net/~shade/8140483/webrev.00/
> >
> > It improves the generated code and performance significantly:
> >  http://cr.openjdk.java.net/~shade/8140483/notes.txt
> >
> > Thanks,
> > -Aleksey
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151027/a1ca90e7/attachment-0001.html>