8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders #10832

coleenp · 2022-10-24T12:16:54Z

This change adds an option EnableWaitForParallelLoad to enable the legacy behavior where the VM will manage synchronization for multiple threads loading the same class using a non-parallel capable class loader that have released the class loader lock. The VM will break the class loader lock for parallel threads trying to load the class, and wait for the first thread that initiated loading the class to complete. The subsequent threads will use the result of the first thread, rather than get a LinkageError: duplicate class definition for loading the class without synchronization.
Releasing the class loader lock was a common workaround for class loaders that used a non-hierarchical delegation scheme to avoid deadlock, before parallel capable class loading was added.
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#registerAsParallelCapable()

Tested with tier1-6 and internal applications.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires a CSR request to be approved

Issues

JDK-8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders
JDK-8295848: Deprecate legacy parallel class loading workaround for non-parallel-capable class loaders (CSR)

Reviewers

Mandy Chung (@mlchung - Reviewer) ⚠️ Review applies to 3f5be0ac
David Holmes (@dholmes-ora - Reviewer) ⚠️ Review applies to 59c07651

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10832/head:pull/10832
$ git checkout pull/10832

Update a local copy of the PR:
$ git checkout pull/10832
$ git pull https://git.openjdk.org/jdk pull/10832/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10832

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10832.diff

bridgekeeper · 2022-10-24T12:18:12Z

👋 Welcome back coleenp! 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 · 2022-10-24T12:24:13Z

@coleenp The following label will be automatically applied to this pull request:

hotspot-runtime

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 pull request command.

src/hotspot/share/runtime/globals.hpp

…arallel-capable class loaders

mlbridge · 2022-11-03T12:50:41Z

Webrevs

mlchung · 2022-11-03T16:39:33Z

/label add core-libs

openjdk · 2022-11-03T16:40:47Z

@mlchung
The core-libs label was successfully added.

mlchung · 2022-11-03T16:48:36Z

Adding core-libs-dev so that more developers know this behavioral change.

Background:

Prior to JDK 7, custom class loaders using non-hierarchical class delegation model were prone to deadlock (see [1] for details). This workaround was added in the VM in JDK 6 to allow parallel class loading for non-parallel-capable class loaders to avoid deadlocks. The VM also supports a common workaround at the time that some customers explicitly issue a wait() on the class loader lock.

Such workaround was kept to allow those customers to migrate to using multi-thread-safe parallel-capable class loaders. It has been 11 years since Java 7 and it's expected that those deadlock-prone custom class loaders should have migrated to the parallel-capable class loaders.

If customers have class loaders depending on this legacy behavior, they can set -XX:+EnableWaitForParallelLoad flag to enable the legacy behavior and migrate to parallel-capable class loaders as soon as possible. The EnableWaitForParallelLoad VM flag will be removed in a future release.

Refers to CSR JDK-8295848 for details.

openjdk · 2022-11-03T16:52:28Z

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

8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders

Reviewed-by: mchung, 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 41 new commits pushed to the master branch:

8836b92: 8296226: Add constructors (String,Throwable) and (Throwable) to InvalidParameterException
51f8e9b: 8296443: NMT: Remove cmdline_tracking_level handling code
087cedc: 8295261: RISC-V: Support ReductionV instructions for Vector API
556377a: 8296270: Memory leak in ClassLoader::setup_bootstrap_search_path_impl
c2f7638: 8296335: Fix accessibility manual test instruction
91292d5: 8286301: Port JEP 425 to RISC-V
581133a: 8294696: BufferedInputStream.transferTo should drain buffer when mark set
d8573b2: 8294488: Delete KCMS transforms wrappers
f857f79: 8015739: Background of JInternalFrame is located out of JInternalFrame
b847fb6: 8296414: [BACKOUT] JDK-8295319: pending_cards_at_gc_start doesn't include cards in thread buffers
... and 31 more: https://git.openjdk.org/jdk/compare/d771abb2fbc72e02faf02f0724aea301953ac5e8...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.

dholmes-ora

It would be a little clearer if the bug synopsis was "Deprecate and disable legacy ...".

A couple of queries below, but otherwise this looks good - and it will be good to see this old workaround disappear.

Thanks.

src/hotspot/share/classfile/placeholders.cpp

src/hotspot/share/classfile/systemDictionary.cpp

openjdk-notifier · 2022-11-06T23:05:58Z

@coleenp Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See OpenJDK Developers’ Guide for more information.

dholmes-ora

Looks good. Thanks.

coleenp · 2022-11-07T23:29:22Z

Thank you for the discussions, CSR, help and code reviews, @mlchung and @dholmes-ora .
/integrate

openjdk · 2022-11-07T23:31:33Z

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

b6738c1: 8295663: Rephrase introduction to testing.md
7e85b41: 8296154: [macos] Change "/Applications" to "Applications" in DMG image
60db5f2: 8294020: improve errors for record declarations
520db1e: 8296485: BuildEEBasicConstraints.java test fails with SunCertPathBuilderException
b29817a: 8296479: Remove stray comment about POST_STRIP_CMD
b6ea69c: 8296480: java/security/cert/pkix/policyChanges/TestPolicy.java is failing
3baad06: 8295953: Use enhanced-for cycle instead of Enumeration in sun.security
ba303c0: 8295893: Improve printing of Constant Pool Cache Entries
f8b2574: 8296137: diags-examples.xml is broken
00d22f6: 8279164: Disable TLS_ECDH_* cipher suites
... and 42 more: https://git.openjdk.org/jdk/compare/d771abb2fbc72e02faf02f0724aea301953ac5e8...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-11-07T23:31:49Z

@coleenp Pushed as commit 76790ad.

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

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Oct 24, 2022

dholmes-ora reviewed Oct 24, 2022

View reviewed changes

src/hotspot/share/runtime/globals.hpp Outdated Show resolved Hide resolved

openjdk bot added the csr Pull request needs approved CSR before integration label Oct 24, 2022

coleenp changed the title ~~8295673: Deprecate JVM class loading double_lock_wait handling~~ 8295673: Deprecate legacy parallel class loading workaround for non-parallel-capable class loaders Oct 26, 2022

openjdk bot removed the csr Pull request needs approved CSR before integration label Nov 3, 2022

8295673: Deprecate legacy parallel class loading workaround for non-p…

3f5be0a

…arallel-capable class loaders

coleenp force-pushed the remove-double-lock branch from 731d87e to 3f5be0a Compare November 3, 2022 12:44

coleenp marked this pull request as ready for review November 3, 2022 12:45

openjdk bot added the rfr Pull request is ready for review label Nov 3, 2022

openjdk bot added the core-libs core-libs-dev@openjdk.org label Nov 3, 2022

mlchung approved these changes Nov 3, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 3, 2022

dholmes-ora reviewed Nov 4, 2022

View reviewed changes

src/hotspot/share/classfile/placeholders.cpp Outdated Show resolved Hide resolved

src/hotspot/share/classfile/systemDictionary.cpp Show resolved Hide resolved

coleenp changed the title ~~8295673: Deprecate legacy parallel class loading workaround for non-parallel-capable class loaders~~ 8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders Nov 6, 2022

Remove assert no longer valid.

59c0765

dholmes-ora approved these changes Nov 7, 2022

View reviewed changes

Fix logic in assert

2d70c34

openjdk bot added the integrated Pull request has been integrated label Nov 7, 2022

openjdk bot closed this Nov 7, 2022

openjdk bot removed the ready Pull request is ready to be integrated label Nov 7, 2022

openjdk bot removed the rfr Pull request is ready for review label Nov 7, 2022

coleenp deleted the remove-double-lock branch November 7, 2022 23:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders #10832

8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders #10832

coleenp commented Oct 24, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 24, 2022

openjdk bot commented Oct 24, 2022

mlbridge bot commented Nov 3, 2022 •

edited

Loading

mlchung commented Nov 3, 2022

openjdk bot commented Nov 3, 2022

mlchung commented Nov 3, 2022 •

edited

Loading

openjdk bot commented Nov 3, 2022 •

edited

Loading

dholmes-ora left a comment

openjdk-notifier bot commented Nov 6, 2022

dholmes-ora left a comment

coleenp commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders #10832

8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders #10832

Conversation

coleenp commented Oct 24, 2022 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Oct 24, 2022

openjdk bot commented Oct 24, 2022

mlbridge bot commented Nov 3, 2022 • edited Loading

Webrevs

mlchung commented Nov 3, 2022

openjdk bot commented Nov 3, 2022

mlchung commented Nov 3, 2022 • edited Loading

openjdk bot commented Nov 3, 2022 • edited Loading

dholmes-ora left a comment

Choose a reason for hiding this comment

openjdk-notifier bot commented Nov 6, 2022

dholmes-ora left a comment

Choose a reason for hiding this comment

coleenp commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

openjdk bot commented Nov 7, 2022

coleenp commented Oct 24, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 3, 2022 •

edited

Loading

mlchung commented Nov 3, 2022 •

edited

Loading

openjdk bot commented Nov 3, 2022 •

edited

Loading