Skip to content
This repository has been archived by the owner. It is now read-only.

8266614: update manpage for -Xlog:async #16

Closed
wants to merge 3 commits into from

Conversation

@dholmes-ora
Copy link
Member

@dholmes-ora dholmes-ora commented Jun 11, 2021

Please review this update to the java manpage to describe the new -Xlog:async flag

There are two places where the text is changed:

  1. At the start where the -Xlog synopsis is given it now shows that -Xlog:directive is an allowed form where directive can be one of: help, disable, async
  2. A new subsection "-Xlog Output Mode" that explains async mode

The commited file is the java.1 nroff version which is not very readable, so I've included a commit that also contains a html version with the changed text flagged by "START NEW TEXT" and "END NEW TEXT". You can view that in rendered html via this link:

https://htmlpreview.github.io/?https://github.com/openjdk/jdk17/blob/8dcf544dfd2e19a3a49cce98d2d9abd9d2756538/java.html

Note that because the nroff file has not been updated for a while it also contains changes unrelated to this PR, the source changes for which have already been reviewed and approved. So just ignore those bits and look at the html file.

Thanks,
David


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk17 pull/16/head:pull/16
$ git checkout pull/16

Update a local copy of the PR:
$ git checkout pull/16
$ git pull https://git.openjdk.java.net/jdk17 pull/16/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16

View PR using the GUI difftool:
$ git pr show -t 16

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk17/pull/16.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jun 11, 2021

👋 Welcome back dholmes! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Jun 11, 2021

@dholmes-ora To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command.

Applicable Labels
  • 2d
  • awt
  • beans
  • build
  • compiler
  • core-libs
  • hotspot
  • hotspot-compiler
  • hotspot-gc
  • hotspot-jfr
  • hotspot-runtime
  • i18n
  • javadoc
  • jdk
  • jmx
  • kulla
  • net
  • nio
  • security
  • serviceability
  • shenandoah
  • sound
  • swing

Loading

@dholmes-ora
Copy link
Member Author

@dholmes-ora dholmes-ora commented Jun 11, 2021

/label add hotspot

Loading

@openjdk openjdk bot added the hotspot label Jun 11, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jun 11, 2021

@dholmes-ora
The hotspot label was successfully added.

Loading

@dholmes-ora dholmes-ora marked this pull request as ready for review Jun 11, 2021
@openjdk openjdk bot added the rfr label Jun 11, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 11, 2021

Webrevs

Loading

@hseigel
Copy link
Member

@hseigel hseigel commented Jun 11, 2021

The changes look good! Here's some optional suggestions.

Under the "Description" heading where it says "The following provides quick reference to the -Xlog command and syntax for options:", perhaps add something about -Xlog:async?

Possible rewording suggestions:

  1. Change "The default value should be big enough to cater for most cases." to
    "The default value should be big enough to handle most cases."

  2. Change "... trade memory overhead for log accuracy if they need to."
    to "... trade memory overhead for log accuracy if needed."

Loading

Copy link
Member

@hseigel hseigel left a comment

Thanks for doing this!
Harold

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Jun 11, 2021

@dholmes-ora This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8266614: update manpage for -Xlog:async

Reviewed-by: hseigel, xliu

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 13 new commits pushed to the master branch:

  • c088d09: 8268522: InstanceKlass::can_be_verified_at_dumptime() returns opposite value
  • abe20c1: 8268333: javac crashes when pattern matching switch contains default case which is not last
  • b318535: 8267579: Thread::cooked_allocated_bytes() hits assert(left >= right) failed: avoid underflow
  • fe48ea9: 8268342: java/foreign/channels/TestAsyncSocketChannels.java fails with "IllegalStateException: This segment is already closed"
  • 6171ae4: 8268630: ProblemList serviceability/jvmti/CompiledMethodLoad/Zombie.java on linux-aarch64
  • 01054e6: 8268470: CDS dynamic dump asserts with JFR RecordingStream
  • e39346e: 8268093: Manual Testcase: "sun/security/krb5/config/native/TestDynamicStore.java" Fails with NPE
  • cce8da2: 8268602: a couple runtime/os tests don't check exit code
  • da043e9: 8268555: Update HttpClient tests that use ITestContext to jtreg 6+1
  • a437ce8: 8268580: runtime/memory/LargePages/TestLargePagesFlags.java should be run in driver mode
  • ... and 3 more: https://git.openjdk.java.net/jdk17/compare/53b6e2c85cab251362d27a1cd0cd37bc7d380360...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

Loading

@openjdk openjdk bot added the ready label Jun 11, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 11, 2021

Mailing list message from David Holmes on hotspot-dev:

Hi Harold,

On 12/06/2021 6:30 am, Harold Seigel wrote:

On Fri, 11 Jun 2021 04:40:09 GMT, David Holmes <dholmes at openjdk.org> wrote:

Please review this update to the java manpage to describe the new -Xlog:async flag

There are two places where the text is changed:

1. At the start where the -Xlog synopsis is given it now shows that `-Xlog:directive` is an allowed form where directive can be one of: help, disable, async
2. A new subsection "-Xlog Output Mode" that explains async mode

The commited file is the java.1 nroff version which is not very readable, so I've included a commit that also contains a html version with the changed text flagged by "START NEW TEXT" and "END NEW TEXT". You can view that in rendered html via this link:

https://htmlpreview.github.io/?https://github.com/openjdk/jdk17/blob/8dcf544dfd2e19a3a49cce98d2d9abd9d2756538/java.html

Note that because the nroff file has not been updated for a while it also contains changes unrelated to this PR, the source changes for which have already been reviewed and approved. So just ignore those bits and look at the html file.

Thanks,
David

Thanks for doing this!
Harold

The changes look good!

Thanks for the review.

Here's some optional suggestions.

Under the "Description" heading where it says "The following provides quick reference to the -Xlog command and syntax for options:", perhaps add something about -Xlog:async?

I think that would just duplicate what is said later.

Possible rewording suggestions:

1. Change "The default value should be big enough to cater for most cases." to
"The default value should be big enough to handle most cases."

2. Change "... trade memory overhead for log accuracy if they need to."
to "... trade memory overhead for log accuracy if needed."

The wording is consistent with the help text which itself was set via a
CSR request, so I'll jsut leave it as-is.

Thanks,
David

Loading

1 similar comment
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 11, 2021

Mailing list message from David Holmes on hotspot-dev:

Hi Harold,

On 12/06/2021 6:30 am, Harold Seigel wrote:

On Fri, 11 Jun 2021 04:40:09 GMT, David Holmes <dholmes at openjdk.org> wrote:

Please review this update to the java manpage to describe the new -Xlog:async flag

There are two places where the text is changed:

1. At the start where the -Xlog synopsis is given it now shows that `-Xlog:directive` is an allowed form where directive can be one of: help, disable, async
2. A new subsection "-Xlog Output Mode" that explains async mode

The commited file is the java.1 nroff version which is not very readable, so I've included a commit that also contains a html version with the changed text flagged by "START NEW TEXT" and "END NEW TEXT". You can view that in rendered html via this link:

https://htmlpreview.github.io/?https://github.com/openjdk/jdk17/blob/8dcf544dfd2e19a3a49cce98d2d9abd9d2756538/java.html

Note that because the nroff file has not been updated for a while it also contains changes unrelated to this PR, the source changes for which have already been reviewed and approved. So just ignore those bits and look at the html file.

Thanks,
David

Thanks for doing this!
Harold

The changes look good!

Thanks for the review.

Here's some optional suggestions.

Under the "Description" heading where it says "The following provides quick reference to the -Xlog command and syntax for options:", perhaps add something about -Xlog:async?

I think that would just duplicate what is said later.

Possible rewording suggestions:

1. Change "The default value should be big enough to cater for most cases." to
"The default value should be big enough to handle most cases."

2. Change "... trade memory overhead for log accuracy if they need to."
to "... trade memory overhead for log accuracy if needed."

The wording is consistent with the help text which itself was set via a
CSR request, so I'll jsut leave it as-is.

Thanks,
David

Loading

@dholmes-ora
Copy link
Member Author

@dholmes-ora dholmes-ora commented Jun 13, 2021

@navyxliu could you review this please.

Thanks,
David

Loading

@navyxliu
Copy link
Contributor

@navyxliu navyxliu commented Jun 14, 2021

Thanks for updating the manage.
LGTM.

Loading

@dholmes-ora
Copy link
Member Author

@dholmes-ora dholmes-ora commented Jun 14, 2021

/integrate

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Jun 14, 2021

Going to push as commit a5bf5e0.
Since your change was applied there have been 17 commits pushed to the master branch:

  • f69e2d5: 8267397: AlgorithmId's OID cache is never refreshed
  • ee30159: 8268621: SunJCE provider may throw unexpected NPE for un-initialized AES KW/KWP Ciphers
  • 702e3ff: 8268366: Incorrect calculation of has_fpu_registers in C1 linear scan
  • bca914b: 8268670: yield statements doesn't allow ~ or ! unary operators in expression
  • c088d09: 8268522: InstanceKlass::can_be_verified_at_dumptime() returns opposite value
  • abe20c1: 8268333: javac crashes when pattern matching switch contains default case which is not last
  • b318535: 8267579: Thread::cooked_allocated_bytes() hits assert(left >= right) failed: avoid underflow
  • fe48ea9: 8268342: java/foreign/channels/TestAsyncSocketChannels.java fails with "IllegalStateException: This segment is already closed"
  • 6171ae4: 8268630: ProblemList serviceability/jvmti/CompiledMethodLoad/Zombie.java on linux-aarch64
  • 01054e6: 8268470: CDS dynamic dump asserts with JFR RecordingStream
  • ... and 7 more: https://git.openjdk.java.net/jdk17/compare/53b6e2c85cab251362d27a1cd0cd37bc7d380360...master

Your commit was automatically rebased without conflicts.

Loading

@openjdk openjdk bot closed this Jun 14, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Jun 14, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jun 14, 2021

@dholmes-ora Pushed as commit a5bf5e0.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Loading

@dholmes-ora dholmes-ora deleted the 8266614 branch Jun 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants