On Thu, 16 May 2024 17:44:22 GMT, Ioi Lam <ik...@openjdk.org> wrote:
>> JavacBench is a test program that compiles 90 Java source files. It uses a
>> fair amount of invokedynamic callsites, so it's good for testing CDS support
>> for indy and lambda expressions.
>>
>> This test was first integrated into the
>> [leyden](https://github.com/openjdk/leyden/tree/premain) repo. Hence some of
>> the files have copyrights in 2023.
>
> 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()
A few minor comments.
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)
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?
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?
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?
-------------
PR Review: https://git.openjdk.org/jdk/pull/19256#pullrequestreview-2064242660
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1605479322
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1605468705
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1605474939
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1605477874
