Odd interaction between ArrayList$Itr and Escape Analysis
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Wed Sep 28 16:17:48 UTC 2016
Kris, thanks for sharing the patch!
IMO the problem we observe is not specific to bridge methods.
It demonstrates a generic short-coming in C2 inlining heuristic: even
though the argument is never used (otherwise, the class would have been
already loaded, right?), we don't inline the whole method.
So, I'd prefer to see a solution which covers the general case.
Can we do that? It seems so: it could be achieved by a null guard on the
argument or a nmethod dependency on the unloaded class.
Best regards,
Vladimir Ivanov
On 9/28/16 6:37 PM, Krystal Mok wrote:
> Hi guys,
>
> Here's the HotSpot-side patch, based on OpenJDK9 HotSpot:
>
> Webrev: http://cr.openjdk.java.net/~kmo/8166840/webrev.00/
>
> Please give me a preliminary idea of how you guys feel about the patch,
> and then I'll start an actual review thread if people agree on the
> direction of this patch.
>
> Note: This is the way javac constructs that "XXX$1" name for the
> accessConstructorTag:
> JDK7u: http://hg.openjdk.java.net/jdk7u/jdk7u/langtools/file/93a2788178e6/src/share/classes/com/sun/tools/javac/comp/Lower.java#l1154
> JDK9:
> http://hg.openjdk.java.net/jdk9/jdk9/langtools/file/9f61004270d8/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java#l1241
>
> So name matching on "$1" suffix is sufficient here to workaround this
> particular pattern from javac.
>
> P.S. I haven't built OpenJDK9 in quite a while now, and apparently the
> makefiles have changed and the scripts that I used to build JDK7u /
> JDK8u doesn't work on JDK9. What's the current recommended way to build
> just HotSpot with fastdebug / product levels?
>
> Thanks,
> Kris (OpenJDK username: kmo)
>
> On Wed, Sep 14, 2016 at 3:12 AM, Vladimir Ivanov
> <vladimir.x.ivanov at oracle.com <mailto:vladimir.x.ivanov at oracle.com>> wrote:
>
> Kris,
>
> And I'm happy to upstream that patch, if the team is interested.
>
>
> Sure, we are definitely interested in fixing that. Feel free to file
> a bug and send the fix out for review.
>
> Now, when I first discovered the problem, my first intuition was
> that
> it's better to "fix" it in javac. But before nest mates in the Class
> file, there isn't much that javac could do. Changing the Java
> libraries
> to not use private constructors in inner classes is also doable, but
> needs changing a lot of files.
>
>
> I agree that javac is not the best place to fix the immediate
> problem: it requires recompilation and there are already lots of
> problematic bytecode shapes out in the wild. The JVM should optimize
> for that case instead.
>
> So I ended up fixing it in the VM, even though I agree fully
> with what
> Rémi brought up.
>
>
> I'm curious how did you fix it. I haven't found a description in the
> thread.
>
> It's possible to force class loading, but I'm worried about
> undesirable effects of class initialization. Is it enough for C2 to
> have the class loaded but not initialized to make it work?
>
> Another approach would be to issue a null check and deoptimize (for
> bridge methods, the check collapses after inlining since the
> argument is always null) or add a nmethod dependency and throw away
> the code when the parameter class is loaded.
>
> Best regards,
> Vladimir Ivanov
>
> The access constructor tag thingy in javac is really a weird
> hack. If
> you guys ever look at the contents of ArrayList$1, it's really empty
> -- the class doesn't even declare some of the usual structures in a
> normal Class file... Hopefully we can get rid of it in javac soon.
>
>
> On Tuesday, September 13, 2016, Vitaly Davidovich
> <vitalyd at gmail.com <mailto:vitalyd at gmail.com>
> <mailto:vitalyd at gmail.com <mailto:vitalyd at gmail.com>>> wrote:
>
>
>
> On Tuesday, September 13, 2016, Remi Forax
> <forax at univ-mlv.fr <mailto:forax at univ-mlv.fr>
> <javascript:_e(%7B%7D,'cvml','forax at univ-mlv.fr
> <mailto:forax at univ-mlv.fr>');>> wrote:
>
> I've always found that the empty inner classes generated by
> javac as a kind of hack.
>
> These classes should be removed in Java 10, thanks to the
> nestmate attributes.
>
>
> http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2016-January/000060.html
> <http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2016-January/000060.html>
>
> <http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2016-January/000060.html
> <http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2016-January/000060.html>>
>
> The other solution, is to have an empty class in the jdk
> which
> is not visible from javac (the class itself can be marked as
> synthetic),
> so javac can use it without creating method clash.
>
> and to solve the problem now, the easy solution is to add a
> package private constructor in ArrayList.Itr,
>
> I'm hoping Oracle can take Kris' (Azul) patch (or do something
> similar). It might catch more cases than just modifying Itr.
>
>
> private class Itr implements Iterator<E> {
> int cursor; // index of next element to return
> int lastRet = -1; // index of last element returned; -1
> if no such
> int expectedModCount = modCount;
>
> Itr() {
> // avoid to generate a synthetic accessor constructor
> }
> }
>
>
> regards,
> Rémi
>
>
> ------------------------------------------------------------------------
>
> *De: *"Vitaly Davidovich" <vitalyd at gmail.com
> <mailto:vitalyd at gmail.com>>
> *À: *"Krystal Mok" <rednaxelafx at gmail.com
> <mailto:rednaxelafx at gmail.com>>
> *Cc: *"hotspot compiler"
> <hotspot-compiler-dev at openjdk.java.net
> <mailto:hotspot-compiler-dev at openjdk.java.net>>
> *Envoyé: *Lundi 12 Septembre 2016 22:15:41
> *Objet: *Re: Odd interaction between ArrayList$Itr and
>
> Escape Analysis
>
>
>
> On Mon, Sep 12, 2016 at 3:56 PM, Krystal Mok
> <rednaxelafx at gmail.com
> <mailto:rednaxelafx at gmail.com>> wrote:
>
> On Mon, Sep 12, 2016 at 12:38 PM, Vitaly Davidovich
> <vitalyd at gmail.com <mailto:vitalyd at gmail.com>>
> wrote:
>
> It seems odd to me as well why inlining
> won't force
> load the missing class(es). If we're
> inlining, it
> means the method itself or the call chain
> it's part
> of is hot - failing to inline can have negative
> side-effects, like this example. I suppose
> there
> must be a good reason why it doesn't do this
> though?
>
>
> That's because we can't. The JIT compilers are
> running
> on their own threads, and they're not real "Java
> threads". So they are not allowed to run
> arbitrary Java
> code. But Java class loading may involve running
> arbitrary Java code, e.g. the
> ClassLoader.loadClass()
> upcall.
> Force class loading can be done on the
> triggering side
> (for the top-level method), because compilation
> tasks
> are triggered from real Java threads, and they're
> allowed to run arbitrary Java code.
>
> I see, makes sense. Perhaps there can be an option
> to turn
> on loading of required types in the entire
> compilation unit,
> after all inlining is done (and therefore make the
> unloaded
> types not be barriers for inlining). I'd personally
> prefer
> that over having odd performance differences.
>
>
> - Kris
>
> Thanks Kris.
>
>
>
> --
> Sent from my phone
>
>
More information about the hotspot-compiler-dev
mailing list