<p dir="ltr">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)?</p>
<p dir="ltr">sent from my phone</p>
<div class="gmail_quote">On Oct 27, 2015 7:20 AM, "Aleksey Shipilev" <<a href="mailto:aleksey.shipilev@oracle.com">aleksey.shipilev@oracle.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
I'd like to start a formal review for the change:<br>
   <a href="https://bugs.openjdk.java.net/browse/JDK-8140483" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8140483</a><br>
<br>
The feedback was overwhelmingly positive so far. Some suggested we use<br>
compiler control and/or maintain the list of trusted classes. I still<br>
think it is an overkill for the fix at hand, and we should instead wait<br>
for the wholesale final field optimizations. Therefore, I suggest we go<br>
with the explicit exceptions.<br>
<br>
Now, it may take two forms:<br>
<br>
 a) Accept all non-serializable java/util/concurrent/atomic classes:<br>
      <a href="http://cr.openjdk.java.net/~shade/8140483/webrev.01/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~shade/8140483/webrev.01/</a><br>
<br>
 b) Spell out A*FU symbol names explicitly:<br>
      <a href="http://cr.openjdk.java.net/~shade/8140483/webrev.02/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~shade/8140483/webrev.02/</a><br>
<br>
I am leaning towards (b), because it does not involve hacking<br>
ciKlass::is_subtype_of, and it appears much safer from the correctness<br>
standpoint. Both changes pass JPRT, and both changes improve A*FU<br>
performance and the generated code quality.<br>
<br>
Thanks,<br>
-Aleksey<br>
<br>
On 10/26/2015 05:14 PM, Aleksey Shipilev wrote:<br>
> Hi,<br>
><br>
> I would like to gauge interest in doing a simple VM change that<br>
> significantly improves the performance of Atomic*FieldUpdaters:<br>
>  <a href="https://bugs.openjdk.java.net/browse/JDK-8140483" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8140483</a><br>
><br>
> In short, we are looking into folding away the checks in A*FU:<br>
><br>
>   private final Class<T> tclass;<br>
>   private final Class<?> cclass;<br>
><br>
>   public final int get(T obj) {<br>
>     if (obj == null || obj.getClass() != tclass || cclass != null)<br>
>       fullCheck(obj);<br>
>     return unsafe.getIntVolatile(obj, offset);<br>
>   }<br>
><br>
> Since most use cases for A*FU involve putting them into static final<br>
> fields, the Updater instances are known constants. Now, the internal<br>
> instance final fields are not trusted by default. This can be "fixed"<br>
> with -XX:+TrustNonStaticFinalFields -- the bad thing about this option<br>
> is that it's global, and it may break user code. Vladimir I. has a<br>
> potential improvement that handles this by tracking the final field<br>
> writes, but there is no clear ETA for that feature.<br>
><br>
> With this little VM improvement, we can close the gap for A*FU, and<br>
> eliminate another reason for using Unsafe today:<br>
>  <a href="http://cr.openjdk.java.net/~shade/8140483/webrev.00/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~shade/8140483/webrev.00/</a><br>
><br>
> It improves the generated code and performance significantly:<br>
>  <a href="http://cr.openjdk.java.net/~shade/8140483/notes.txt" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~shade/8140483/notes.txt</a><br>
><br>
> Thanks,<br>
> -Aleksey<br>
><br>
<br>
<br>
</blockquote></div>