RFR(S): 8159620: -XX:-UseOnStackReplacement does not work together with -XX:+TieredCompilation on ppc64 and sparc

[Volker Simonis]

On Tue, Jun 21, 2016 at 10:44 AM, Tobias Hartmann
<tobias.hartmann at oracle.com> wrote:
> Hi Volker,
>
> On 21.06.2016 09:37, Volker Simonis wrote:
>> On Mon, Jun 20, 2016 at 10:59 AM, Tobias Hartmann
>> <tobias.hartmann at oracle.com> wrote:
>>> Hi Volker,
>>>
>>> On 20.06.2016 10:52, Volker Simonis wrote:
>>>> Hi Tobias,
>>>>
>>>> thanks for sponsoring! I've uploaded a new webrev with you and
>>>> Vladimir as reviewers:
>>>>
>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v2/
>>>>
>>>> You can find the changeset there:
>>>>
>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v2/hotspot.changeset
>>>
>>> Thanks, I just noticed that the test has
>>>
>>> 26  * @bug 9999999
>>>
>>> You can fix this in-place. I'll then run the required pre-integration testing (~24h) and push your change afterwards.
>>>
>>
>> Hi Tobias,
>>
>> good catch and sorry, I somehow missed your mail yesterday.
>>
>> You can find the new changeset here:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v3/hotspot.changeset
>
> Thank you!
>
> I just looked at the test results and unfortunately DisableOSRTest fails on all platforms (including SPARC) if -Xcomp is set:
>
> ----------System.err:(13/1215)----------
> java.lang.RuntimeException: "public static void DisableOSRTest.main(java.lang.String[]) throws java.lang.Exception" shouldn't be OSR compiled if running with -XX:-UseOnStackReplacement!
>         at DisableOSRTest.main(DisableOSRTest.java:59)
>         at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base/Native Method)
>         at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base/NativeMethodAccessorImpl.java:62)
>         at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base/DelegatingMethodAccessorImpl.java:43)
>         at java.lang.reflect.Method.invoke(java.base/Method.java:531)
>         at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110)
>         at java.lang.Thread.run(java.base/Thread.java:843)
>
> There are several OSR compilations in the log:
>
>  12916 4242 %  b  4       DisableOSRTest::main @ 19 (61 bytes)
>

Hi Tobias,

you're right! Thanks for finding this new problem:)

I initially only fixed OSR from interpreted code. But C1 compiled code
can also trigger an OSR request (and -Xcomp triggers a C1 compilation
of main before it is invoked for the first time). Switching this off
with the help of -XX:-UseOnStackReplacement never worked, but it was
actually easy to fix. Please find the new change here:

http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v4/

I think it would good if somebody (maybe the initial reviewers) could
review the new C1 changes to prevent OSR in C1-compiled code.

Thank you and best regards,
Volker


> Best regards,
> Tobias
>
>>
>> Thank you and best regards,
>> Volker
>>
>>
>>> Best regards,
>>> Tobias
>>>
>>>>
>>>> Thanks,
>>>> Volker
>>>>
>>>>
>>>> On Mon, Jun 20, 2016 at 10:46 AM, Tobias Hartmann
>>>> <tobias.hartmann at oracle.com> wrote:
>>>>> Hi Volker,
>>>>>
>>>>> you fix looks good to me! I can do the sponsoring, please just send me a changeset.
>>>>>
>>>>> Best regards,
>>>>> Tobias
>>>>>
>>>>> On 20.06.2016 10:16, Volker Simonis wrote:
>>>>>> Thanks Vladimir!
>>>>>>
>>>>>> .. I still need a sponsor :(
>>>>>>
>>>>>> Regards,
>>>>>> Volker
>>>>>>
>>>>>>
>>>>>> On Fri, Jun 17, 2016 at 10:53 PM, Vladimir Kozlov
>>>>>> <vladimir.kozlov at oracle.com> wrote:
>>>>>>> Looks good.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>>
>>>>>>> On 6/17/16 2:22 AM, Volker Simonis wrote:
>>>>>>>>
>>>>>>>> Hi Goetz,
>>>>>>>>
>>>>>>>> thanks for the review.
>>>>>>>> You're right, I've fixed the "else":
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v1/
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Volker
>>>>>>>>
>>>>>>>> On Fri, Jun 17, 2016 at 11:08 AM, Lindenmaier, Goetz
>>>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi Volker,
>>>>>>>>>
>>>>>>>>> thanks for doing this fix, I also have run into this issue before ...
>>>>>>>>> Looks good.
>>>>>>>>>
>>>>>>>>> Small nit: usually
>>>>>>>>>   }
>>>>>>>>>   else {
>>>>>>>>> are on one line.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>>   Goetz.
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
>>>>>>>>>> Behalf Of Volker Simonis
>>>>>>>>>> Sent: Donnerstag, 16. Juni 2016 16:54
>>>>>>>>>> To: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
>>>>>>>>>> Subject: RFR(S): 8159620: -XX:-UseOnStackReplacement does not work
>>>>>>>>>> together with -XX:+TieredCompilation on ppc64 and sparc
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> can I please have a review and sponsor for the following small change
>>>>>>>>>> which fixes -XX:-UseOnStackReplacement to work together with
>>>>>>>>>> -XX:+TieredCompilation:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620/
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8159620
>>>>>>>>>>
>>>>>>>>>> This is a long standing bug on SPARC and as the ppc64 template
>>>>>>>>>> interpreter was initially forked from the SPARC implementation, it
>>>>>>>>>> also manifests there. The problem is that in the case of tiered
>>>>>>>>>> compilation the interpreter unconditionally calls
>>>>>>>>>> InterpreterRuntime::frequency_counter_overflow if the back edge
>>>>>>>>>> counter overflows. This triggers an OSR compilation, even if OSR was
>>>>>>>>>> switched off with -XX:-UseOnStackReplacement.
>>>>>>>>>>
>>>>>>>>>> The fix is simple - just don't call
>>>>>>>>>> InterpreterRuntime::frequency_counter_overflow if OSR has been
>>>>>>>>>> switched off.
>>>>>>>>>>
>>>>>>>>>> Thank you and best regards,
>>>>>>>>>> Volker