RFR: Some patches for sherman

[John Rose]

In short, Peter is right; sorry Martin.

That annotation is private to java.base and not currently a feature that
has support beyond its uses in java.base.  The purpose of @Stable is
to allow lazily evaluated variables (both static and non-static) to be
constant-folded like static finals.  In the case of a non-static @Stable
variable, the containing reference must first constant-fold in order for
the @Stable field to fold.  (The rules for arrays are even trickier.)

The optimizations for @Stable are not aimed at proving properties
of @Stable fields inside non-constant objects (or non-c. arrays).
But you are right, Martin, that the rules for @Stable would allow
the optimization you are asking for.  If we ever do lazy evaluation
as a first class language feature (I can dream), we'll want both
kinds of optimizations to be routine.

BTW, note that the rules of @Stable do not guarantee stability
of "z.res" unless "z.res" has been observed to be non-null in
a dominating code path.  As long as it has its default value
of null, the variable not yet guaranteed to be constant.  To
optimize this correctly we would probably need loop predication
(or perhaps peeling) to optimistically sample z.res and ensure
that it is non-null before going into the real loop.  I see that
"z.res" is a tuple of some sort, so it's likely that normal loop
code will null-check it implicitly, which is a start.  If the loop
peels for other reasons, then the stable invariant property
would fall out "for free".  But as I said, the JIT is not looking
for @Stable fields except inside of constant (not just
loop-invariant) objects.

I don't recommend using @Stable except as a way to extend
the reach of constant folding.  Proving other properties like
loop invariance isn't on the list yet.

The docs for @Stable don't spell this out as clearly as one
might want, but this sentence comes closest:  "More specifically,
the HotSpot VM will process non-null stable fields (final or
otherwise) in a similar manner to static final fields with respect to
promoting the field's value to a constant."

— John

On Mar 30, 2018, at 3:49 PM, Martin Buchholz <martinrb at google.com> wrote:
> 
> The qualified people may not be reading this mailing list.  Adding authors/reviewers of Stable.java.  The question is whether in 
> 
> 
> final ZipFile z = ...
> while (...) {
>   use(z.res); 
> }
> 
> 
> annotating instance field ZipFile.res as @Stable will enable the optimization of reading the res field only once.
> 
> 
> On Fri, Mar 30, 2018 at 1:16 AM, Peter Levart <peter.levart at gmail.com <mailto:peter.levart at gmail.com>> wrote:
> 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 <mailto:peter.levart at gmail.com>>
> To:     Martin Buchholz <martinrb at google.com <mailto:martinrb at google.com>>
> 
> 
> 
> Hi Martin,
> 
> On Fri, 30 Mar 2018, 05:28 Martin Buchholz, <martinrb at google.com <mailto:martinrb at google.com> <mailto: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> <mailto: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/~martin/webrevs/jdk/zip-cleanup/>
>         <http://cr.openjdk.java.net/%7Emartin/webrevs/jdk/zip-cleanup/ <http://cr.openjdk.java.net/%7Emartin/webrevs/jdk/zip-cleanup/>>
>         https://bugs.openjdk.java.net/browse/JDK-8200124 <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
> 
> 
> 
> 
>