[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: RFR 8224137: Analyze and port invocation tests to jtreg and co-locate to jdk repo
From: Harold Seigel <harold.seigel () oracle ! com>
Date: 2019-06-25 13:48:50
Message-ID: afd156fe-7492-37d2-2577-e5c3d00187e8 () oracle ! com
[Download RAW message or body]
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@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@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
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic