RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit

Yumin Qi-3
Hi, Please review
  Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
  archive names (static/dynamic or static only)
  loaded classes and their loaders
  number of shared symbols
  number of shared strings
  full vm version stored in shared archive.
  added two tests for custom loader test and dynamic test.

  Tests: tier1,tier2,tier3,tier4

Thanks
Yumin

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

Commit messages:
 - Fix extra space at end of line
 - 8244540: Print more information with -XX:+PrintSharedArchiveAndExit

Changes: https://git.openjdk.java.net/jdk/pull/3187/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3187&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8244540
  Stats: 275 lines in 7 files changed: 266 ins; 0 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3187.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3187/head:pull/3187

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit

Ioi Lam-2
On Thu, 25 Mar 2021 03:54:07 GMT, Yumin Qi <[hidden email]> wrote:

> Hi, Please review
>   Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
>   archive names (static/dynamic or static only)
>   loaded classes and their loaders
>   number of shared symbols
>   number of shared strings
>   full vm version stored in shared archive.
>   added two tests for custom loader test and dynamic test.
>
>   Tests: tier1,tier2,tier3,tier4
>
> Thanks
> Yumin

Looks good overall. I think the order of the output can be changed a little to make it more readable:

VM version: Java HotSpot(TM) 64-Bit Server VM (fastdebug ......

Number of shared symbols: 40699
Number of shared strings: 7601

Base archive name: /jdk2/bld/fastdebug/images/jdk/lib/server/classes.jsa
Base archive version 11

(classes in the base archive)


Dynamic archive name: PrintSharedArchiveAndExit.java-top.jsa
Dynamic archive version 11

(classes in the dynamic archive)

Also, now the counter is reset when you go from Unregistered to Lambda. I think it shouldn't be reset so we can easily tell the combined number of classes in each archive file.

  17: sun.hotspot.WhiteBox loaded by: boot_loader
Shared Unregistered Dictionary
  18: CustomLoadee loaded by: unregistered_loader
Shared Lambda Dictionary
   1: sun.hotspot.WhiteBox$$Lambda$10/0x0000000801045b88 loaded by: boot_loader

src/hotspot/share/classfile/systemDictionaryShared.cpp line 2253:

> 2251:     ResourceMark rm;
> 2252:     _st->print_cr("%4d: %s loaded by: %s", (_index++), record->_klass->external_name(),
> 2253:         class_loader_name_for_shared(record->_klass));

The "loaded by: xxx_loader" seems a bit verbose:

  23: java.lang.invoke.VarHandleLongs$FieldInstanceReadWrite loaded by: boot_loader
  24: java.util.Map loaded by: boot_loader

How about

  23: java.lang.invoke.VarHandleLongs$FieldInstanceReadWrite boot_loader
  24: java.util.Map boot_loader

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

Changes requested by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit [v2]

Yumin Qi-3
In reply to this post by Yumin Qi-3
> Hi, Please review
>   Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
>   archive names (static/dynamic or static only)
>   loaded classes and their loaders
>   number of shared symbols
>   number of shared strings
>   full vm version stored in shared archive.
>   added two tests for custom loader test and dynamic test.
>
>   Tests: tier1,tier2,tier3,tier4
>
> Thanks
> Yumin

Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:

  Fixed class output with no reset index. Separated output from dynamic from static. Removed verbose loaded_by from output.

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3187/files
  - new: https://git.openjdk.java.net/jdk/pull/3187/files/f424cfc6..1a68b5ec

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3187&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3187&range=00-01

  Stats: 29 lines in 5 files changed: 17 ins; 2 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3187.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3187/head:pull/3187

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit [v2]

Ioi Lam-2
On Tue, 30 Mar 2021 03:40:33 GMT, Yumin Qi <[hidden email]> wrote:

>> Hi, Please review
>>   Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
>>   archive names (static/dynamic or static only)
>>   loaded classes and their loaders
>>   number of shared symbols
>>   number of shared strings
>>   full vm version stored in shared archive.
>>   added two tests for custom loader test and dynamic test.
>>
>>   Tests: tier1,tier2,tier3,tier4
>>
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fixed class output with no reset index. Separated output from dynamic from static. Removed verbose loaded_by from output.

LGTM

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

Marked as reviewed by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit [v2]

Calvin Cheung
In reply to this post by Yumin Qi-3
On Tue, 30 Mar 2021 03:40:33 GMT, Yumin Qi <[hidden email]> wrote:

>> Hi, Please review
>>   Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
>>   archive names (static/dynamic or static only)
>>   loaded classes and their loaders
>>   number of shared symbols
>>   number of shared strings
>>   full vm version stored in shared archive.
>>   added two tests for custom loader test and dynamic test.
>>
>>   Tests: tier1,tier2,tier3,tier4
>>
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fixed class output with no reset index. Separated output from dynamic from static. Removed verbose loaded_by from output.

Looks good. Just a small nit on the test cases.

Thanks,
Calvin

test/hotspot/jtreg/runtime/cds/appcds/customLoader/PrintSharedArchiveAndExit.java line 85:

> 83:               .shouldContain("Shared Unregistered Dictionary")
> 84:               .shouldMatch("Number of shared symbols: \\d+")
> 85:               .shouldMatch("Number of shared strings: \\d+");

Since the print output includes "VM version", perhaps the above should look for it?

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/PrintSharedArchiveAndExit.java line 93:

> 91:                       .shouldContain("CustomLoadee unregistered_loader")
> 92:                       .shouldContain("Shared Builtin Dictionary")
> 93:                       .shouldContain("Shared Unregistered Dictionary")

Since the print output includes "VM version", perhaps the above should look for it?

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

Marked as reviewed by ccheung (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit [v2]

Yumin Qi-3
On Tue, 30 Mar 2021 20:53:22 GMT, Calvin Cheung <[hidden email]> wrote:

>> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fixed class output with no reset index. Separated output from dynamic from static. Removed verbose loaded_by from output.
>
> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/PrintSharedArchiveAndExit.java line 93:
>
>> 91:                       .shouldContain("CustomLoadee unregistered_loader")
>> 92:                       .shouldContain("Shared Builtin Dictionary")
>> 93:                       .shouldContain("Shared Unregistered Dictionary")
>
> Since the print output includes "VM version", perhaps the above should look for it?

Thanks, will add the check.

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

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit [v3]

Yumin Qi-3
In reply to this post by Yumin Qi-3
> Hi, Please review
>   Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
>   archive names (static/dynamic or static only)
>   loaded classes and their loaders
>   number of shared symbols
>   number of shared strings
>   full vm version stored in shared archive.
>   added two tests for custom loader test and dynamic test.
>
>   Tests: tier1,tier2,tier3,tier4
>
> Thanks
> Yumin

Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:

  Add matching VM version string in output

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3187/files
  - new: https://git.openjdk.java.net/jdk/pull/3187/files/1a68b5ec..a24c7c86

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3187&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3187&range=01-02

  Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3187.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3187/head:pull/3187

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit [v2]

Yumin Qi-3
In reply to this post by Calvin Cheung
On Tue, 30 Mar 2021 20:52:37 GMT, Calvin Cheung <[hidden email]> wrote:

>> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fixed class output with no reset index. Separated output from dynamic from static. Removed verbose loaded_by from output.
>
> test/hotspot/jtreg/runtime/cds/appcds/customLoader/PrintSharedArchiveAndExit.java line 85:
>
>> 83:               .shouldContain("Shared Unregistered Dictionary")
>> 84:               .shouldMatch("Number of shared symbols: \\d+")
>> 85:               .shouldMatch("Number of shared strings: \\d+");
>
> Since the print output includes "VM version", perhaps the above should look for it?

Done. Thanks.

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

PR: https://git.openjdk.java.net/jdk/pull/3187
Reply | Threaded
Open this post in threaded view
|

Integrated: 8244540: Print more information with -XX:+PrintSharedArchiveAndExit

Yumin Qi-3
In reply to this post by Yumin Qi-3
On Thu, 25 Mar 2021 03:54:07 GMT, Yumin Qi <[hidden email]> wrote:

> Hi, Please review
>   Added more info in printout when -XX:+PrintSharedArchiveAndExit used:
>   archive names (static/dynamic or static only)
>   loaded classes and their loaders
>   number of shared symbols
>   number of shared strings
>   full vm version stored in shared archive.
>   added two tests for custom loader test and dynamic test.
>
>   Tests: tier1,tier2,tier3,tier4
>
> Thanks
> Yumin

This pull request has now been integrated.

Changeset: 928fa5b5
Author:    Yumin Qi <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/928fa5b5
Stats:     294 lines in 8 files changed: 283 ins; 0 del; 11 mod

8244540: Print more information with -XX:+PrintSharedArchiveAndExit

Reviewed-by: iklam, ccheung

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

PR: https://git.openjdk.java.net/jdk/pull/3187