RFR 8224137: Analyze and port invocation tests to jtreg and co-locate to jdk repo

Tue Jun 25 13:48:50 UTC 2019

Thanks David!

 >> May take me a while to find the details ...

No need to do this.

Harold

On 6/25/2019 9:10 AM, David Holmes wrote:
> On 25/06/2019 8:53 am, Harold Seigel wrote:
>> Hi David,
>>
>> Please see comments inline.
>>
>> On 6/24/2019 4:15 PM, David Holmes wrote:
>>> Hi Harold,
>>>
>>> On 24/06/2019 1:04 pm, Harold Seigel wrote:
>>>> Hi David,
>>>>
>>>> Thanks for pointing this out.  My intent is to push the current 
>>>> tests and then create an enhancement to improve them.  I'd like to 
>>>> add C1 and Graal testing as part of the improvements.
>>>
>>> Okay. Wasn't sure why you went for the -Xint / -Xcomp additions at 
>>> this stage? :)
>> I plan to also test c1 and graal but would like to start with these.
>>>
>>>> Also, note that passing TRUE as the first argument to 
>>>> ProcessBuild.createJavaProcessBuilder() adds test.vm.opts and 
>>>> test.java.opts to the java arguments.
>>>
>>> Okay so your explicit Xint and Xcomp can conflict with what gets 
>>> passed through by jtreg. I'm not sure which order flags will be 
>>> presented, so even if "last flag wins" it's not clear exactly what 
>>> will get tested. In particular using Xcomp when jtreg passes in 
>>> Graal flags, can cause issues.
>> Is there an open bug for the Xcomp / Graal flags issue?
>
> It isn't a bug per-se, simply that if you have certain flags on an 
> @run or explicit exec of the VM, to try and test C1/C2, if you then 
> apply the Graal flags on top, it causes Graal to execute in a slow 
> mode that leads to timeouts. So a number of tests have been modified. 
> May take me a while to find the details ...
>
>>>
>>> So maybe for this initial push explicit mode selection should be 
>>> avoided?
>>
>> I think it would be better to pass FALSE as the first argument to 
>> ProcessBuild.createJavaProcessBuilder(). Then I will know what is 
>> getting tested.
>
> That's a good/safe starting point.
>
> Thanks,
> David
>
>> Thanks, Harold
>>
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> Harold
>>>>
>>>> On 6/21/2019 9:18 PM, David Holmes wrote:
>>>>> Hi Harold,
>>>>>
>>>>> Not a review ...
>>>>>
>>>>> On 21/06/2019 10:55 am, Harold Seigel wrote:
>>>>>> Hi Coleen,
>>>>>>
>>>>>> Can you take one more look?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137.3/webrev/
>>>>>>
>>>>>> I changed three files, invokespecialTests.java, 
>>>>>> invokeinterfaceTests.java, and invokevirtualTests.java to 
>>>>>> explicitly specify -Xint and -Xcomp to the sub-tests.
>>>>>
>>>>> If you want to check all the compilation environments you need to 
>>>>> be more elaborate than just using -Xcomp. You may need to test C1 
>>>>> only and C2 only. And then you need to consider Graal as well.
>>>>>
>>>>> IIUC from a brief look at these tests they will never pass through 
>>>>> the VM flags given to jtreg - is that correct?
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Thanks again!
>>>>>>
>>>>>> Harold
>>>>>>
>>>>>> On 6/20/2019 6:37 PM, coleen.phillimore at oracle.com wrote:
>>>>>>> Great, thanks. Looks good!
>>>>>>> Coleen
>>>>>>>
>>>>>>> On 6/20/19 1:00 PM, Harold Seigel wrote:
>>>>>>>> Hi Coleen,
>>>>>>>>
>>>>>>>> Thanks for reviewing this change!
>>>>>>>>
>>>>>>>> Please review this updated webrev:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137.2/webrev/index.html 
>>>>>>>>
>>>>>>>>
>>>>>>>> The -XDignore.symbol.file has been removed and TEST.groups has 
>>>>>>>> been changed.  Otherwise, it is the same as the previous webrev.
>>>>>>>>
>>>>>>>> Mgmt said to leave the copyright years as 2009, 2019, so I did 
>>>>>>>> not change those.
>>>>>>>> Thanks! Harold
>>>>>>>>
>>>>>>>> On 6/19/2019 5:41 PM, coleen.phillimore at oracle.com wrote:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/test/hotspot/jtreg/runtime/InvocationTests/invokeinterfaceTests.java.html 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>   35  * @compile -XDignore.symbol.file 
>>>>>>>>> invokeinterface/Checker.java invokeinterface/ClassGenerator.java
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Do these still need -XDignore.symbol.file ?
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/test/hotspot/jtreg/TEST.groups.udiff.html 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Can you specify all the tests in the directory by directory? 
>>>>>>>>> like:
>>>>>>>>>
>>>>>>>>> + -runtime/InvocationTests \
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/test/hotspot/jtreg/runtime/InvocationTests/shared/Caller.java.html 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>    2  * Copyright (c) 2009, 2019, Oracle and/or its 
>>>>>>>>> affiliates. All rights reserved.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Some of the copyrights say 2009, which is when the tests were 
>>>>>>>>> written but I think we're supposed to have the original date 
>>>>>>>>> when they're added to the repository.
>>>>>>>>>
>>>>>>>>> Well done getting these tests into jtreg and the repository!
>>>>>>>>> Thanks,
>>>>>>>>> Coleen
>>>>>>>>>
>>>>>>>>> On 6/17/19 2:57 PM, Harold Seigel wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Please review this JDK-14 change to move the invocation tests 
>>>>>>>>>> written by Vladimir Ivanov into the JDK repo and make them 
>>>>>>>>>> runnable using JTReg.
>>>>>>>>>>
>>>>>>>>>> This webrev adds three tests, invokeinterfaceTests.java, 
>>>>>>>>>> invokespecialTests.java, and invokevirtualTests.java. Each 
>>>>>>>>>> tests run its set of sub-tests twice, once using class file 
>>>>>>>>>> version 51 and once using the current class file version.
>>>>>>>>>>
>>>>>>>>>> Open Webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/
>>>>>>>>>>
>>>>>>>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8224137
>>>>>>>>>>
>>>>>>>>>> The tests were tested on Linux-x64, Solaris, Windows, and Mac 
>>>>>>>>>> OS X.
>>>>>>>>>>
>>>>>>>>>> The original tests can be found attached to JDK-8163974 
>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8163974>. Besides 
>>>>>>>>>> the changes needed for JTReg and adding copyrights, I made 
>>>>>>>>>> the following additional changes.
>>>>>>>>>>
>>>>>>>>>> 1. The tests now use the JDK's asm support instead of 
>>>>>>>>>> providing its own
>>>>>>>>>>    asm libraries.
>>>>>>>>>> 2. Only sub-test failures are written to the .jtr files. 
>>>>>>>>>> Writing all
>>>>>>>>>>    sub-test results caused JTReg to truncate the output.
>>>>>>>>>> 3. Changed src/invokeinterface/Checker.java to skip private 
>>>>>>>>>> methods
>>>>>>>>>>    when looking for an overloading method.
>>>>>>>>>>
>>>>>>>>>> The tests contain "TODO" comments and other thing needing 
>>>>>>>>>> clean-up.  These will be addressed in a future RFE.
>>>>>>>>>>
>>>>>>>>>> I put the tests into hs-tier3 because the 
>>>>>>>>>> invokeInterfaceTests.java test can run for up to 10 minutes 
>>>>>>>>>> (on Mac).  The other two tests take only 1-2 minutes.  Is 
>>>>>>>>>> there a better tier for these tests?
>>>>>>>>>>
>>>>>>>>>> Thanks, Harold
>>>>>>>>>>
>>>>>>>>>
>>>>>>>