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

[Volker Simonis]

Thanks David & Goetz!

Regards,
Volker

On Fri, Jul 13, 2018 at 10:32 AM, David Holmes <david.holmes at oracle.com> wrote:
> On 13/07/2018 5:33 PM, Volker Simonis wrote:
>>
>> 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!
>
>
> I was surprised too. :)
>
>> 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/
>
>
> The comment placement seems odd. You could just add an extra line to the
> @comment part, or just move to be a class level comment rather than inside
> main.
>
> No need to see any update regardless.
>
> Thanks,
> David
>
>
>> 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
>>>>>>
>>>
>