Fwd: Re: RFR: Some patches for sherman

[Peter Levart]

I'm forwarding this private message to core-libs-dev so a qualified 
person may read it...

Regards, Peter


-------- Forwarded Message --------
Subject: 	Re: RFR: Some patches for sherman
Date: 	Fri, 30 Mar 2018 06:18:26 +0000
From: 	Peter Levart <peter.levart at gmail.com>
To: 	Martin Buchholz <martinrb at google.com>



Hi Martin,

On Fri, 30 Mar 2018, 05:28 Martin Buchholz, <martinrb at google.com 
<mailto:martinrb at google.com>> wrote:



    On Thu, Mar 29, 2018 at 7:57 AM, Peter Levart
    <peter.levart at gmail.com <mailto:peter.levart at gmail.com>> wrote:

        Hi Martin,

        On 03/27/2018 02:21 AM, Martin Buchholz wrote:
>         8200124: Various cleanups in jar/zip
>         http://cr.openjdk.java.net/~martin/webrevs/jdk/zip-cleanup/
>         <http://cr.openjdk.java.net/%7Emartin/webrevs/jdk/zip-cleanup/>
>         https://bugs.openjdk.java.net/browse/JDK-8200124

        As I already mentioned for your Thread cleanup patch, do you
        think that ZipFile objects are something that ever gets assigned
        to static final or @Stable fields? Only in that case would
        @Stable have any effect for ZipFile fields. @Stable should
        really be reserved for classes which instances are normally
        assigned to static final fields, such as ThreadLocal,
        ClassValue, etc... If the holder of the @Stable field can not be
        made constant, then neither can the value of that field.


    Peter, I was very surprised by your reply.  I re-read the doc for
    Stable and it says to me that in

    ZipFile z = ...

    while (...) {
       z.res ...
    }

    z.res only needs to be read once.


This is true even without @Stable on res field. And JIT already does all 
it can without it. What JIT cannot do in above situation is make the 
value of res a compile-time constant embedded in generated code, 
possibly triggering other optimizations that arrise from the value of 
the field. And the reason is not the absense of @Stable but the fact 
that ZipFile instance can not be prooved to be constant.


    If hotspot only optimizes @Stable for static fields, I would regard
    that as a bug.
    Only being able to optimize static fields is not very useful.


It optimizes (constant-folds) @Stable static fields and @Stable instance 
fields for which it can proove that the holder object can not change 
during the lifetime of generated code. In practice this means that there 
has to be a chain of @Stable fields leading to optimized value, rooted 
at some static final or static @Stable field.

Maybe the same optimization could also be applied for locals that are 
accessed in a loop for which the JIT could proove that it never ends or 
if it ends that the generated code is deoptimized. But I doubt this is 
what current JIT does. In your ZipFile case this would mean JIT would be 
generating separate code blob for each ZipFile instance.

All above is just a speculation and how I understand the @ Stable 
amnotation. Someone more qualified could chime-in clear things a bit.

Regards, Peter