RFR (L): 8068685 - [TESTBUG] Ensure @library works in jtreg -agentvm mode

David Holmes david.holmes at oracle.com
Sun Mar 1 22:57:13 UTC 2015 

On 28/02/2015 3:03 AM, Mikhailo Seledtsov wrote:
> I agree with Staffan that the kind of increase in text execution speed
> is worth the effort and risk.

Just bear in mind that anyone writing tests needs to be more aware of 
any assumptions that test makes about the global state of the VM, and 
what affect the test has on the global state of the VM.

My concern with getting this up and running is that the test that fails 
is not the test that should be being run in othervm - and finding the 
test that is causing the problem is non-obvious.

> Also, the reversal of this change is only a single line of code:
> test/Makefile
>
>     JTREG_BASIC_OPTIONS += -agentvm
>
> The rest of the changes will not harm in any way, and will still be
> helpful for people or systems that explicitly specify "-agentvm" option
> when running the tests. Perhaps, George could first push all the changes
> except for the "-agentvm" change; then run the RBT tests as Staffan
> recommends; if that is fine and additional issues resolved, include
> "JTREG_BASIC_OPTIONS += -agentvm" in a separate changeset.

I think the slowdown caused by building the test library makes that 
infeasible. But there's no need to push anything to run it through RBT 
is there?

Also I am still concerned about some of the othervm changes George made 
that should not be necessary. The CDS tests for example do all their 
testing by launching other VMs, so the actual CD test should not need to 
be othervm itself - and indeed with my testing it did not need to be.

Thanks,
David

> Thank you,
> Misha
>
> On 2/27/2015 1:15 AM, Staffan Larsen wrote:
>> You are right that this is a risk we are taking. Tests that leave the
>> JVM in an un-clean state should be running in main/othervm mode to
>> isolate them. It should be noted that if a test fails, then jtreg will
>> /not/ reuse that JVM since it can be an unknown state. Still, I’m
>> guessing that we will see some initial problems with tests that need
>> to be isolated. It would be good to run through the tests in agent
>> mode a fair amount of times on all platforms (say 20? - easy to do
>> with Remote Build and Test) before we do this change to shake out any
>> obvious problems. I think the increased speed of the tests is still
>> worth the effort to clean up and isolate some of the tests.
>>
>> /Staffan
>>
>>> On 26 feb 2015, at 21:17, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> I've been thinking more about this change based on Staffan's
>>> clarification of how "othervm" is the current default for testing. I
>>> think the move to agentvm is very risky and may result in very
>>> unpredictable test results. The problem being that tests no longer
>>> start out in a clean VM with a pristine state - this can impact many
>>> things that might affect how some of the tests execute. And the
>>> problems may vary depending on exactly how the tests are run, the
>>> concurrency level and number of agent VMs in use. There may not be
>>> any guarantee that the same jtreg command line will result in the
>>> same tests being executed in the same sequence on the same agent VMs.
>>>
>>> It may be that the introduction of "main/othervm" in some tests was
>>> already an indication of this problem. But the real problem may be
>>> the interaction between tests.
>>>
>>> David
>>>
>>> On 26/02/2015 6:14 PM, David Holmes wrote:
>>>> Hi Staffan,
>>>>
>>>> On 26/02/2015 6:00 PM, Staffan Larsen wrote:
>>>>>> On 25 feb 2015, at 14:42, George Triantafillou
>>>>>> <george.triantafillou at oracle.com
>>>>>> <mailto:george.triantafillou at oracle.com>> 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
>>>>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/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
>>>>>> spechttp://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.
>>>>>>>
>>>>>>>> which resulted in longer test execution times.  In order to offset
>>>>>>>> these
>>>>>>>> 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 time?
>>>>>> The jtreg command
>>>>>> helphttp://openjdk.java.net/jtreg/command-help.htmldoc 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 agentvm.
>>>>> Re-reading the documentation, checking the sources and running some
>>>>> experiments show that the default mode for jtreg is not samevm, but
>>>>> othervm. In othervm mode, jtreg will create a new JVM for each test
>>>>> /and/ for each javac invocation. In agentvm mode a pool of JVMs
>>>>> will be
>>>>> reused for both javac and for the tests. When running in agentmode,
>>>>> even
>>>>> if a test says “main/othervm”, the javac invocation of the test will
>>>>> happen in the agent. So running in agentvm mode saves time even if all
>>>>> tests say “main/othervm”.
>>>> Thanks for clarifying that. It would seem main/othervm is only ever
>>>> needed to override -agentvm or -samevm. That makes sense.
>>>>
>>>> I still wonder how many times the test library gets built per set of
>>>> tests ? :)
>>>>
>>>>> The following passage from the documentation is confusing:
>>>>>
>>>>> ----
>>>>> Test Mode Options
>>>>>
>>>>> When the JavaTest harness is used to run tests, two possibly different
>>>>> versions of the JDK are used: the JDK version used to run the harness
>>>>> and the JDK version used to run the test(s). The following options
>>>>> provide a means to specify the JDK version used to run the tests. The
>>>>> default is to use the same JDK version (provided by JAVA_HOME) for
>>>>> both
>>>>> the harness and the tests, and for each test to run in its own JVM.
>>>>> ----
>>>>>
>>>>> Notice how the last sentence seems to begin by saying samevm mode
>>>>> is the
>>>>> default, but its really just talking about the version of the JDK used
>>>>> to for running the tests. The sentence ends by saying (not very
>>>>> explicitly) that othervm is the default mode. I’ll see if we can
>>>>> clarify
>>>>> that.
>>>> It is even more confusing if you take into account that with samevm it
>>>> uses the testjdk, but you can also set compilejdk to a different jdk,
>>>> which suggests it will run the tests in the same vm as the harness but
>>>> launch separate vms to do the compilation.
>>>>
>>>> Well now I've said that perhaps not so confusing after all :)
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> /Staffan
>>>>>
>>>>>
>>>>>>>> test/testlibrary/com/oracle/java/testlibrary/ProcessTools.java:
>>>>>>>> added
>>>>>>>> -classpath since -agentvm passes the classpath differently than
>>>>>>>> -othervm
>>>>>>>>
>>>>>>>> *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.
>>>>>>
>>>>>> -George
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Thanks to Erik Helin for providing a solution for passing classpath
>>>>>>>> with
>>>>>>>> -agentvm.
>>>>>>>>
>>>>>>>> The fix was tested locally on Linux with jtreg and on all platforms
>>>>>>>> with
>>>>>>>> the JPRT hotspot testset.*
>>>>>>>> *
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>> -George
>

More information about the hotspot-runtime-dev mailing list