8366584: Add an InstanceKlass::super() method that returns InstanceKlass* #27037

iklam · 2025-09-02T01:31:59Z

By adding an InstanceKlass* InstanceKlass::super() method to shadow Klass* Klass:super(), this PR makes it possible to simplify the following code

InstanceKlass* ik;
InstanceKlass* s;
s = InstanceKlass::cast(ik->super()); // before JDK-8366024 
s = ik->java_super(); // after JDK-8366024

to

s = k->super();

So you no longer need to do casting or need to understand what java_super() is.

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-8366584: Add an InstanceKlass::super() method that returns InstanceKlass* (Enhancement - P4)

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Coleen Phillimore (@coleenp - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27037

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…ass*

bridgekeeper · 2025-09-02T01:33:05Z

👋 Welcome back iklam! 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-09-02T01:33:48Z

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

8366584: Add an InstanceKlass::super() method that returns InstanceKlass*

Reviewed-by: dholmes, coleenp

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

becc35f: 8366400: JCK test api/java_text/DecimalFormat/Parse.html fails after JDK-8363972
431f467: 8361635: Missing List length validation in the Class-File API
8d23661: 8366155: Serial: Obsolete PretenureSizeThreshold
... and 55 more: https://git.openjdk.org/jdk/compare/2427c901b31dbdccc6f8f39404875a0140460479...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-09-02T01:34:21Z

@iklam The following labels will be automatically applied to this pull request:

hotspot
serviceability

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

mlbridge · 2025-09-02T01:38:16Z

Webrevs

dholmes-ora

Looks good! Great to see all those java_super() calls disappear. A couple of nits/comments.

Thanks

dholmes-ora · 2025-09-02T04:56:50Z

src/hotspot/share/memory/heapInspection.cpp

-  Klass* super = ((InstanceKlass*)cie->klass())->java_super();
+  InstanceKlass* super = InstanceKlass::cast(cie->klass())->super();


Pre-existing, but if this cast is safe then KlassInfoEntry::_klass should be declared InstanceKlass. Otherwise this cast is not safe! Separate RFE for that.

I looked at the KlassHierarchy code and it looks at only instance klasses.

jdk/src/hotspot/share/memory/heapInspection.cpp

Lines 318 to 319 in 80fb708

if (cie->klass()->is_instance_klass()) {

_elements->append(cie);

I added an assert to make this more apparent.

The KlassInfoEntry is used by other code that could sometimes store a non-instance klass. I'll leave it as is for now.

dholmes-ora · 2025-09-02T04:59:09Z

src/hotspot/share/oops/instanceKlass.hpp

+  // This shadows Klass::super(). The _super of an InstanceKlass is
+  // always an InstanceKlass (or nullptr)
+  InstanceKlass* super() const {
+    return (Klass::super() == nullptr) ? nullptr : InstanceKlass::cast(Klass::super());


Is it better/simpler/cleaner to just do:

return static_cast<InstanceKlass*>(Klass::super());

?

I think the term is "hides" not "shadows". InstanceKlass::cast() is better - one additional check that super is always another InstanceKlass.

Do we still need java_super ?

There are a few places that use it. Maybe we should look at these again in the future.

compiler dependencies: ./share/code/dependencies.cpp: for (InstanceKlass* super = k->java_super(); super != nullptr; super = super->java_super()) { ./share/code/dependencies.cpp: _klass = _klass->java_super(); subklass and sibling: ./share/oops/instanceKlass.cpp: _current = _current->java_super(); // backtrack; no more sibling subclasses left ./share/oops/klass.cpp: InstanceKlass* super = java_super(); ./share/oops/klass.cpp: && (super->java_super() == nullptr || !is_interface())), Some general cases where you have a Klass but wants same super as in java.lang.Class::getSuperClass ./share/oops/klassVtable.cpp: InstanceKlass* super = _klass->java_super(); ./share/prims/jni.cpp: // Rules of Class.getSuperClass as implemented by KLass::java_super: ./share/prims/jni.cpp: Klass* super = k->java_super(); ./share/prims/jni.cpp: "java_super computation depends on interface, array, other super"); ./share/services/heapDumper.cpp: InstanceKlass* java_super = k->java_super(); ./share/services/heapDumper.cpp: assert(java_super != nullptr, "checking"); ./share/services/heapDumper.cpp: writer->write_classID(java_super);

I changed to hides. There are mentions of both shadows and hides in our comments.

src/hotspot/share/oops/instanceKlass.hpp

dholmes-ora · 2025-09-02T05:31:30Z

src/hotspot/share/oops/klassVtable.cpp

@@ -1571,7 +1571,7 @@ void klassVtable::verify(outputStream* st, bool forced) {
  // verify consistency with superKlass vtable
  Klass* super = _klass->super();
  if (super != nullptr) {
-    InstanceKlass* sk = InstanceKlass::cast(super);
+    InstanceKlass* sk = InstanceKlass::cast(super); // why are we sure this is InstanceKlass??


I don't think it is guaranteed to be an IK. But AFAICS we don't actually need an IK here anyway, we should be able to use super directly.

coleenp

Looks good. This will help with InstanceKlass casting.

dholmes-ora

Updates look good. Thanks

iklam · 2025-09-04T02:30:22Z

Thanks @coleenp @dholmes-ora for the review
/integrate

openjdk · 2025-09-04T02:31:15Z

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

11743b1: 8366695: Test sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java timed out
ed62bda: 8366694: Test JdbStopInNotificationThreadTest.java timed out after 60 second
02dd211: 8366692: Several gc/shenandoah tests timed out
... and 58 more: https://git.openjdk.org/jdk/compare/2427c901b31dbdccc6f8f39404875a0140460479...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-09-04T02:31:23Z

@iklam Pushed as commit f4d73d2.

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

8366584: Add an InstanceKlass::super() method that returns InstanceKl…

5423667

…ass*

openjdk bot added serviceability [email protected] hotspot [email protected] rfr Pull request is ready for review labels Sep 2, 2025

dholmes-ora reviewed Sep 2, 2025

View reviewed changes

iklam added 2 commits September 2, 2025 13:58

Added assert in heapInspection.cpp

e1bac27

@dholmes-ora and @coleep comments

dd0a3c5

coleenp approved these changes Sep 3, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 3, 2025

dholmes-ora approved these changes Sep 4, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Sep 4, 2025

openjdk bot closed this Sep 4, 2025

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

		Klass* super = ((InstanceKlass*)cie->klass())->java_super();
		InstanceKlass* super = InstanceKlass::cast(cie->klass())->super();

	if (cie->klass()->is_instance_klass()) {
	_elements->append(cie);

8366584: Add an InstanceKlass::super() method that returns InstanceKlass* #27037

8366584: Add an InstanceKlass::super() method that returns InstanceKlass* #27037

Conversation

iklam commented Sep 2, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Sep 2, 2025

Uh oh!

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

Uh oh!

Uh oh!

openjdk bot commented Sep 2, 2025

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

dholmes-ora left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coleenp left a comment

Choose a reason for hiding this comment

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

iklam commented Sep 4, 2025

Uh oh!

openjdk bot commented Sep 4, 2025

Uh oh!

openjdk bot commented Sep 4, 2025

Uh oh!

Uh oh!

iklam commented Sep 2, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 2, 2025 •

edited

Loading

mlbridge bot commented Sep 2, 2025 •

edited

Loading

dholmes-ora left a comment •

edited

Loading