gmane.comp.java.openjdk.hotspot.compiler.devel

will hotspot refuse to compile huge functions

Andy Nuss — 2013-05-23T05:46:17

I have a very big function that's hard to refactor, almost 500 lines of sparsely commented code.  Hard to believe, but its a complex finite automata.  Will hotspot compile it?

hg: hsx/hotspot-comp/hotspot: 2 new changesets

2013-05-23T03:13:27

Changeset: 1682bec79205
Author:    kvn
Date:      2013-05-22 09:02 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/1682bec79205

8014811: loopTransform.cpp assert(cmp_end->in(2) == limit) failed
Summary: Stop current iteration of loop opts if partial_peel() failed and it created node clones outside processed loop.
Reviewed-by: roland

! src/share/vm/opto/loopnode.hpp
! src/share/vm/opto/loopopts.cpp

Changeset: 71a2d06b9c2b
Author:    kvn
Date:      2013-05-22 17:39 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/71a2d06b9c2b

Merge

hg: hsx/hotspot-comp/hotspot: 2 new changesets

2013-05-23T00:34:02

Changeset: ec922e5c545a
Author:    anoll
Date:      2013-05-22 10:28 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/ec922e5c545a

8012312: hsdis fails to compile with binutils-2.23.2
Summary: added <config.h> to header file to make hsdis compile with binutils 2.23.*
Reviewed-by: kvn, twisti

! src/share/tools/hsdis/hsdis.c

Changeset: b4907b24ed48
Author:    twisti
Date:      2013-05-22 11:44 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/b4907b24ed48

Merge

Re: RFR (XS): JDK-8013496: Code cache management command line optionswork only in special order. Another order of arguments does notdeliver the second parameter to the jvm.

Vladimir Kozlov — 2013-05-22T22:17:46

"if diff 1.out 2.out >$NULL" may not work on all platforms.
Could you do it similar to test/compiler/5091921/Test6890943.sh ?

Thanks,
Vladimir

On 5/22/13 2:21 PM, Albert Noll wrote:

Re: RFR (XS): JDK-8013496: Code cache management command line optionswork only in special order. Another order of arguments does notdeliver the second parameter to the jvm.

Albert Noll — 2013-05-22T21:21:12

Hi,

I added a test that checks that the patch works.

Many thanks in advance for reviewing!

webrev: https://jbs.oracle.com/bugs/browse/JDK-8013496
jbs: http://cr.openjdk.java.net/~anoll/8013496/webrev.03/ 
<http://cr.openjdk.java.net/%7Eanoll/8013496/webrev.03/>

Best,
Albert

On 07.05.2013 19:25, Christian Thalinger wrote:

Re: RFR (XS): JDK-8014430: JRE crashes instead of stop compilation onfull Code Cache. Internal Error (c1_Compiler.cpp:87)

Christian Thalinger — 2013-05-22T21:16:24

On May 21, 2013, at 7:41 AM, Albert Noll <albert.noll-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org> wrote:


That looks good.  -- Chris

hg: hsx/hotspot-comp/hotspot: 8012371: Adjust Tiered compilethreshold according to available space in code cache

2013-05-22T18:39:57

Changeset: 91eba9f82325
Author:    anoll
Date:      2013-05-16 15:46 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/91eba9f82325

8012371: Adjust Tiered compile threshold according to available space in code cache
Summary: Added command line parameter to define a threshold at which C1 compilation threshold for  is increased.
Reviewed-by: kvn, iveresov

! src/share/vm/code/codeCache.cpp
! src/share/vm/code/codeCache.hpp
! src/share/vm/runtime/advancedThresholdPolicy.cpp
! src/share/vm/runtime/advancedThresholdPolicy.hpp
! src/share/vm/runtime/arguments.cpp
! src/share/vm/runtime/globals.hpp

Re: RFR (S) 8014811: loopTransform.cpp assert(cmp_end->in(2) == limit)failed

Vladimir Kozlov — 2013-05-22T15:28:26

Thank you, Roland

Vladimir

On 5/22/13 7:40 AM, Roland Westrelin wrote:

Re: RFR (M) 8010927: Kitchensink crashed with SIGSEGV, Problematicframe: v ~StubRoutines::checkcast_arraycopy

Vladimir Kozlov — 2013-05-22T15:27:37

Thank you, Roland


Correct. It also helped to fix the problem since the code on other 
platforms work so I don't have to spend a lot time on the fix (I did 
tried to fix existing code first).

Thanks,
Vladimir

Re: RFR (S) 8014811: loopTransform.cpp assert(cmp_end->in(2) ==limit) failed

Roland Westrelin — 2013-05-22T14:40:11


That looks good to me.

Roland.

Re: RFR (M) 8010927: Kitchensink crashed with SIGSEGV,Problematic frame: v ~StubRoutines::checkcast_arraycopy

Roland Westrelin — 2013-05-22T14:38:00

It looks good to me.


If I understand correctly, this is not strictly required to fix the bug. It's simply to make code similar on all platforms, right?

Roland.

Re: RFR (XS): JDK-8014430: JRE crashes instead of stop compilationon full Code Cache. Internal Error (c1_Compiler.cpp:87)

Albert Noll — 2013-05-22T14:21:20

Hi,

Valdimir, thanks for your comments. I guess we need a second review for 
the adapted
version before we can push?

  Best,
Albert

On 21.05.2013 19:50, Vladimir Kozlov wrote:

Re: RFR (S) : 8014362 : Need to expose some processor features viaUnsafe interface

David Chase — 2013-05-22T11:43:20

I think the idea is that false is safe, therefore it is the default.

On 2013-05-22, at 7:23 AM, David Holmes <david.holmes-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org> wrote:

Re: RFR (S) : 8014362 : Need to expose some processor features viaUnsafe interface

David Holmes — 2013-05-22T11:23:36

Neither of those statements require that the default value of the flag 
be false. You can have it be true and then verify it is supported - and 
clear it if not; that all happens before anything would issue the new 
instructions. If it is not supported and was asked for on the 
command-line you can detect that too.

I just find it an odd convention because you can't look at globals.hpp 
and the command-line and know the value of the flag. But maybe that 
isn't as common as I think (I tend to mostly look at runtime flags)

Anyway, moving on ...

David

RFR (S) 8014811: loopTransform.cpp assert(cmp_end->in(2) == limit)failed

Vladimir Kozlov — 2013-05-22T01:23:46

http://cr.openjdk.java.net/~kvn/8014811/webrev

partial_peel() method leaves other loop's info in incorrect state if it 
bailouts due to new phi nodes creation. clone_for_use_outside_loop() 
places node clones near uses which is not correct in some cases. In the 
bug case it places loop's limit node inside the loop which use it.
It is fine in case partial_peel() exits normally since it stops current 
iteration of loop opts and nodes placement is redone during next iteration.
The fix is to do the same if partial_peel() failed but it created node 
clones outside processed loop.
Note, fixing clone_for_use_outside_loop() to find a correct place for 
clones is much more difficult. And this failing path is rare.

jtreg and JPRT testing.

Thanks,
Vladimir

Re: on stack replacement vs optimal compiled code

Vladimir Kozlov — 2013-05-21T23:49:09

In most cases both compilations (OSR and normal) happen at the same time 
because normal compilation is triggered by (invocation count + 
backbranch (osr) count) > threshold. Next time the function is called it 
will user normal compiled code.


Not true. The performance of the loop code (for which OSR is compiled) 
should be about the same as for normal compilation. The main problem 
with OSR is it is compiled before a profiling information is collected 
for a code after the loop. As result the compiled code will hit uncommon 
trap after loop, execution returned to Interpreter to get more profiling 
and the method will be recompiled.


The statement is true only if by "critical" you mean "hot" (frequently 
executed). If the function is called rarely but you need to compile it 
to get best performance (since it is critical) then you need warm it up.

Vladimir

on stack replacement vs optimal compiled code

Andy Nuss — 2013-05-21T23:10:40

Hi,

If a long looping function gets the benefit of "on stack replacement", and later hotspot determines that it has been executed more than 10,000 times, will it be recompiled again?

I read somewhere that OSR code is about half as fast as optimally jitted code.  I read somewhere else that Oracle says there is never a need to warmup critical functions for hotspot.


Andy

Re: RFR(XS): JDK-8010941: MinJumpTableSize is set to 18, investigateif that's still optimal

Aleksey Shipilev — 2013-05-21T22:48:46


Yeah, that works if the benchmark is the single method call. Anything
more complex require more complex warmup.


Or, (chanting) use JMH:
 http://hg.openjdk.java.net/code-tools/jmh/file/tip/jmh-samples/src/main/java/org/openjdk/jmh/samples/JMHSample_16_CompilerControl.java

Or, (chanting again) use JMH, because it does not really use indexed
loops, but rather volatile-predicated loop, so the loop unrolling is
ineffective (double ineffective with the source data re-read from the
fields on every iteration).

Before you jump on other platforms, look into the assembly to see if
your benchmark are actually generate the code that makes sense (i.e.
trapped on any of the issues Vladimir and me had speculated here):
 https://wiki.openjdk.java.net/display/HotSpot/PrintAssembly

-Aleksey.

Re: RFR(XS): JDK-8010941: MinJumpTableSize is set to 18, investigateif that's still optimal

Vladimir Kozlov — 2013-05-21T18:17:27

We usually do about 20000 iterations and run with -Xbatch to make sure 
tested method is compiled before time measurement.


You can avoid it by 
-XX:CompileCommand=dontinline,Test::multiply_by_power_of_ten

You can't relay on results if method is inlined because of Aleksey's 
pointed problems.

An other problem I see is different multiply values may take different 
times (I am talking hw instruction execution, C2 does not optimize 
double multiply). Also you have additional conversion int->double and 
long->double code which will affect results. You need the same code 
size/latency for each branch to get correct result.

Saying all that, the result you get (5) is about right. But I would like 
to see data from your experiment. And may be we can use the same value 
on all platforms (don't forget embedded).

thanks,
Vladimir

Re: RFR (XS): JDK-8014430: JRE crashes instead of stop compilationon full Code Cache. Internal Error (c1_Compiler.cpp:87)

Vladimir Kozlov — 2013-05-21T17:50:21

It is good. (Change Copyright year).


C2 does the same for the same code so leave it as it is.
I was concern about places in C1 where it may allocate other temp 
buffers. But it looks like there are non since you and I can't find it.

thanks,
Vladimir

Re: RFR (S) : 8014362 : Need to expose some processor features viaUnsafe interface

Vladimir Kozlov — 2013-05-21T14:46:54

David C. is correct. We set flags for new instructions to false by default because it is most common case (old cpus). 
Also it allows us to track incorrect settings on command line when an instruction is not available.

Vladimir