8255047: Add HotSpot UseDebuggerErgo flags #763

stefank · 2020-10-20T10:10:12Z

Some debuggers don't work well with many threads, and/or incompletely restricts the number of used CPUs to one.

This flag is intended as a catch-all for HotSpot developers (not available in product builds) to allow us to more easily use those debuggers.

Currently, the proposal is to let the flag fix a few things:

Turn down the number of JVM threads
Turn off NUMA
Force processor_id() to return 0 instead of values above processor_count()

(1) is purely ergonomics: gdb, rr, valgrind is faster and seems to work much better with fewer threads. The values would still be overridable by devs.

(2) and (3) deals with the fact that some debuggers change the reported processor count, but don't change the processor ids returned by sched_getcpu. This causes problems for ZGC and NUMA, that both assumes that they can rely on os::processor_id() < os::processor_count().

The current proposed flag name is -XX:+LimitedCPUsDebugging. I'm not entirely happy with that name, but I been able to find a better name.

An alternative to having one flag, is to split this into two flags, and maybe that would solve the naming problem. However, the usability aspects will be worse.

If we can't find a suitable name, I rather introduce a flag called:
-XX:DebuggerWorkarounds or -XX:DebuggerWorkaround1

Any suggestions / opinions? I really do want to at least fix the (2, 3) problem, because I keep having to add this to every single branch I'm working on.

Progress

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

Testing

	Linux x32	Linux x64	Windows x64	macOS x64
Build	✔️ (1/1 passed)	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)		✔️ (9/9 passed)	❌ (1/9 failed)	✔️ (9/9 passed)

Failed test task

Windows x64 (hs/tier1 compiler)

Issue

JDK-8255047: Add HotSpot UseDebuggerErgo flags

Reviewers

Daniel D. Daugherty (@dcubed-ojdk - Reviewer)
David Holmes (@dholmes-ora - Reviewer)

Download

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

… count

stefank · 2020-10-20T10:10:20Z

/label add hotspot

bridgekeeper · 2020-10-20T10:14:39Z

👋 Welcome back stefank! 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-10-20T10:18:49Z

@stefank
The hotspot label was successfully added.

mlbridge · 2020-10-20T10:22:18Z

Webrevs

01: Full - Incremental (a7aa7b9)
00: Full (67e8775)

dcubed-ojdk · 2020-10-20T14:52:33Z

Perhaps: UseDebuggerErgo for the main option name with
UseDebuggerErgo1 ... UseDebuggerErgoN for the suboptions
where UseDebuggerErgo enables all the numbered
UseDebuggerErgo<X> options in one go.

Update: Yes, this is the way that I have always wished that
UseNewCode* worked.

stefank · 2020-10-21T07:24:52Z

@dcubed-ojdk Thanks for the suggestion. I like it.

… count

stefank · 2020-10-21T11:36:06Z

I've updated the patch with the suggestion from Dan. The flags now work as follows:
-XX:+UseDebuggerErgo turns on all ergonomics / workarounds
-XX:-/+UseDebuggerErgo1 fixes processor id vs processor count problems
-XX:-/+UseDebuggerErgo2 limits the number of JVM threads and turns off NUMA

dcubed-ojdk

Thumbs up.

dcubed-ojdk · 2020-10-21T13:56:37Z

src/hotspot/os/linux/os_linux.cpp

+  }
+#endif
+
+  assert(id >= 0 && id < _processor_count, "Invalid processor id [%d]", id);


Thanks for adding the bad id value to the assert() output.

dholmes-ora

LGTM!

Thanks,
David

stefank · 2020-10-22T06:35:03Z

Thanks @dcubed-ojdk and @dholmes-ora!

stefank · 2020-10-22T07:59:42Z

/integrate

openjdk · 2020-10-22T08:00:45Z

@stefank This PR has not yet been marked as ready for integration.

openjdk · 2020-10-22T08:06:11Z

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

8255047: Add HotSpot UseDebuggerErgo flags

Reviewed-by: dcubed, dholmes

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

211bb62: 8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner
299e115: 8198395: Test javax/swing/plaf/metal/MetalLookAndFeel/Test8039750.java fails in mach5
a5b7bc5: 7156347: javax/swing/JList/6462008/bug6462008.java fails
b25d894: 8252204: AArch64: Implement SHA3 accelerator/intrinsic
7d3d4da: 8240709: Enable javax/swing/UI/UnninstallUIMemoryLeaks/UnninstallUIMemoryLeaks.java on all L&F
5d26229: 8255174: Vector API unit tests for missed public api code coverage
b9186be: 6606767: resexhausted00[34] fail assert(!thread->owns_locks(), "must release all locks when leaving VM")
1191a63: 8199697: FIPS 186-4 RSA Key Generation
60d3fa2: 8255022: Documentation missing for Vector API zero methods
9ade94b: 8206311: Add docs-javase, docs-reference to CI build
... and 63 more: https://git.openjdk.java.net/jdk/compare/21e67e54604e60f57106be32d48ba75ebf8c1130...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.

stefank · 2020-10-22T08:23:43Z

/integrate

openjdk · 2020-10-22T08:28:36Z

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

211bb62: 8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner
299e115: 8198395: Test javax/swing/plaf/metal/MetalLookAndFeel/Test8039750.java fails in mach5
a5b7bc5: 7156347: javax/swing/JList/6462008/bug6462008.java fails
b25d894: 8252204: AArch64: Implement SHA3 accelerator/intrinsic
7d3d4da: 8240709: Enable javax/swing/UI/UnninstallUIMemoryLeaks/UnninstallUIMemoryLeaks.java on all L&F
5d26229: 8255174: Vector API unit tests for missed public api code coverage
b9186be: 6606767: resexhausted00[34] fail assert(!thread->owns_locks(), "must release all locks when leaving VM")
1191a63: 8199697: FIPS 186-4 RSA Key Generation
60d3fa2: 8255022: Documentation missing for Vector API zero methods
9ade94b: 8206311: Add docs-javase, docs-reference to CI build
... and 63 more: https://git.openjdk.java.net/jdk/compare/21e67e54604e60f57106be32d48ba75ebf8c1130...master

Your commit was automatically rebased without conflicts.

Pushed as commit ae72b52.

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

8255047: Add HotSpot flag to use with debuggers that restrict the CPU…

67e8775

… count

openjdk bot added rfr Pull request is ready for review hotspot hotspot-dev@openjdk.org labels Oct 20, 2020

8255047: Add HotSpot flag to use with debuggers that restrict the CPU…

a7aa7b9

… count

dcubed-ojdk approved these changes Oct 21, 2020

View reviewed changes

dholmes-ora approved these changes Oct 22, 2020

View reviewed changes

stefank changed the title ~~8255047: Add HotSpot flag to use with debuggers that restrict the CPU…~~ 8255047: Add HotSpot UseDebuggerErgo flags Oct 22, 2020

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

openjdk bot closed this Oct 22, 2020

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

stefank deleted the 8255047_debugger_workarounds branch October 22, 2020 13:09

8255047: Add HotSpot UseDebuggerErgo flags #763

8255047: Add HotSpot UseDebuggerErgo flags #763

stefank commented Oct 20, 2020 •

edited by openjdk bot

stefank commented Oct 20, 2020

bridgekeeper bot commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mlbridge bot commented Oct 20, 2020 •

edited

dcubed-ojdk commented Oct 20, 2020 •

edited

stefank commented Oct 21, 2020

stefank commented Oct 21, 2020 •

edited

dcubed-ojdk left a comment

dcubed-ojdk Oct 21, 2020

dholmes-ora left a comment

stefank commented Oct 22, 2020

stefank commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

stefank commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

8255047: Add HotSpot UseDebuggerErgo flags #763

8255047: Add HotSpot UseDebuggerErgo flags #763

Conversation

stefank commented Oct 20, 2020 • edited by openjdk bot

Progress

Testing

Issue

Reviewers

Download

stefank commented Oct 20, 2020

bridgekeeper bot commented Oct 20, 2020

openjdk bot commented Oct 20, 2020

mlbridge bot commented Oct 20, 2020 • edited

Webrevs

dcubed-ojdk commented Oct 20, 2020 • edited

stefank commented Oct 21, 2020

stefank commented Oct 21, 2020 • edited

dcubed-ojdk left a comment

Choose a reason for hiding this comment

dcubed-ojdk Oct 21, 2020

Choose a reason for hiding this comment

dholmes-ora left a comment

Choose a reason for hiding this comment

stefank commented Oct 22, 2020

stefank commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

stefank commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

stefank commented Oct 20, 2020 •

edited by openjdk bot

mlbridge bot commented Oct 20, 2020 •

edited

dcubed-ojdk commented Oct 20, 2020 •

edited

stefank commented Oct 21, 2020 •

edited