Re: RFR: JDK-8306441: Two phase segmented heap dump [v20]

2023-07-31 Thread Yi Yang
On Wed, 19 Jul 2023 12:42:27 GMT, Yi Yang  wrote:

>> ### Motivation and proposal
>> Hi, heap dump brings about pauses for application's execution(STW), this is 
>> a well-known pain. JDK-8252842 have added parallel support to heapdump in an 
>> attempt to alleviate this issue. However, all concurrent threads 
>> competitively write heap data to the same file, and more memory is required 
>> to maintain the concurrent buffer queue. In experiments, we did not feel a 
>> significant performance improvement from that.
>> 
>> The minor-pause solution, which is presented in this PR, is a two-phase 
>> segmented heap dump:
>> 
>> - Phase 1(STW): Concurrent threads directly write data to multiple heap 
>> files.
>> - Phase 2(Non-STW): Merge multiple heap files into one complete heap dump 
>> file. This process can happen outside safepoint.
>> 
>> Now concurrent worker threads are not required to maintain a buffer queue, 
>> which would result in more memory overhead, nor do they need to compete for 
>> locks. The changes in the overall design are as follows:
>> 
>> ![image](https://github.com/openjdk/jdk/assets/5010047/77e4764a-62b5-4336-8b45-fc880ba14c4a)
>> Fig1. Before
>> 
>> ![image](https://github.com/openjdk/jdk/assets/5010047/931ab874-64d1-4337-ae32-3066eed809fc)
>> Fig2. After this patch
>> 
>> ### Performance evaluation
>> | memory | numOfThread | CompressionMode | STW | Total |
>> | ---| --- | --- | --- |  |
>> | 8g | 1 T | N | 15.612 | 15.612 |
>> | 8g | 32 T | N | 2.561725 | 14.498 |
>> | 8g | 32 T | C1 | 2.3084878 | 14.198 |
>> | 8g | 32 T | C2 | 10.9355128 | 21.882 |
>> | 8g | 96 T | N | 2.6790452 | 14.012 |
>> | 8g | 96 T | C1 | 2.3044796 | 3.589 |
>> | 8g | 96 T | C2 | 9.7585151 | 20.219 |
>> | 16g | 1 T | N | 26.278 | 26.278 |
>> | 16g | 32 T | N | 5.231374 | 26.417 |
>> | 16g | 32 T | C1 | 5.6946983 | 6.538 |
>> | 16g | 32 T | C2 | 21.8211105 | 41.133 |
>> | 16g | 96 T | N | 6.2445556 | 27.141 |
>> | 16g | 96 T | C1 | 4.6007096 | 6.259 |
>> | 16g | 96 T | C2 | 19.2965783 | 39.007 |
>> | 32g | 1 T | N | 48.149 | 48.149 |
>> | 32g | 32 T | N | 10.7734677 | 61.643 |
>> | 32g | 32 T | C1 | 10.1642097 | 10.903 |
>> | 32g | 32 T | C2 | 43.8407607 | 88.152 |
>> | 32g | 96 T | N | 13.1522042 | 61.432 |
>> | 32g | 96 T | C1 | 9.0954641 | 9.885 |
>> | 32g | 96 T | C2 | 38.9900931 | 80.574 |
>> | 64g | 1 T | N | 100.583 | 100.583 |
>> | 64g | 32 T | N | 20.9233744 | 134.701 |
>> | 64g | 32 T | C1 | 18.5023784 | 19.358 |
>> | 64g | 32 T | C2 | 86.4748377 | 172.707 |
>> | 64g | 96 T | N | 26.7374116 | 126.08 |
>> | 64g | ...
>
> Yi Yang has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   test failure on Windows

Hi Kevin, 

1. CSR

> Actually are we changing the default behaviour: from "parallel if you can", 
> to "serial unless you specify".

We are not changing the default behavior. Now the default is still parallel 
(unless using SerialGC or parallel can not work), the VM will automatically 
calculate the appropriate parallelism unless the user specifies it.

Currently, this patch only adds the -parallel option to jcmd, AFAIK, this does 
not require a CSR. Only modifying jmap would require a CSR.

Given the compatibility issues previously mentioned in 
https://github.com/openjdk/jdk/pull/2261#discussion_r565295921 and subsequent 
bugs such as [JDK-8219721](https://bugs.openjdk.java.net/browse/JDK-8219721) 
[JDK-8219896](https://bugs.openjdk.java.net/browse/JDK-8219896), I'm hesitant 
to add a -parallel option to jmap, as it could lead to similar compatibility 
issues and bugs. Therefore, the best approach may be to keep it as it is:

- For jmap, VM calculates the number of parallel threads (and fallback to 
serial when unable to parallelize)
- For jcmd, VM provides fine-grained control, i.e. allowing -parallel=1-X to 
control whether to use serial or parallel.

Do you think this is acceptable?

2. Test

> Running the changes, I got a test failure on macos debug (x64 and aarch64):
> test/hotspot/jtreg/serviceability/HeapDump/IntegrityHeapDumpTest.java

Fixed.

The reason is that `SATestUtils.createProcessBuilder` in IntegrityHeapDump.java 
may require [user input for a 
password](https://github.com/y1yang0/jdk/actions/runs/5599168245/job/1511990#step:9:978)
 when starting a process. I have modified test to be consistent with 
DuplicateClassArrayClassesTest.java to avert this problem.

3. Release Note

>This enhancement should probably have a release note, and we could briefly 
>explain what parallel means.
> I can help with the wording if needed, if you'd like to start something

Thank you in advance! I've created related release-note issue, and I'll draft 
an initial version for this, please feel free to rephrase it.

-

PR Comment: https://git.openjdk.org/jdk/pull/13667#issuecomment-1659605530


Re: RFR: JDK-8306441: Two phase segmented heap dump [v21]

2023-07-31 Thread Yi Yang
> ### Motivation and proposal
> Hi, heap dump brings about pauses for application's execution(STW), this is a 
> well-known pain. JDK-8252842 have added parallel support to heapdump in an 
> attempt to alleviate this issue. However, all concurrent threads 
> competitively write heap data to the same file, and more memory is required 
> to maintain the concurrent buffer queue. In experiments, we did not feel a 
> significant performance improvement from that.
> 
> The minor-pause solution, which is presented in this PR, is a two-phase 
> segmented heap dump:
> 
> - Phase 1(STW): Concurrent threads directly write data to multiple heap files.
> - Phase 2(Non-STW): Merge multiple heap files into one complete heap dump 
> file. This process can happen outside safepoint.
> 
> Now concurrent worker threads are not required to maintain a buffer queue, 
> which would result in more memory overhead, nor do they need to compete for 
> locks. The changes in the overall design are as follows:
> 
> ![image](https://github.com/openjdk/jdk/assets/5010047/77e4764a-62b5-4336-8b45-fc880ba14c4a)
> Fig1. Before
> 
> ![image](https://github.com/openjdk/jdk/assets/5010047/931ab874-64d1-4337-ae32-3066eed809fc)
> Fig2. After this patch
> 
> ### Performance evaluation
> | memory | numOfThread | CompressionMode | STW | Total |
> | ---| --- | --- | --- |  |
> | 8g | 1 T | N | 15.612 | 15.612 |
> | 8g | 32 T | N | 2.561725 | 14.498 |
> | 8g | 32 T | C1 | 2.3084878 | 14.198 |
> | 8g | 32 T | C2 | 10.9355128 | 21.882 |
> | 8g | 96 T | N | 2.6790452 | 14.012 |
> | 8g | 96 T | C1 | 2.3044796 | 3.589 |
> | 8g | 96 T | C2 | 9.7585151 | 20.219 |
> | 16g | 1 T | N | 26.278 | 26.278 |
> | 16g | 32 T | N | 5.231374 | 26.417 |
> | 16g | 32 T | C1 | 5.6946983 | 6.538 |
> | 16g | 32 T | C2 | 21.8211105 | 41.133 |
> | 16g | 96 T | N | 6.2445556 | 27.141 |
> | 16g | 96 T | C1 | 4.6007096 | 6.259 |
> | 16g | 96 T | C2 | 19.2965783 | 39.007 |
> | 32g | 1 T | N | 48.149 | 48.149 |
> | 32g | 32 T | N | 10.7734677 | 61.643 |
> | 32g | 32 T | C1 | 10.1642097 | 10.903 |
> | 32g | 32 T | C2 | 43.8407607 | 88.152 |
> | 32g | 96 T | N | 13.1522042 | 61.432 |
> | 32g | 96 T | C1 | 9.0954641 | 9.885 |
> | 32g | 96 T | C2 | 38.9900931 | 80.574 |
> | 64g | 1 T | N | 100.583 | 100.583 |
> | 64g | 32 T | N | 20.9233744 | 134.701 |
> | 64g | 32 T | C1 | 18.5023784 | 19.358 |
> | 64g | 32 T | C2 | 86.4748377 | 172.707 |
> | 64g | 96 T | N | 26.7374116 | 126.08 |
> | 64g | 96 T | C1 | 16.8101551 | 17.938 |
> | 64g | 96 T | C2 | 80.1626621 | 169.003 |
> | 128g | 1 T | ...

Yi Yang has updated the pull request incrementally with one additional commit 
since the last revision:

  test failure on mac

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/13667/files
  - new: https://git.openjdk.org/jdk/pull/13667/files/469be625..408e86df

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=13667&range=20
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13667&range=19-20

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/13667.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13667/head:pull/13667

PR: https://git.openjdk.org/jdk/pull/13667


Re: RFR: JDK-8306441: Two phase segmented heap dump [v20]

2023-07-31 Thread Kevin Walls
On Wed, 19 Jul 2023 12:42:27 GMT, Yi Yang  wrote:

>> ### Motivation and proposal
>> Hi, heap dump brings about pauses for application's execution(STW), this is 
>> a well-known pain. JDK-8252842 have added parallel support to heapdump in an 
>> attempt to alleviate this issue. However, all concurrent threads 
>> competitively write heap data to the same file, and more memory is required 
>> to maintain the concurrent buffer queue. In experiments, we did not feel a 
>> significant performance improvement from that.
>> 
>> The minor-pause solution, which is presented in this PR, is a two-phase 
>> segmented heap dump:
>> 
>> - Phase 1(STW): Concurrent threads directly write data to multiple heap 
>> files.
>> - Phase 2(Non-STW): Merge multiple heap files into one complete heap dump 
>> file. This process can happen outside safepoint.
>> 
>> Now concurrent worker threads are not required to maintain a buffer queue, 
>> which would result in more memory overhead, nor do they need to compete for 
>> locks. The changes in the overall design are as follows:
>> 
>> ![image](https://github.com/openjdk/jdk/assets/5010047/77e4764a-62b5-4336-8b45-fc880ba14c4a)
>> Fig1. Before
>> 
>> ![image](https://github.com/openjdk/jdk/assets/5010047/931ab874-64d1-4337-ae32-3066eed809fc)
>> Fig2. After this patch
>> 
>> ### Performance evaluation
>> | memory | numOfThread | CompressionMode | STW | Total |
>> | ---| --- | --- | --- |  |
>> | 8g | 1 T | N | 15.612 | 15.612 |
>> | 8g | 32 T | N | 2.561725 | 14.498 |
>> | 8g | 32 T | C1 | 2.3084878 | 14.198 |
>> | 8g | 32 T | C2 | 10.9355128 | 21.882 |
>> | 8g | 96 T | N | 2.6790452 | 14.012 |
>> | 8g | 96 T | C1 | 2.3044796 | 3.589 |
>> | 8g | 96 T | C2 | 9.7585151 | 20.219 |
>> | 16g | 1 T | N | 26.278 | 26.278 |
>> | 16g | 32 T | N | 5.231374 | 26.417 |
>> | 16g | 32 T | C1 | 5.6946983 | 6.538 |
>> | 16g | 32 T | C2 | 21.8211105 | 41.133 |
>> | 16g | 96 T | N | 6.2445556 | 27.141 |
>> | 16g | 96 T | C1 | 4.6007096 | 6.259 |
>> | 16g | 96 T | C2 | 19.2965783 | 39.007 |
>> | 32g | 1 T | N | 48.149 | 48.149 |
>> | 32g | 32 T | N | 10.7734677 | 61.643 |
>> | 32g | 32 T | C1 | 10.1642097 | 10.903 |
>> | 32g | 32 T | C2 | 43.8407607 | 88.152 |
>> | 32g | 96 T | N | 13.1522042 | 61.432 |
>> | 32g | 96 T | C1 | 9.0954641 | 9.885 |
>> | 32g | 96 T | C2 | 38.9900931 | 80.574 |
>> | 64g | 1 T | N | 100.583 | 100.583 |
>> | 64g | 32 T | N | 20.9233744 | 134.701 |
>> | 64g | 32 T | C1 | 18.5023784 | 19.358 |
>> | 64g | 32 T | C2 | 86.4748377 | 172.707 |
>> | 64g | 96 T | N | 26.7374116 | 126.08 |
>> | 64g | ...
>
> Yi Yang has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   test failure on Windows

Hi, a few more points today, but I'm getting there. 8-)

1
The previous parallel dump change:
8252842: Extend jmap to support parallel heap dump
..the notes suggest it was going to have a CSR for the new option, but was then 
decided not to add the option to specify the number of threads.
That CSR was https://bugs.openjdk.org/browse/JDK-8260424 which was withdrawn, 
but looks like we should do one for this change.
If we had a reasonable expectation of what the best number to use was, we could 
do the same thing here.  If we might get that wrong, we will need the user to 
be able to specify.
Actually are we changing the default behaviour: from "parallel if you can", to 
"serial unless you specify".


2
Running the changes, I got a test failure on macos debug (x64 and aarch64):

test/hotspot/jtreg/serviceability/HeapDump/IntegrityHeapDumpTest.java

On different machines, one x64 and one aarch64, got the same kind of error:
--stderr:(6/609)--
com.sun.tools.attach.AttachNotSupportedException: Unable to open socket file 
/var/folders/zz/zyxvpxvq6csfxvn_n0/T/.java_pid36008: target process 
36008 doesn't respond within 10500ms or HotSpot VM not loaded
at 
jdk.attach/sun.tools.attach.VirtualMachineImpl.(VirtualMachineImpl.java:95)
at 
jdk.attach/sun.tools.attach.AttachProviderImpl.attachVirtualMachine(AttachProviderImpl.java:58)
at 
jdk.attach/com.sun.tools.attach.VirtualMachine.attach(VirtualMachine.java:207)
at jdk.jcmd/sun.tools.jcmd.JCmd.executeCommandForPid(JCmd.java:113)
at jdk.jcmd/sun.tools.jcmd.JCmd.main(JCmd.java:97)

..so the test fails with: java.lang.RuntimeException: Expected to get exit 
value of [0], exit value is: [1]

Other tests e.g. test/hotspot/jtreg/serviceability/dcmd/gc/HeapDumpTest.java 
run OK.

The failing test IntegrityHeapDumpTest.java extends LingeredApp.
test/hotspot/jtreg/serviceability/HeapDump/DuplicateArrayClassesTest.java does 
this, and runs and passes.

I don't yet understand the failure.  Could things have gone wrong in macosx 
debug builds in the attach listener.. I didn't get a stack of the LingeredApp 
under test, but will try again.


3
Docs
https://openjdk.org/guide/#release-notes
This enhancement should probab

Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread David Holmes
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Thanks for the reviews everyone.

-

PR Comment: https://git.openjdk.org/jdk21/pull/151#issuecomment-1659164663


[jdk21] Integrated: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread David Holmes
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

This pull request has now been integrated.

Changeset: 0439d584
Author:David Holmes 
URL:   
https://git.openjdk.org/jdk21/commit/0439d584221f4d92fd1f4097a331f83dcdb2c12c
Stats: 163 lines in 28 files changed: 37 ins; 60 del; 66 mod

8300937: Update nroff pages in JDK 21 before RC

Reviewed-by: mchung, iris, egahlin

-

PR: https://git.openjdk.org/jdk21/pull/151


Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread Erik Gahlin
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Marked as reviewed by egahlin (Reviewer).

-

PR Review: https://git.openjdk.org/jdk21/pull/151#pullrequestreview-141442


Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread Iris Clark
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Looks good.

The flow changes are unfortunate, but a tool update has the potential to have 
far more wide-spread changes.  I'm happy that's all there was.

-

Marked as reviewed by iris (Reviewer).

PR Review: https://git.openjdk.org/jdk21/pull/151#pullrequestreview-1555365638


Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread Mandy Chung
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Changes in java and jdeps are good.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk21/pull/151#pullrequestreview-1555334390


Re: RFR: 8312623: SA add NestHost and NestMembers attributes when dumping class

2023-07-31 Thread Ashutosh Mehra
On Thu, 27 Jul 2023 22:39:03 GMT, David Holmes  wrote:

>> @dholmes-ora sorry for responding late. I got sidetracked by some other work.
>> 
>>> We need to be sure this works as expected for top-level classes that have 
>>> no nest members, and deeply nested nest members, plus dynamically injected 
>>> hidden classes that are nest members.
>> 
>> I am not sure I understand this concern. We are getting nest-host and 
>> nest-members from the InstanceKlass. As long as this information is recorded 
>> in InstanceKlass, it would work. Can you please elaborate your concern about 
>> the cases you feel may not work.
>> 
>>> I'm unclear if this is intended to only expose the same details as would be 
>>> statically defined in the attribute in the classfile?
>> 
>> It is to expose the details as the JVM sees, which may be different from 
>> what is statically defined in the classfile if agents are involved.
>
> @ashu-mehra you indicated that you had only done two basic manual tests to 
> check the output. You need to check it for the cases that I flagged too. In 
> the VM every top-level class is its own nest-host, but that is not expressed 
> in a classfile attribute (it is just the defined semantics) so displaying 
> this as-if it were an explicit attribute may not be right.

@dholmes-ora I confirmed there is no nest-host or nest-members attributes 
generated by this patch for a top level class which doesn't have any 
nest-members. Is that what you wanted to verify?

-

PR Comment: https://git.openjdk.org/jdk/pull/15005#issuecomment-1658599841


Re: [jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread Daniel Fuchs
On Mon, 31 Jul 2023 08:33:07 GMT, David Holmes  wrote:

> Main changes are to use 21 instead of 21-ea. In addition the following files 
> contain additional updates from the closed sources:
> 
> - src/java.base/share/man/java.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
>  [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
> StartFlightRecording: consider moving mention of multiple comma-separated 
> parameters to the front
> 
> There are also some formatting differences related to:
> 
>  [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help 
> output for --module-path / -p is incomplete
> 
> Likely a different version of pandoc was used.
> 
> - src/java.base/share/man/keytool.1
> 
>  [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
> contains a special character
> 
> - src/jdk.compiler/share/man/javac.1
> 
>  [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac 
> tool page for -proc:full
> 
> also some formatting differences.
> 
> - src/jdk.jartool/share/man/jarsigner.1
> 
>  [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
> man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)
> 
> - src/jdk.jcmd/share/man/jcmd.1
> 
>  [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
> manpage markdown source for JFR filename expansion
> 
> - src/jdk.jdeps/share/man/jdeps.1
> 
>  [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) 
> Deprecate jdeps -profile option
> 
> - src/jdk.jfr/share/man/jfr.1
> 
> Formatting changes.
> 
> - src/jdk.jshell/share/man/jshell.1
> 
>  [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
> manpage to include TOOLING description

Changes to jwebserver.1 look good to me.

-

PR Comment: https://git.openjdk.org/jdk21/pull/151#issuecomment-1658469731


[jdk21] RFR: 8300937: Update nroff pages in JDK 21 before RC

2023-07-31 Thread David Holmes
Main changes are to use 21 instead of 21-ea. In addition the following files 
contain additional updates from the closed sources:

- src/java.base/share/man/java.1

 [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
manpage markdown source for JFR filename expansion
 [JDK-8221633](https://bugs.openjdk.org/browse/JDK-8221633): 
StartFlightRecording: consider moving mention of multiple comma-separated 
parameters to the front

There are also some formatting differences related to:

 [JDK-8309670](https://bugs.openjdk.org/browse/JDK-8309670): java -help output 
for --module-path / -p is incomplete

Likely a different version of pandoc was used.

- src/java.base/share/man/keytool.1

 [JDK-8290626](https://bugs.openjdk.org/browse/JDK-8290626): keytool manpage 
contains a special character

- src/jdk.compiler/share/man/javac.1

 [JDK-8308456](https://bugs.openjdk.org/browse/JDK-8308456): Update javac tool 
page for -proc:full

also some formatting differences.

- src/jdk.jartool/share/man/jarsigner.1

 [JDK-8303928](https://bugs.openjdk.org/browse/JDK-8303928): Update jarsigner 
man page after [JDK-8303410](https://bugs.openjdk.org/browse/JDK-8303410)

- src/jdk.jcmd/share/man/jcmd.1

 [JDK-8273131](https://bugs.openjdk.org/browse/JDK-8273131): Update the java 
manpage markdown source for JFR filename expansion

- src/jdk.jdeps/share/man/jdeps.1

 [JDK-8301207](https://bugs.openjdk.org/browse/JDK-8301207): (jdeps) Deprecate 
jdeps -profile option

- src/jdk.jfr/share/man/jfr.1

Formatting changes.

- src/jdk.jshell/share/man/jshell.1

 [JDK-8308988](https://bugs.openjdk.org/browse/JDK-8308988): Update JShell 
manpage to include TOOLING description

-

Commit messages:
 - 8300937: Update nroff pages in JDK 21 before RC

Changes: https://git.openjdk.org/jdk21/pull/151/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=151&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8300937
  Stats: 163 lines in 28 files changed: 37 ins; 60 del; 66 mod
  Patch: https://git.openjdk.org/jdk21/pull/151.diff
  Fetch: git fetch https://git.openjdk.org/jdk21.git pull/151/head:pull/151

PR: https://git.openjdk.org/jdk21/pull/151