8260878: com/sun/jdi/JdbOptions.java fails without jfr

[DamonFool]

Hi all,

com/sun/jdi/JdbOptions.java fails in our ci/cd when jfr is disabled.
It would be better to fix it.

Thanks.
Best regards,
Jie

---

Progress

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

Issue

• JDK-8260878: com/sun/jdi/JdbOptions.java fails without jfr

Reviewers

• Alex Menkov (@alexmenkov - Reviewer)
• Chris Plummer (@plummercj - Reviewer)

Download

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

[DamonFool]

/issue add JDK-8260878
/test
/label add serviceability
/cc serviceability

[bridgekeeper]

👋 Welcome back jiefu! 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]

@DamonFool This issue is referenced in the PR title - it will now be updated.

[openjdk]

@DamonFool
The serviceability label was successfully added.

[openjdk]

@DamonFool The serviceability label was already applied.

[mlbridge]

Webrevs

• 00: Full (b5a15fd)

[plummercj]

You need to be careful when using -xx:+IgnoreUnrecognizedVMOptions, because it can result in not detecting typos in the option names, which could happen with this test in the future if any other mods are made. Other options here are to skip this part of the test if JFR is not present, or better yet don't make the test rely on JFR. I don't think there is a reason that it needs to. It seems other options could have been used for the testing. Perhaps @alexmenkov can comment on why it was done this way.

[DamonFool]

Thanks @plummercj for your review.

Let's wait for @alexmenkov 's comments.
Thanks.

[alexmenkov]

Special handling for options with commas, quotes and single quotes was added many years ago for JFR (as it uses commas inside). The logic there is a bit tricky and I didn't want to brake existing functionality when I made some changes there, so I used "live" examples of JRF options in the test.
I don't know other VM options which use commas inside.

[alexmenkov]

As the test verifies correct parsing of "options", I think it's ok to use IgnoreUnrecognizedVMOptions here

[plummercj]

ok

[openjdk]

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

8260878: com/sun/jdi/JdbOptions.java fails without jfr

Reviewed-by: amenkov, cjplummer

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

• 69189f8: 8256421: Add 2 HARICA roots to cacerts truststore
• f546fd0: 8260902: CDS mapping errors should not lead to unconditional output
• d7b1fc5: 8260707: java/lang/instrument/PremainClass/InheritAgent0100.java times out
• 0093183: 8260368: [PPC64] GC interface needs enhancement to support GCs with load barriers
• defcb04: 8260867: ProblemList java/awt/FullScreen/TranslucentWindow/TranslucentWindow.java on linux
• a421bfa: 8259839: SystemDictionary exports too much implementation
• 189b65b: 8260264: Move common os_ inline methods to a common posix source file
• 288a4fe: 8260643: Remove parallel version handling in CardTableRS::younger_refs_in_space_iterate()
• ddd2951: 8260571: Add PrintMetaspaceStatistics to print metaspace statistics upon VM exit
• fe407cf: 8260420: C2 compilation fails with assert(found_sfpt) failed: no node in loop that's not input to safepoint

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[DamonFool]

Thanks @alexmenkov and @plummercj .
/integrate

[openjdk]

@DamonFool Since your change was applied there have been 16 commits pushed to the master branch:

• d423d36: 8258508: Merge G1RedirtyCardsQueue into qset
• bec6043: 8259570: (macos) tools/jpackage tests fails with 'hdiutil: couldn't eject "disk2" - Resource busy'
• ffbcf1b: 8260471: Change SystemDictionary::X_klass calls to vmClasses::X_klass
• 9af3339: 8261003: Bad Copyright header format after JDK-8183372
• 6dc3c6d: 8183372: Refactor java/lang/Class shell tests to java
• 105d3e8: 8260861: TrustStoreDescriptor log the same value
• 69189f8: 8256421: Add 2 HARICA roots to cacerts truststore
• f546fd0: 8260902: CDS mapping errors should not lead to unconditional output
• d7b1fc5: 8260707: java/lang/instrument/PremainClass/InheritAgent0100.java times out
• 0093183: 8260368: [PPC64] GC interface needs enhancement to support GCs with load barriers
• ... and 6 more: https://git.openjdk.java.net/jdk/compare/474dba2d8b93b5969a11635820b07c0ceff1836a...master

Your commit was automatically rebased without conflicts.

Pushed as commit a47befc.

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