RFR (L): 8068685 - [TESTBUG] Ensure @library works in jtreg -agentvm mode
david.holmes at oracle.com
Thu Feb 26 03:50:17 UTC 2015
One correction ...
On 26/02/2015 12:16 PM, David Holmes wrote:
> Hi George,
> On 25/02/2015 11:42 PM, George Triantafillou wrote:
>> Hi David,
>> Thanks for your review. Comments inline:
>> On 2/24/2015 11:55 PM, David Holmes wrote:
>>> On 25/02/2015 6:45 AM, George Triantafillou wrote:
>>>> Please review this fix for JDK-8068685:
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8068685
>>>> webrev: http://cr.openjdk.java.net/~gtriantafill/8068685/webrev.00
>>>> "@build com.oracle.java.testlibrary.*" was added to all relevant tests,
>>> I thought we had an RFE open against jtreg to avoid the need to do
>>> this, but it seems I was mistaken. :(
>>> Most of the additions to @build seem okay. A few minor issues:
>>> --- old/test/compiler/whitebox/ForceNMethodSweepTest.java 2015-02-24
>>> 10:31:03.991071066 -0800
>>> +++ new/test/compiler/whitebox/ForceNMethodSweepTest.java 2015-02-24
>>> 10:31:03.617052982 -0800
>>> @@ -35,6 +35,9 @@
>>> * @test
>>> * @bug 8059624 8064669
>>> * @library /testlibrary /../../test/lib
>>> + * @build com.oracle.java.testlibrary.* ForceNMethodSweepTest
>>> + * @ignore 8066998
>>> + * @library /testlibrary /../../test/lib
>>> * @build ForceNMethodSweepTest
>>> The second @library shouldn't have been added.
>> Good catch, thanks. Changed.
>>> --- old/test/gc/g1/TestStringSymbolTableStats.java 2015-02-24
>>> 10:32:28.209143019 -0800
>>> +++ new/test/gc/g1/TestStringSymbolTableStats.java 2015-02-24
>>> 10:32:27.855125901 -0800
>>> @@ -22,11 +22,13 @@
>>> - * @test TestStringSymbolTableStats.java
>>> + * @test TestStringSymbolTableStats
>>> Why did you change the @test? I see a lot of unchanged @test foo.java.
>> According to the jtreg tag spec
>> http://openjdk.java.net/jtreg/tag-spec.html, either way is correct. The
>> only tests that were considered for this change included "@library
>> /testlibrary", so I changed this one for consistency with other tests
>> that I've written. I'll change it back if it's a concern.
> No big deal but caused confusion when reviewing.
>>>> which resulted in longer test execution times. In order to offset
>>>> longer test execution times and to support running jtreg in -agentvm
>>>> mode, the following changes were made:
>>>> test/Makefile: added -agentvm option
>>> I'm unclear exactly what this buys us - do we just save on the startup
>>> costs of new vms running jtreg? I thought the default was samevm anyway?
>>> Also does this mean that all VMs will share a single copy of the
>>> testlibrary or will we still have a copy per test that gets built each
>> The jtreg command help http://openjdk.java.net/jtreg/command-help.html
>> doc states:
>> -agentvm "Run tests using a pool of reusable JVMs."
>> -samevm "If possible, run each test in the same JVM as the JavaTest"
>> I'm not really sure how samevm decides "if possible", but the intent of
>> this change was to explicitly add "@build com.oracle.java.testlibrary.*"
>> to relevant jtreg runtime tests without increasing test execution
>> times. The analysis below shows decreased test execution times using
> Yes but I'd like to understand why that is the case, and whether we are
> still being inefficient in this even if better than the initial attempt.
> It is very unclear to me how the addition of agentvm is influencing the
> building of the testlibrary.
>>>> test/testlibrary/com/oracle/java/testlibrary/ProcessTools.java: added
>>>> -classpath since -agentvm passes the classpath differently than
>>>> *Testing shows an 11% speed improvement for JPRT execution of the
>>>> hotspot testset, and a 48% speed improvement for
>>>> hotspot_runtime*/gc*/compiler*/serviceability* tests.**
>>>> *Note that some tests required the explicit addition of -othervm in
>>>> order for the tests to pass. In addition, a number of gc, compiler and
>>>> runtime tests were modified with @run so they would run successfully
>>>> with -agentvm. Consequently, reviews from members of the compiler, gc,
>>>> runtime, and serviceability teams would be very helpful.
>>> I'm unclear about the need for some of the othervm changes. I would
>>> only expect othervm to be needed if passing -XX options via @run ??
>> I added @run to some tests, and some of them failed with -agentvm until
>> I added -othervm.
> This is a puzzle - perhaps a jtreg bug. Without the @build for the
> testlibrary you can use -agentvm without a problem based on a few
> experiments I did. Once you add the @build you then also have to add
> @run, but the simple: @run test fails with e.g.:
> test result: Error. Bad action for script: CdsSameObjectAlignment
User error. You must have an action, such as main, after @run.
> However it suffices to add "main" in the @run:
> @run main CdsDifferentObjectAlignment
> without needing to add /othervm
>>>> Thanks to Erik Helin for providing a solution for passing classpath
>>>> The fix was tested locally on Linux with jtreg and on all platforms
>>>> the JPRT hotspot testset.*
More information about the hotspot-dev