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

[Lindenmaier, Goetz]

Yes, Looks Good!

Thanks, Goetz 

> Am 13.07.2018 um 09:33 schrieb Volker Simonis <volker.simonis at gmail.com>:
> 
>> 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
>>>>> 
>>