RFR: 8332340: Add JavacBench as a test case for CDS [v2]

[Ioi Lam]

On Fri, 17 May 2024 19:45:00 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   (1) comments from @liach; (2) added javadoc; (3) Use createTestJavaProcessBuilder() instead of createLimitedTestJavaProcessBuilder()
>
> test/hotspot/jtreg/runtime/cds/appcds/applications/JavacBench.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
> 
> Since this is a new file, should the copyright year be only 2024?
> (same for other files)

The file was first added in the Leyden repo in 2023, so I think we should use that as the initial copyright year.

> test/hotspot/jtreg/runtime/cds/appcds/applications/JavacBench.java line 43:
> 
>> 41: import jdk.test.lib.cds.CDSAppTester;
>> 42: import jdk.test.lib.helpers.ClassFileInstaller;
>> 43: import jdk.test.lib.process.OutputAnalyzer;
> 
> Is this import needed?

Removed.

> test/hotspot/jtreg/runtime/cds/appcds/applications/JavacBenchApp.java line 222:
> 
>> 220:         }
>> 221:         long elapsed = System.currentTimeMillis() - started;
>> 222:         if (System.getProperty("JavacBenchApp.silent") == null) {
> 
> Should line 221 be inside the 'if' block?

I moved it inside the 'if' block.

> test/lib/jdk/test/lib/cds/CDSAppTester.java line 218:
> 
>> 216:         }
>> 217: 
>> 218:         throw new RuntimeException("Must have exactly one command line argument: STATIC or DYNAMIC");
> 
> Why not check the number of args at the beginning of the method?

Fixed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047769
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047882
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047859
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047805