RFR: 8239423: [TESTBUG] jdk/jfr/jvm/TestJFRIntrinsic.java failed with -XX:-TieredCompilation

Wed Feb 26 01:51:58 UTC 2020

Hi,

On 25/02/2020 6:34 pm, Ao Qi wrote:
> On Tue, Feb 25, 2020 at 12:42 AM Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>
>> Hi
>>
>> I found it's hard to find all the tests that use the modified
>> functions, because not all the tests use the modified functions
>> directly.
>>
>>
>> I'd just change the functions in question to crash JVM, and then all the tests which start to fail are the tests which use these functions directly or indirectly. that's quite similar to your 2nd way.
>> I'll be very surprised if there are any non hotspot tests (modulo jfr) which use this API, so I'd just run hotspot and jdk/jfr tests.
> 
> Thanks, Igor. I am doing. I selected tests with TEST="test/hotspot
> test/jdk/jdk/jfr". It would take some time.
> 
>>
>> I also tested them with c1-only configuration. 2 failures were
>>
>> found, but regression were not found.
>>
>> what are these two failures?
> 
> Their names and part of stderr are in below.
> 
> test/hotspot/jtreg/compiler/jvmci/compilerToVM/IsMatureVsReprofileTest.java:
> ----------------------
> STDERR:
> java.lang.RuntimeException: Multiple times invoked method should have
> MDO: expected 0 to not equal 0
> at jdk.test.lib.Asserts.fail(Asserts.java:594)
> at jdk.test.lib.Asserts.assertNotEquals(Asserts.java:394)
> at jdk.test.lib.Asserts.assertNE(Asserts.java:367)
> at compiler.jvmci.compilerToVM.IsMatureVsReprofileTest.test(IsMatureVsReprofileTest.java:82)
> at compiler.jvmci.compilerToVM.IsMatureVsReprofileTest.main(IsMatureVsReprofileTest.java:65)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
> at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
> at java.base/java.lang.Thread.run(Thread.java:832)
> 
> JavaTest Message: Test threw exception: java.lang.RuntimeException:
> Multiple times invoked method should have MDO: expected 0 to not equal
> 0
> JavaTest Message: shutting down test
> ----------------------
> 
> test/hotspot/jtreg/compiler/jvmci/compilerToVM/ReprofileTest.java:
> ----------------------
> STDERR:
> java.lang.RuntimeException: public int
> compiler.jvmci.compilerToVM.DummyClass.dummyInstanceFunction() :
> profiling info wasn't changed after 1500 invocations: expected
> DefaultProfilingInfo<> to not equal DefaultProfilingInfo<>
> at jdk.test.lib.Asserts.fail(Asserts.java:594)
> at jdk.test.lib.Asserts.assertNotEquals(Asserts.java:394)
> at jdk.test.lib.Asserts.assertNE(Asserts.java:367)
> at compiler.jvmci.compilerToVM.ReprofileTest.runSanityTest(ReprofileTest.java:101)
> at java.base/java.util.ArrayList.forEach(ArrayList.java:1510)
> at compiler.jvmci.compilerToVM.ReprofileTest.main(ReprofileTest.java:64)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
> at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
> at java.base/java.lang.Thread.run(Thread.java:832)
> 
> JavaTest Message: Test threw exception: java.lang.RuntimeException:
> public int compiler.jvmci.compilerToVM.DummyClass.dummyInstanceFunction()
> : profiling info wasn't changed after 1500 invocations: expected
> DefaultProfilingInfo<> to not equal DefaultProfilingInfo<>
> JavaTest Message: shutting down test
> ----------------------

Those tests don't look like they handle a C1-only configuration to me. 
I'd just file a bug and let the JVMCI folk deal with that.

>>
>> Do I need do some other tests?
>>
>> applications/ctw tests definitely use the modified methods, so I'd recommend you to run one tests from there (e.g. applications/ctw/modules/java_base.java)
> 
> I will also do these tests.
> 
>>
>> regarding your patch, 'cl_min' name seems to be confusion (at least to me) as it actually works as an upper bound for valid compilation levels. also given you had to change the same code in 3 different places, wouldn't it be better to factor that out? so you'll have smth like:
>>
>> +  if (compLevel < CompLevel_none || compLevel > highestCompLevel()) {
>>       return false; // Intrinsic is not available on a non-existent compilation level.
>>     }
> 
> like this: http://cr.openjdk.java.net/~aoqi/8239423/webrev.02/ ?

That looks good to me!

Thanks,
David

> Thanks,
> Ao Qi
> 
>>
>>
>> I'd like to cycle back on David's idea ("I actually think it is a bug that the VM allows you to set TieredStopAtLevel even though it is ignored on those configurations") Isn't the actual problem in how TieredStopAtLevel flag is handled in cases when TieredCompilation is disabled and/or JVM is built w/o tiered compilation support? I mean if JVM ensured that in such cases TieredStopAtLevel is set to CompLevel_highest_tier, all these WB methods would be correct.
>>
>>
>> Thanks,
>> -- Igor
>>
>> On Feb 24, 2020, at 3:39 AM, Ao Qi <aoqi at loongson.cn> wrote:
>>
>> On Fri, Feb 21, 2020 at 10:42 AM Ao Qi <aoqi at loongson.cn> wrote:
>>
>>
>> We need to find all tests that use the modified WB functions and see if
>> they all pass under normal and non-tiered configurations.
>>
>>
>> I found it's hard to find all the tests that use the modified
>> functions, because not all the tests use the modified functions
>> directly. I try to find them by two ways:
>>   - I grep all modified functions' name in test dir and found all the
>> tests contain the names. Some found files are not jtreg tests. They
>> are used by other tests or libs. I also grep these files' name to find
>> tests that use these files. However I missed the situation that other
>> libs use these files.
>>   - I added some tty->print at the modified positions and run hotspot
>> (hotspot_all_no_apps), jdk (jdk_stable) and langtools (jdk lib tools)
>> tests. Then I found all jtr files which contain the added print.
>> The test list found by the above two ways are here:
>> http://cr.openjdk.java.net/~aoqi/8239423/all.txt . These tests passed
>> with default and -XX:-TieredCompilation parameter and passed under
>> --with-jvm-features=-compiler1 configuration. I also tested them with
>> c1-only configuration. 2 failures were found, but regression were not
>> found.
>>
>> I also tested tier1-3, both release and fastdebug, with default and
>> -XX:-TieredCompilation parameter.
>>
>> Do I need do some other tests?
>>
>> Cheers,
>> Ao Qi
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> I agree. I will try to find these tests.
>>
>> Cheers,
>> Ao Qi
>>
>>
>> Thanks,
>> David
>>
>>
>>
> 