Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]

2024-03-28 Thread Matthias Baesken
On Wed, 27 Mar 2024 23:26:14 GMT, Chris Plummer  wrote:

> 
> Backporting also came to mind since the trouble shooting guide would need to 
> be updated for each Oracle version this PR is backported to. And yes, 
> HeapDumpGzipLevel docs need to match the actual implementation. I'm not sure 
> if HeapDumpGzipLevel has been backported to 8, 11, and 17 (in the open for 
> for the Oracle release). I'm inclined to say a separate jbs issue should 
> track updating docs for HeapDumpGzipLevel, but then we also have the question 
> of whether or not HeapDumpGzipLevel should impact the jcmd if this PR is to 
> be pushed.

HeapDumpGzipLevel is in openjdk17 and openjdk21 .

-

PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2024698315


Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]

2024-03-27 Thread Chris Plummer
On Wed, 27 Mar 2024 12:36:31 GMT, Matthias Baesken  wrote:

> Sorry maybe it is maybe obvious for you but where should I open a "docs CR", 
> would that be a separate JBS issue with Component name docs ?

You would file a JBS issue with component "docs" and subcategory "hotspot". But 
let's hold off on that for now until if it is decided if the PR is going to be 
pushed. My main reason for mentioning it was to point out additional fallout of 
this change.

> Should I include the HeapDumpGzipLevel in the same "docs CR" (but this might 
> be bad for potential backporting) ?

Backporting also came to mind since the trouble shooting guide would need to be 
updated for each Oracle version this PR is backported to. And yes, 
HeapDumpGzipLevel docs need to match the actual implementation. I'm not sure if 
HeapDumpGzipLevel has been backported to 8, 11, and 17 (in the open for for the 
Oracle release).  I'm inclined to say a separate jbs issue should track 
updating docs for HeapDumpGzipLevel, but then we also have the question of 
whether or not HeapDumpGzipLevel should impact the jcmd if this PR is to be 
pushed.

-

PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2024147551


Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]

2024-03-27 Thread Matthias Baesken
On Tue, 26 Mar 2024 21:42:15 GMT, Chris Plummer  wrote:

> A docs CR will need to be filed to get it updated (and I see it is already 
> appears out-of-date w.r.t. HeapDumpGzipLevel)

Sorry maybe it is maybe obvious for you but where should I open a "docs CR", 
would that be a separate JBS issue with Component name docs ?
Should I include the HeapDumpGzipLevel in the same "docs CR" (but this might be 
bad for potential backporting) ?

-

PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2022660444


Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]

2024-03-26 Thread Chris Plummer
On Tue, 26 Mar 2024 13:57:55 GMT, Matthias Baesken  wrote:

>> Currently jcmd command GC.heap_dump only works with an additionally provided 
>> file name.
>> Syntax : GC.heap_dump [options] 
>> 
>> In case the JVM has the XX - flag HeapDumpPath set, we should support an 
>> additional mode where the  is optional.
>> In case the filename is NOT set, we take the HeapDumpPath (file or 
>> directory);
>> 
>> new syntax :
>> GC.heap_dump [options]  .. has precedence over second option
>> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set
>> 
>> This would be a simplification e.g. for support cases where a filename or 
>> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the 
>> path is intended/recommended for usage also in the jcmd case.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   suggestions from Chris for diagnosticCommand.cpp

We have a couple of additional documents that need to be updated:

https://docs.oracle.com/en/java/javase/21/troubleshoot/command-line-options1.html#GUID-A84ECBFB-B6CF-44C3-B627-58BB509C8D05

This is an Oracle produced document. A `docs` CR will need to be filed to get 
it updated (and I see it is already appears out-of-date w.r.t. 
`HeapDumpGzipLevel`)

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

Much like jcmd.1, this one is also derived from a closed markdown file, so 
eventually someone internal at Oracle will need to update the markdown file, 
but we need to see the proposed changes in the java.1 file first. This document 
is also missing the `HeapDumpGzipLevel` update.

If your counting, there are 7 places where documentation will need updating

1. Oracle trouble shooting guide
2. open java.1
3. closed markdown file that java.1 is generated from
4. open jcmd.1
5. closed markdown file that jcmd.1 is generated from
6. globals.hpp
7. jcmd help output

-

PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2021523855


Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]

2024-03-26 Thread Matthias Baesken
> Currently jcmd command GC.heap_dump only works with an additionally provided 
> file name.
> Syntax : GC.heap_dump [options] 
> 
> In case the JVM has the XX - flag HeapDumpPath set, we should support an 
> additional mode where the  is optional.
> In case the filename is NOT set, we take the HeapDumpPath (file or directory);
> 
> new syntax :
> GC.heap_dump [options]  .. has precedence over second option
> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set
> 
> This would be a simplification e.g. for support cases where a filename or 
> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the 
> path is intended/recommended for usage also in the jcmd case.

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  suggestions from Chris for diagnosticCommand.cpp

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18190/files
  - new: https://git.openjdk.org/jdk/pull/18190/files/8b48c911..b414b1be

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18190&range=08
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18190&range=07-08

  Stats: 12 lines in 1 file changed: 0 ins; 3 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/18190.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190

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