Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]
On Tue, 21 May 2024 21:03:20 GMT, Matias Saavedra Silva
wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> @calvinccheung comments
>
> test/lib/jdk/test/lib/cds/CDSAppTester.java line 147:
>
>> 145: }
>> 146:
>> 147: private OutputAnalyzer dumpStaticArchive() throws Exception {
>
> The code from 156 to 162 is repeated 3 times here, is it worth making another
> function for this?
Thanks for the suggestion. I've refactored the code a bit to make it easier to
write new execution modes (which will be needed in Leyden).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1610325214
Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]
On Mon, 20 May 2024 17:24:22 GMT, Ioi Lam 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:
>
> @calvinccheung comments
LGTM, and thanks for the new utilities! That should make writing CDS tests a
lot easier. I have a few style considerations but you can take them or leave
them.
test/lib/jdk/test/lib/cds/CDSAppTester.java line 98:
> 96: }
> 97:
> 98: private final String name;
Could these fields and the constructor be moved to the top of the class?
test/lib/jdk/test/lib/cds/CDSAppTester.java line 147:
> 145: }
> 146:
> 147: private OutputAnalyzer dumpStaticArchive() throws Exception {
The code from 156 to 162 is repeated 3 times here, is it worth making another
function for this?
-
Marked as reviewed by matsaave (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/19256#pullrequestreview-2069620296
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1608953801
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1608949352
Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]
On Mon, 20 May 2024 17:24:22 GMT, Ioi Lam 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: > > @calvinccheung comments Updates look good. Thanks! - Marked as reviewed by ccheung (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19256#pullrequestreview-2069075764
Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]
> 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: @calvinccheung comments - Changes: - all: https://git.openjdk.org/jdk/pull/19256/files - new: https://git.openjdk.org/jdk/pull/19256/files/354665a3..ad97efa1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19256=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19256=01-02 Stats: 14 lines in 3 files changed: 5 ins; 6 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19256.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19256/head:pull/19256 PR: https://git.openjdk.org/jdk/pull/19256
