Odd interaction between ArrayList$Itr and Escape Analysis
Krystal Mok
rednaxelafx at gmail.com
Wed Sep 28 15:37:19 UTC 2016
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> 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>> wrote:
>>
>>
>>
>> On Tuesday, September 13, 2016, Remi Forax <forax at univ-mlv.fr
>> <javascript:_e(%7B%7D,'cvml','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-expert
>> s/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>
>> *À: *"Krystal Mok" <rednaxelafx at gmail.com>
>> *Cc: *"hotspot compiler" <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> wrote:
>>
>> On Mon, Sep 12, 2016 at 12:38 PM, Vitaly Davidovich
>> <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
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160928/514eb371/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list