8369296: Add fast class init checks in interpreter for resolving ConstantPool entries for static field #27676

ashu-mehra · 2025-10-07T14:25:47Z

This patch adds fast clinit barrier in the interpreter when resolving cp entry for a static field.

Testing: tested x86-64 by running hotspot_runtime group
Specifically, runtime/clinit/ClassInitBarrier.java fails if the block for adding clinit_barrier is commented out in TemplateTable::resolve_cache_and_index_for_field

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

Issue

JDK-8369296: Add fast class init checks in interpreter for resolving ConstantPool entries for static field (Enhancement - P4)

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer) Review applies to 5ddd660d
Amit Kumar (@offamitkumar - Committer) Review applies to 4eb89b8b
Martin Doerr (@TheRealMDoerr - Reviewer)
Fei Yang (@RealFYang - Reviewer) Review applies to 5ddd660d
Hamlin Li (@Hamlin-Li - Reviewer) Review applies to 5ddd660d
Vladimir Ivanov (@iwanowww - Reviewer)

Contributors

Vladimir Ivanov <vlivanov@openjdk.org>
Amit Kumar <amitkumar@openjdk.org>
Fei Yang <fyang@openjdk.org>
Martin Doerr <mdoerr@openjdk.org>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27676

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…tantPool entries for static field Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2025-10-07T14:26:40Z

/contributor add @iwanowww

bridgekeeper · 2025-10-07T14:27:37Z

👋 Welcome back asmehra! 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 · 2025-10-07T14:29:05Z

@ashu-mehra 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:

8369296: Add fast class init checks in interpreter for resolving ConstantPool entries for static field

Co-authored-by: Vladimir Ivanov <vlivanov@openjdk.org>
Co-authored-by: Amit Kumar <amitkumar@openjdk.org>
Co-authored-by: Fei Yang <fyang@openjdk.org>
Co-authored-by: Martin Doerr <mdoerr@openjdk.org>
Reviewed-by: mdoerr, vlivanov, kvn, amitkumar, fyang, mli

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

64ff706: 8369806: Remove nsk/jvmti/AttachOnDemand/attach020 from problemlist
4b85ba9: 8369657: [AIX] TOC overflow in static-launcher build when building slowdebug after JDK-8352064
7266369: 8369616: JavaFrameAnchor on RISC-V has unnecessary barriers and wrong store order in MacroAssembler
... and 141 more: https://git.openjdk.org/jdk/compare/72319167543a28295276f11178c17bef6680c32f...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.

openjdk · 2025-10-07T14:29:54Z

@ashu-mehra
Contributor Vladimir Ivanov <vlivanov@openjdk.org> successfully added.

openjdk · 2025-10-07T14:30:37Z

@ashu-mehra The following label will be automatically applied to this pull request:

hotspot

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.

mlbridge · 2025-10-07T14:34:25Z

Webrevs

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2025-10-07T16:05:18Z

@offamitkumar @Hamlin-Li @RealFYang @TheRealMDoerr
I need help in adding arch specific code for s390, risc-v and ppc for this PR. Would you be able to contribute the code for these platforms? Specifically, we need to add a call to clinit_barrier in TemplateTable::resolve_cache_and_index_for_field.

@vnkozlov @adinn @iwanowww can you please review this patch.

offamitkumar · 2025-10-07T16:37:24Z

Hi @ashu-mehra,

This is fixing the test failure on s390:

diff --git a/src/hotspot/cpu/s390/templateTable_s390.cpp b/src/hotspot/cpu/s390/templateTable_s390.cpp
index 2b39cc8318c..cf34bff7746 100644
--- a/src/hotspot/cpu/s390/templateTable_s390.cpp
+++ b/src/hotspot/cpu/s390/templateTable_s390.cpp
@@ -2409,6 +2409,7 @@ void TemplateTable::resolve_cache_and_index_for_field(int byte_no,
   assert(byte_no == f1_byte || byte_no == f2_byte, "byte_no out of range");
 
   NearLabel resolved;
+  Label L_clinit_barrier_slow;
 
   Bytecodes::Code code = bytecode();
   switch (code) {
@@ -2425,6 +2426,8 @@ void TemplateTable::resolve_cache_and_index_for_field(int byte_no,
   __ z_bre(resolved);
 
   // resolve first time through
+  // Class initialization barrier slow path lands here as well.
+  __ bind(L_clinit_barrier_slow);
   address entry = CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_from_cache);
   __ load_const_optimized(Z_ARG2, (int)code);
   __ call_VM(noreg, entry, Z_ARG2);
@@ -2434,6 +2437,15 @@ void TemplateTable::resolve_cache_and_index_for_field(int byte_no,
 
   __ bind(resolved);
 
+  // Class initialization barrier for static fields
+  if (VM_Version::supports_fast_class_init_checks() &&
+      (bytecode() == Bytecodes::_getstatic || bytecode() == Bytecodes::_putstatic)) {
+    const Register field_holder = index;
+
+    __ load_sized_value(field_holder, Address(cache, ResolvedFieldEntry::field_holder_offset()), sizeof(void*), false);
+    __ clinit_barrier(field_holder, Z_thread, nullptr /*L_fast_path*/, &L_clinit_barrier_slow);
+  }
+
   BLOCK_COMMENT("} resolve_cache_and_index_for_field");
 }

iwanowww · 2025-10-07T17:02:02Z

x86- and aarch64-specific changes look good.

There are 3 other platforms (ppc, s390, and riscv) which report VM_Version::supports_fast_class_init_checks() == true. All of them require similar changes in platform-specific code.

vnkozlov

Can we loop to L_clinit_barrier_slow several times until class is initialized?

iwanowww · 2025-10-07T17:46:24Z

Can we loop to L_clinit_barrier_slow several times until class is initialized?

No, clinit_barrier either "blocks" (not yet initialized or being initialized, but not in init thread) or returns right away (fully initialized or being initialized and in init thread). No need for repeated attempts.

vnkozlov · 2025-10-07T18:07:09Z

Can we loop to L_clinit_barrier_slow several times until class is initialized?

No, clinit_barrier either "blocks" (not yet initialized or being initialized, but not in init thread) or returns right away (fully initialized or being initialized and in init thread). No need for repeated attempts.

First, what do you mean "blocks"? It wait something with lock?

Second, my question was that slow path in clinit_barrier will jump to L_clinit_barrier_slow which is before clinit_barrier there are no branch there so after call_VM we again will call clinit_barrier? what I am missing?

ashu-mehra · 2025-10-07T18:22:21Z

First, what do you mean "blocks"? It wait something with lock?

Second, my question was that slow path in clinit_barrier will jump to L_clinit_barrier_slow which is before clinit_barrier there are no branch there so after call_VM we again will call clinit_barrier? what I am missing?

clinit_barrier itself doesn't block. Its a quick check for class fully initialized or the current thread is the init thread. If not, then it jumps to the slow path. In this case slow path is the call to InterpreterRuntime::resolve_from_cache which may initialize the klass or get blocked if the klass is being initialized by another thread.
After returning it again executes clinit_barrier. But this time the class should have initialized, so it just falls through.

Also, I think clinit_barrier is not a very accurate name because it doesn't cause initialization of the klass. I think fast_clinit_check better conveys what it is doing, but that renaming can be done later if required.

vnkozlov · 2025-10-07T18:29:21Z

Thank you @ashu-mehra for explanation. So we can call clinit_barrier second time but not more. Good.

fast_clinit_check better conveys what it is doing,

Agree.

vnkozlov · 2025-10-07T18:40:53Z

I submitted our testing.

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2025-10-07T18:56:15Z

@offamitkumar thanks for the patch. I have added it.

ashu-mehra · 2025-10-07T18:56:36Z

/contributor add @offamitkumar

openjdk · 2025-10-07T18:57:11Z

@ashu-mehra
Contributor Amit Kumar <amitkumar@openjdk.org> successfully added.

iwanowww · 2025-10-07T19:02:13Z

After returning it again executes clinit_barrier. But this time the class should have initialized, so it just falls through.

There are no guarantees that the class is initialized. But the only case when it doesn't hold is when the class is being initialized and current thread is initializing one. So, it is guaranteed that slow path is taken at most once.

I think fast_clinit_check better conveys what it is doing

clinit_barrier implements a fast path check, so clinit_barrier_fast_path maybe a more accurate variant.

ashu-mehra · 2025-10-07T19:12:34Z

clinit_barrier implements a fast path check, so clinit_barrier_fast_path maybe a more accurate variant.

yes, clinit_barrier_fast_path is better.

RealFYang · 2025-10-08T03:27:14Z

@ashu-mehra
Hi, Thanks for the ping! And here is the riscv-specific changes:
27676-riscv.diff.txt
hotspot:tier1 and hotspot_runtime passed with fastdebug build on linux-riscv64.

adinn · 2025-10-08T13:12:27Z

x86- and aarch64-specific changes look fine to me.

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2025-10-08T13:38:52Z

@RealFYang thanks for the patch. I added it.

ashu-mehra · 2025-10-08T13:39:06Z

/contributor add @RealFYang

TheRealMDoerr

Unfortunately, this is no longer correct for PPC64 because the isync instruction is missing on some paths, now. While this looks good for other platforms, can you revert only PPC64 to the previous version, please?

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2025-10-10T14:35:25Z

Unfortunately, this is no longer correct for PPC64 because the isync instruction is missing on some paths, now. While this looks good for other platforms, can you revert only PPC64 to the previous version, please?

@TheRealMDoerr thanks for catching the bug. The resolved non-static case is missing the isync. Reverted the ppc changes as suggested.

TheRealMDoerr

Thank you for improving it for all platforms! I think it is better readably, now. Only a minor nit. We'll retest it over the weekend.

TheRealMDoerr · 2025-10-10T16:10:06Z

src/hotspot/cpu/ppc/templateTable_ppc_64.cpp

    const Register klass  = Rscratch;

+
+


Can you remove these extra empty lines, please?

vnkozlov

aarch64 and x64 changes looks good to me.
I need to run new testing.

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

vnkozlov

My v08 testing passed.

RealFYang

Update to RISC-V part looks fine. And hotspot_runtime still test good with fastdebug build on linux-riscv64.

TheRealMDoerr · 2025-10-13T08:36:13Z

Test results look good on all SAP supported platforms (including linux and AIX on PPC64).

Hamlin-Li

Riscv part looks good, Thanks!

iwanowww · 2025-10-13T21:33:56Z

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp

  assert(byte_no == f1_byte || byte_no == f2_byte, "byte_no out of range");

-  Label resolved, clinit_barrier_slow;
+  Label Lclinit_barrier_slow, Ldone;


Minor comment: why don't you use L_name instead? Lname is a new idiom for naming labels on most of the platforms. (The convention is either L_name or simply name almost everywhere.)

Sorry, I guess that was my mistake. Changing it would be fine with me. Not a big deal, though.

I will update the label names to use the convention L_name.

iwanowww · 2025-10-13T21:36:19Z

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp

-  __ br(Assembler::EQ, resolved);
+
+  // Class initialization barrier for static methods
+  if (VM_Version::supports_fast_class_init_checks() && bytecode() == Bytecodes::_invokestatic) {


FTR VM_Version::supports_fast_class_init_checks() is redundant in platform-specific code.

The method is intended to be used in shared code to support dispatching between 2 execution modes, but for each platform is it known well ahead whether fast class init checks are supported or not.

(It does make sense to keep it as an assert through.)

True, but I also like the current implementation from a readability perspective.

Let's keep it for now. I will remove VM_Version::supports_fast_class_init_checks() from platform-specific code in a follow-up PR.

Sounds good, thanks!

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

ashu-mehra · 2025-10-14T14:18:15Z

@iwanowww I addressed your suggestion to rename the labels.

vnkozlov · 2025-10-14T16:02:23Z

No code change after v08 (only labels were renamed and empty lines removed) - no need to re-test.

ashu-mehra · 2025-10-14T18:14:02Z

okay, lets get this in. Thanks everyone for the reviews and suggestions.

ashu-mehra · 2025-10-14T18:14:09Z

/integrate

openjdk · 2025-10-14T18:15:07Z

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

09e8797: 8368633: (fs) Path.toRealPath(NOFOLLOW_LINKS) very slow on macOS
65b8fe6: 8368907: (fs) Windows Error code 1314 not translated to AccessDeniedException (win)
d6537c6: 8344918: Unused private variables in SwingUtilities.java
... and 146 more: https://git.openjdk.org/jdk/compare/72319167543a28295276f11178c17bef6680c32f...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-10-14T18:15:21Z

@ashu-mehra Pushed as commit 622a611.

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

8369296: Add fast class init checks in interpreter for resolving Cons…

667db8a

…tantPool entries for static field Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

openjdk bot added the hotspot hotspot-dev@openjdk.org label Oct 7, 2025

openjdk bot added the rfr Pull request is ready for review label Oct 7, 2025

Fix compile failure

995d4bb

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

vnkozlov reviewed Oct 7, 2025

View reviewed changes

Add support for s390

0ef8bdd

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

Add support for riscv

73bc815

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

TheRealMDoerr suggested changes Oct 10, 2025

View reviewed changes

Revert ppc64 changes

c81f380

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

TheRealMDoerr reviewed Oct 10, 2025

View reviewed changes

vnkozlov reviewed Oct 10, 2025

View reviewed changes

Remove blank lines

5ddd660

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

TheRealMDoerr approved these changes Oct 10, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 10, 2025

vnkozlov approved these changes Oct 12, 2025

View reviewed changes

RealFYang approved these changes Oct 13, 2025

View reviewed changes

Hamlin-Li approved these changes Oct 13, 2025

View reviewed changes

iwanowww reviewed Oct 13, 2025

View reviewed changes

Rename labels

ef89c4d

Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>

openjdk bot removed the ready Pull request is ready to be integrated label Oct 14, 2025

TheRealMDoerr approved these changes Oct 14, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 14, 2025

iwanowww approved these changes Oct 14, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Oct 14, 2025

openjdk bot closed this Oct 14, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 14, 2025

TheRealMDoerr mentioned this pull request Oct 14, 2025

8369238: Allow virtual thread preemption on some common class initialization paths #27802

Open

3 tasks

ashu-mehra deleted the fast-clinit-barrier-static-fields branch October 14, 2025 20:23

8369296: Add fast class init checks in interpreter for resolving ConstantPool entries for static field #27676

8369296: Add fast class init checks in interpreter for resolving ConstantPool entries for static field #27676

Uh oh!

Conversation

ashu-mehra commented Oct 7, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Uh oh!

ashu-mehra commented Oct 7, 2025

Uh oh!

bridgekeeper bot commented Oct 7, 2025

Uh oh!

openjdk bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Oct 7, 2025

Uh oh!

openjdk bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

ashu-mehra commented Oct 7, 2025

Uh oh!

offamitkumar commented Oct 7, 2025

Uh oh!

iwanowww commented Oct 7, 2025

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

iwanowww commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vnkozlov commented Oct 7, 2025

Uh oh!

ashu-mehra commented Oct 7, 2025

Uh oh!

vnkozlov commented Oct 7, 2025

Uh oh!

vnkozlov commented Oct 7, 2025

Uh oh!

ashu-mehra commented Oct 7, 2025

Uh oh!

ashu-mehra commented Oct 7, 2025

Uh oh!

openjdk bot commented Oct 7, 2025

Uh oh!

iwanowww commented Oct 7, 2025

Uh oh!

ashu-mehra commented Oct 7, 2025

Uh oh!

RealFYang commented Oct 8, 2025

Uh oh!

adinn commented Oct 8, 2025

Uh oh!

ashu-mehra commented Oct 8, 2025

Uh oh!

ashu-mehra commented Oct 8, 2025

Uh oh!

TheRealMDoerr left a comment

Choose a reason for hiding this comment

Uh oh!

ashu-mehra commented Oct 10, 2025

Uh oh!

TheRealMDoerr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

ashu-mehra commented Oct 7, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 7, 2025 •

edited

Loading

openjdk bot commented Oct 7, 2025 •

edited

Loading

mlbridge bot commented Oct 7, 2025 •

edited

Loading

iwanowww commented Oct 7, 2025 •

edited

Loading