8253641: Missing newline in the printout of certain JFR events #248

mbien · 2020-09-18T11:29:21Z

Trivial fix which adds a missing new line to PrettyWriter::printClassLoader if clazz == null.

PrettyWriter is used for both the jfr CLI tool and also for the impl of the public JFR API behind jdk.jfr.Event::toString().

how to reproduce using JFR CLI:

To generate a JFR dump for testing, simply start HelloWorld.java

public class HelloWorld {
    public static void main(String[] args) {System.out.println("hello there");}
}

with
jdk-15+36/bin/java -XX:StartFlightRecording=filename=dump.jfr,dumponexit=true HelloWorld.java

current result for events where the classLoader field equals null (note: newline missing):

jdk-15+36/bin/jfr print --events jdk.ModuleRequire dump.jfr
...
jdk.ModuleRequire {
  startTime = 16:12:43.524
  source = {
    name = "jdk.nio.mapmode"
    version = "15"
    location = "jrt:/jdk.nio.mapmode"
    classLoader = null  }
  requiredModule = {
    name = "java.base"
    version = "15"
    location = "jrt:/java.base"
    classLoader = null  }
}
...

after the patch, using same jfr dump and displaying the same event:

jdk16/build/linux-x86_64-server-release/jdk/bin/jfr print --events jdk.ModuleRequire dump.jfr
...
jdk.ModuleRequire {
  startTime = 16:12:43.524
  source = {
    name = "jdk.nio.mapmode"
    version = "15"
    location = "jrt:/jdk.nio.mapmode"
    classLoader = null
  }
  requiredModule = {
    name = "java.base"
    version = "15"
    location = "jrt:/java.base"
    classLoader = null
  }
}
...

sidenote:

There would be an opportunity to merge the two sequential if-else blocks[1] in printClassLoader over the clazz variable after this patch, but this would obfuscate the actual bug fix, so I decided to omit it. Let me know if I should add that.
[1] https://github.com/mbien/jdk/blob/67750e2b599b3f2a6b814d7bceada785ca4051f9/src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java#L440-L448

Progress

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

Issue

JDK-8253641: Missing newline in the printout of certain JFR events

Reviewers

Markus Grönlund (@mgronlun - Reviewer)

Contributors

Michael Bien <mbien42@gmail.com>

Download

$ git fetch https://git.openjdk.java.net/jdk pull/248/head:pull/248
$ git checkout pull/248

bridgekeeper · 2020-09-18T11:30:43Z

👋 Welcome back mbien! 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.

openjdk · 2020-09-18T11:32:03Z

@mbien The following label will be automatically applied to this pull request: hotspot-jfr.

When this pull request is ready to be reviewed, an RFR email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label (add|remove) "label" command.

mlbridge · 2020-09-25T12:53:36Z

Webrevs

02: Full - Incremental (9ee3291)
01: Full - Incremental (1cc205171ca7dfd83fc807ddc39006e1f2b934e2)
00: Full (67750e2b599b3f2a6b814d7bceada785ca4051f9)

mbien · 2020-10-02T14:25:40Z

Tiny PR looking for reviewers - now even with an issue assigned to it.

mgronlun · 2020-10-20T12:06:20Z

Hi Michael, I think you can go ahead and merge the blocks.

Thanks
Markus

mgronlun · 2020-10-20T12:11:11Z

You will also need to align the name of this PR with the synopsis of the JIRA issue, the bots complain about:

Integration blocker

⚠️ Title mismatch between PR and JBS for issue JDK-8253641

…der is null.

mbien · 2020-10-20T13:49:21Z

@mgronlun Thank you for the review, I made some changes:

changed the original fix so that the postFix is printed in every case after comparing it with similar methods (please review again)
added commit which merges the two consecutive ifs over same variable
changed PR title to issue title

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java

…ader.

mgronlun · 2020-10-20T14:28:22Z

/test tier1

openjdk · 2020-10-20T14:28:37Z

@mgronlun the test group tier-1 does not exist

mgronlun · 2020-10-20T17:20:33Z

/contributor mbien

openjdk · 2020-10-20T17:21:17Z

@mgronlun Only the author (@mbien) is allowed to issue the contributor command.

mgronlun

Thanks @mbien for fixing this.

mbien · 2020-10-20T17:22:21Z

@mgronlun no problem. Thank you for the review!

openjdk · 2020-10-20T17:22:30Z

@mbien 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:

8253641: Missing newline in the printout of certain JFR events

Co-authored-by: Michael Bien <mbien42@gmail.com>
Reviewed-by: mgronlun

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 447 new commits pushed to the master branch:

89e5444: 8254788: Dead code in the sun.java2d.xr.XRPMBlitLoops$XrSwToPMBlit
3267b09: 8254883: ZGC: Make the ZArrayIterator reusable for ZRelocationSetIterators
cb6167b: 8254557: Compiler crashes with java.lang.AssertionError: isSubtype UNKNOWN
40f847e: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
f167a71: 8254264: Remove redundant cross_modify_fence()
44f9271: 8254974: Fix stutter typo in TypeElement
76fdd7f: 8255038: Adjust adapter_code_size to account for -Xlog:methodhandles in debug builds
cdc8c40: 8254878: Move last piece of ZArray to GrowableArray
294e070: 8254998: C2: assert(!n->as_Loop()->is_transformed_long_loop()) failure with -XX:StressLongCountedLoop=1
3ee0380: 8234393: [macos] printing ignores printer tray
... and 437 more: https://git.openjdk.java.net/jdk/compare/86a16400bd94d41c7961bf6dcac539c6a796dad3...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@mgronlun) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mgronlun · 2020-10-20T17:29:46Z

You can issue a comment "/contributor add mbien" and then another comment "/integrate". I will then sponsor your change for integration. Sorry for the misspelling.

mbien · 2020-10-20T17:31:29Z

/contributor mbien

openjdk · 2020-10-20T17:31:36Z

@mbien Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

mbien · 2020-10-20T17:32:11Z

/contributor add mbien

openjdk · 2020-10-20T17:33:49Z

@mbien Could not parse mbien as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

mbien · 2020-10-20T17:35:49Z

/contributor add @mbien

openjdk · 2020-10-20T17:36:22Z

@mbien Could not parse @mbien as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

mbien · 2020-10-20T17:40:36Z

/contributor add Michael Bien mbien42@gmail.com

openjdk · 2020-10-20T17:41:25Z

@mbien
Contributor Michael Bien <mbien42@gmail.com> successfully added.

mbien · 2020-10-20T17:42:52Z

/integrate

mbien · 2020-10-20T17:43:30Z

(sorry for the spam, the bot was stubborn)

openjdk · 2020-10-20T17:44:27Z

@mbien
Your change (at version 9ee3291) is now ready to be sponsored by a Committer.

mgronlun · 2020-10-20T17:44:47Z

@mbien no problem, we are all new to the process here. Sorry for providing some vague instructions.

mgronlun · 2020-10-20T17:46:34Z

/sponsor

openjdk · 2020-10-20T17:47:56Z

@mgronlun @mbien Since your change was applied there have been 447 commits pushed to the master branch:

89e5444: 8254788: Dead code in the sun.java2d.xr.XRPMBlitLoops$XrSwToPMBlit
3267b09: 8254883: ZGC: Make the ZArrayIterator reusable for ZRelocationSetIterators
cb6167b: 8254557: Compiler crashes with java.lang.AssertionError: isSubtype UNKNOWN
40f847e: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
f167a71: 8254264: Remove redundant cross_modify_fence()
44f9271: 8254974: Fix stutter typo in TypeElement
76fdd7f: 8255038: Adjust adapter_code_size to account for -Xlog:methodhandles in debug builds
cdc8c40: 8254878: Move last piece of ZArray to GrowableArray
294e070: 8254998: C2: assert(!n->as_Loop()->is_transformed_long_loop()) failure with -XX:StressLongCountedLoop=1
3ee0380: 8234393: [macos] printing ignores printer tray
... and 437 more: https://git.openjdk.java.net/jdk/compare/86a16400bd94d41c7961bf6dcac539c6a796dad3...master

Your commit was automatically rebased without conflicts.

Pushed as commit e577c8c.

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

openjdk bot added the hotspot-jfr hotspot-jfr-dev@openjdk.org label Sep 18, 2020

mbien changed the title ~~[PATCH] bugfix: Missing newline in PrettyWriter::printClassLoader if classloader is null.~~ 8253641: Missing newline in PrettyWriter::printClassLoader if classloader is null. Sep 25, 2020

openjdk bot added the rfr Pull request is ready for review label Sep 25, 2020

bugfix: Missing newline in PrettyWriter::printClassLoader if classloa…

5cec42d

…der is null.

mbien force-pushed the jfr_print_fix branch from 67750e2 to 1cc2051 Compare October 20, 2020 13:37

mbien changed the title ~~8253641: Missing newline in PrettyWriter::printClassLoader if classloader is null.~~ JDK-8253641: Missing newline in PrettyWriter::printClassLoader if classloader is null. Oct 20, 2020

mbien changed the title ~~JDK-8253641: Missing newline in PrettyWriter::printClassLoader if classloader is null.~~ 8253641: Missing newline in the printout of certain JFR events Oct 20, 2020

mgronlun reviewed Oct 20, 2020

View reviewed changes

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java Outdated Show resolved Hide resolved

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java Show resolved Hide resolved

small refactoring: merged two if blocks in PrettyWriter::printClassLo…

9ee3291

…ader.

mbien force-pushed the jfr_print_fix branch from 1cc2051 to 9ee3291 Compare October 20, 2020 14:24

mgronlun approved these changes Oct 20, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 20, 2020

openjdk bot added the sponsor Pull request is ready to be sponsored label Oct 20, 2020

openjdk bot closed this Oct 20, 2020

openjdk bot added integrated Pull request has been integrated and removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 20, 2020

8253641: Missing newline in the printout of certain JFR events #248

8253641: Missing newline in the printout of certain JFR events #248

mbien commented Sep 18, 2020 •

edited by openjdk bot

bridgekeeper bot commented Sep 18, 2020

openjdk bot commented Sep 18, 2020 •

edited by bridgekeeper bot

mlbridge bot commented Sep 25, 2020 •

edited

mbien commented Oct 2, 2020

mgronlun commented Oct 20, 2020

mgronlun commented Oct 20, 2020

mbien commented Oct 20, 2020 •

edited

mgronlun commented Oct 20, 2020 •

edited

openjdk bot commented Oct 20, 2020

mgronlun commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mgronlun left a comment

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020 •

edited

mgronlun commented Oct 20, 2020 •

edited

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mgronlun commented Oct 20, 2020

mgronlun commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

8253641: Missing newline in the printout of certain JFR events #248

8253641: Missing newline in the printout of certain JFR events #248

Conversation

mbien commented Sep 18, 2020 • edited by openjdk bot

Progress

Issue

Reviewers

Contributors

Download

bridgekeeper bot commented Sep 18, 2020

openjdk bot commented Sep 18, 2020 • edited by bridgekeeper bot

mlbridge bot commented Sep 25, 2020 • edited

Webrevs

mbien commented Oct 2, 2020

mgronlun commented Oct 20, 2020

mgronlun commented Oct 20, 2020

mbien commented Oct 20, 2020 • edited

mgronlun commented Oct 20, 2020 • edited

openjdk bot commented Oct 20, 2020

mgronlun commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mgronlun left a comment

Choose a reason for hiding this comment

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020 • edited

mgronlun commented Oct 20, 2020 • edited

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Oct 20, 2020

mbien commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mgronlun commented Oct 20, 2020

mgronlun commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mbien commented Sep 18, 2020 •

edited by openjdk bot

openjdk bot commented Sep 18, 2020 •

edited by bridgekeeper bot

mlbridge bot commented Sep 25, 2020 •

edited

mbien commented Oct 20, 2020 •

edited

mgronlun commented Oct 20, 2020 •

edited

openjdk bot commented Oct 20, 2020 •

edited

mgronlun commented Oct 20, 2020 •

edited