RFR: 8212630: jtreg/gc/logging/TestMetaSpaceLog.java failed with Agent timed out

[Leonid Mesnik]

> On May 30, 2019, at 5:04 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
>> On May 30, 2019, at 6:22 PM, Leonid Mesnik <leonid.mesnik at oracle.com> wrote:
>> 
>> Hi
>> 
>> I think that '@run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI' could be replaced with @run main/driver because
>> test spawns other process and don't use any options and WB.
> 
> The main tests are using sun.hotspot.gc.GC, which uses WB.
> 
Sorry, I added this comment after realized how tests could be rewritten. Currently using of WB is needed.
>> Also I think you need to have several separate headers for each test instead of multiple runs. There is only one result per test (not for every run) and test is is skipped if any of GC is not supported. Additionally these tests might be guarded with require instead of throwing SkipException. So your tests is not going to be executed saying if ZGC is selected by jtreg CLI. Here is the example of fix which update test with different GCs. It contains a lot of examples of similar tests.
>> http://hg.openjdk.java.net/jdk/jdk/rev/9b8926bf85c1
>> 
>> And in the case of new timeouts/other failures we will know exactly which GC was used right in the report.
> 
> I thought about using multiple @tests rather than multiple @runs, but
> decided not to take that path, as doing so is much more verbose, for
> not much (if any) benefit. The verbosity comes from duplicating most
> of the @test block N times (including build and install WB support,
> since the child process uses it too), rather than just having N @run
> lines plus a little bit of code in main to look up the requested GC
> object and dispatch on whether it's supported. I don't think getting N
> results rather than 1 and avoiding the SkippedException case are
> worth that "code" duplication. (The multiple @test use-case really
> wants some sort of template mechanism.)
> 
> Note that I tested it with an additional @run line for Shenandoah
> (with it configured out as usual for our builds, so not supported),
> and that worked fine, giving the expected "passed because skipped"
> result.
As I understand in this case test is not going to execute any @run after skipped one. So if anyone decide to add Shenandoah testing as first @run then test is going to test nothing and always pass as skipped in our testing. 

Please note that this test is always executed with all GCs specified even if specific GC is set using jtreg options. It doesn't harm for single test but increase wasting time if many tests use this approach. 

BTW If test is not GC specific then it might just pick GC from jtreg command-line. It might be no sense to run it with all GCs explicitly each time.

Leonid 
> 
> This test wouldn't have been executed with ZGC or Shenandoah
> previously either. If we want it to test those we should now add new
> @run lines for them. (Or new @test blocks, but that just makes my
> verbosity complaint even worse.) (The old version required adding
> "if-supported then test" blocks for them.) (And note that this test
> fails as expected for Epsilon.)
> 
> I see several other tests in that changeset that could be restructured
> like I'm doing here (any using GC.isSupported() seem like candidates
> for this same treatment), perhaps avoiding similar timeouts for
> similar reasons.
> 
>> 
>> Leonid
>> 
>>> On May 30, 2019, at 12:29 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> 
>>> […]
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8212630
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~kbarrett/8212630/open.00/
>>> 
>>> Testing:
>>> A few dozen local executions of the test.  That's not enough and
>>> probably not in a configuration to encounter the problem -- failures
>>> have been occurring on the order of 0.5% of the time under high load.
> 
>