[11] RFR(S): 8207067: [test] prevent timeouts in serviceability/tmtools/jstat/{GcTest02, GcCauseTest02}.java

[Volker Simonis]

On Thu, Jul 12, 2018 at 11:55 PM, David Holmes <david.holmes at oracle.com> wrote:
> On 13/07/2018 12:51 AM, Lindenmaier, Goetz wrote:
>>
>> Hi Volker,
>>
>> I had a look at your change.
>> Your intent is plausible.
>>
>> Won’t it help to just set -XX:+IgnoreUnrecognizedVMOptions?
>
>
> Yes! Great suggestion.
>

I wouldn't have believed that it works, because VerifiyDependencies is
actually not "unknown" but "constant" in non-debug builds. But it
actually really does!

This of course considerably simplifies the change. I've also shortened
the comment as suggested by David. Please find the updated version
here:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8207067.v1/

OK to push now?

Thank you and best regards,
Volker

>> Alternatively, you could specify two test setups, one with @requires
>> vm.debug, the
>> other with !vm.debug.
>
>
> I thought about that one too but the amount of duplicated @xxx stuff looked
> pretty ugly.
>
> Cheers,
> David
>
>
>> If non of these are possible, your change looks good.
>>
>> Best regards,
>>    Goetz.
>>
>>> -----Original Message-----
>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>> bounces at openjdk.java.net] On Behalf Of David Holmes
>>> Sent: Donnerstag, 12. Juli 2018 01:27
>>> To: Volker Simonis <volker.simonis at gmail.com>; hotspot-runtime-
>>> dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net>
>>> Subject: Re: [11] RFR(S): 8207067: [test] prevent timeouts in
>>> serviceability/tmtools/jstat/{GcTest02, GcCauseTest02}.java
>>>
>>> Hi Volker,
>>>
>>> On 12/07/2018 3:30 AM, Volker Simonis wrote:
>>>>
>>>> Hi,
>>>> can I please have a review for the following test fix which prevents
>>>> eventual test timeouts:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8207067
>>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8207067/
>>>>
>>>> The two tests
>>>
>>> hotspot/jtreg/serviceability/tmtools/jstat/{GcTest02,GcCauseTest02}.java
>>>>
>>>> produce more than 90_000 classes until they eat up ~70% of the 128M
>>>> meta space they run with. The loading of each of these classes
>>>> triggers a full dependency check for ALL nmethods in debug/fastdebug
>>>> builds because 'VerifyDependencies' is 'true' there. This slows down
>>>> the tests from about 3 sec. in the opt build to about 88 sec. in the
>>>> fastdebug build on x86_64 and from about 4 sec. to about 560 sec. on
>>>> ppc64.
>>>>
>>>> Because the tests are not about dependency checking, it makes sense to
>>>> switch of 'VerifyDependencies' if they are run inside a
>>>> debug/fastdebug VM and decrease the execution time down to about 6
>>>> sec. on both x86_64 and ppc64.
>>>
>>>
>>> It's very annoying that this can't be fixed the obvious way by just
>>> specifying -XX:-VerifyDependencies. Maybe jtreg could add a
>>> debug_only(...)  capability ... :(
>>>
>>> It's unclear to me whether it is safe to disable VerifyDependencies
>>> after the VM has commenced execution. This might lead to
>>> inconsistencies. Probably need the compiler folk to clarify that.
>>>
>>> That aside the comments are far too elaborate and better suited for the
>>> bug report. The tests could use @bug lines (though it would be nice to
>>> track down the original bug that added the tests). The comments could
>>> reduce to a simple:
>>>
>>> // This test produces more than 90_000 classes until it eats up ~70% of
>>> the 128M meta space.
>>> // With VerifyDependencies enabled in debug builds this slows the test
>>> down considerably.
>>> // As it is a develop flag, if we see that it is "constant" then we know
>>> this is a product build.
>>>
>>> Though there is already Platform.isDebugBuild() if you wanted something
>>> more direct. (Given you need WB to change the flag it doesn't really
>>> make much difference.)
>>>
>>> Thanks,
>>> David
>>>
>>>> Thank you and best regards,
>>>> Volker
>>>>
>