From acbe84c5e8a1209ab6f46480fb6f686303c84a61 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 25 Feb 2025 13:57:10 +0100 Subject: [PATCH 1/6] Fix --- src/hotspot/share/oops/klass.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp index 9f166e13751a7..b2eeb2a2539d9 100644 --- a/src/hotspot/share/oops/klass.cpp +++ b/src/hotspot/share/oops/klass.cpp @@ -329,7 +329,7 @@ jint Klass::array_layout_helper(BasicType etype) { } int Klass::modifier_flags() const { - int mods = java_lang_Class::modifiers(java_mirror()); + int mods = java_lang_Class::modifiers(java_mirror_no_keepalive()); assert(mods == compute_modifier_flags(), "should be same"); return mods; } From 7269b441bafd7ea812349c5a3678b3186a07e6fb Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 25 Feb 2025 19:52:00 +0100 Subject: [PATCH 2/6] Re-introduce Klass cache --- src/hotspot/share/classfile/javaClasses.cpp | 1 + src/hotspot/share/oops/klass.cpp | 15 +++++++++++---- src/hotspot/share/oops/klass.hpp | 6 ++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index d6d1f799253cc..24d93eadbf7d0 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -1064,6 +1064,7 @@ void java_lang_Class::allocate_mirror(Klass* k, bool is_scratch, Handle protecti // Set the modifiers flag. int computed_modifiers = k->compute_modifier_flags(); set_modifiers(mirror(), computed_modifiers); + k->cache_modifier_flags(computed_modifiers); InstanceMirrorKlass* mk = InstanceMirrorKlass::cast(mirror->klass()); assert(oop_size(mirror()) == mk->instance_size(k), "should have been set"); diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp index b2eeb2a2539d9..d3f3b658e6521 100644 --- a/src/hotspot/share/oops/klass.cpp +++ b/src/hotspot/share/oops/klass.cpp @@ -291,7 +291,8 @@ static markWord make_prototype(const Klass* kls) { return prototype; } -Klass::Klass() : _kind(UnknownKlassKind) { +Klass::Klass() : _kind(UnknownKlassKind), + _cached_modifier_flags(max_jushort) { assert(CDSConfig::is_dumping_static_archive() || CDSConfig::is_using_archive(), "only for cds"); } @@ -300,6 +301,7 @@ Klass::Klass() : _kind(UnknownKlassKind) { // The constructor is also used from CppVtableCloner, // which doesn't zero out the memory before calling the constructor. Klass::Klass(KlassKind kind) : _kind(kind), + _cached_modifier_flags(max_jushort), _prototype_header(make_prototype(this)), _shared_class_path_index(-1) { CDS_ONLY(_shared_class_flags = 0;) @@ -328,10 +330,15 @@ jint Klass::array_layout_helper(BasicType etype) { return lh; } +void Klass::cache_modifier_flags(int flags) { + _cached_modifier_flags = checked_cast(flags); +} + int Klass::modifier_flags() const { - int mods = java_lang_Class::modifiers(java_mirror_no_keepalive()); - assert(mods == compute_modifier_flags(), "should be same"); - return mods; + int flags = checked_cast(_cached_modifier_flags); + assert(flags != max_jushort, "should be initialized"); + assert(flags == compute_modifier_flags(), "should be same"); + return flags; } bool Klass::can_be_primary_super_slow() const { diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp index 9ea021713fc42..bddec085e5a29 100644 --- a/src/hotspot/share/oops/klass.hpp +++ b/src/hotspot/share/oops/klass.hpp @@ -123,6 +123,11 @@ class Klass : public Metadata { AccessFlags _access_flags; // Access flags. The class/interface distinction is stored here. // Some flags created by the JVM, not in the class file itself, // are in _misc_flags below. + + // Java mirror carries the modifier flags. This cached field allows accessing + // modifiers when Java mirror is already dead, e.g. during class unloading. + u2 _cached_modifier_flags; + KlassFlags _misc_flags; // The fields _super_check_offset, _secondary_super_cache, _secondary_supers @@ -750,6 +755,7 @@ class Klass : public Metadata { public: virtual u2 compute_modifier_flags() const = 0; + void cache_modifier_flags(int flags); int modifier_flags() const; // JVMTI support From c302a4a2fd9e281fe3f0d62a68cfc2b68fa4eee2 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 25 Feb 2025 21:03:15 +0100 Subject: [PATCH 3/6] Alternate fix: compute modifiers directly --- src/hotspot/share/classfile/javaClasses.cpp | 1 - .../jfr/recorder/checkpoint/types/jfrTypeSet.cpp | 2 +- src/hotspot/share/oops/klass.cpp | 15 ++++----------- src/hotspot/share/oops/klass.hpp | 9 +++------ 4 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 24d93eadbf7d0..d6d1f799253cc 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -1064,7 +1064,6 @@ void java_lang_Class::allocate_mirror(Klass* k, bool is_scratch, Handle protecti // Set the modifiers flag. int computed_modifiers = k->compute_modifier_flags(); set_modifiers(mirror(), computed_modifiers); - k->cache_modifier_flags(computed_modifiers); InstanceMirrorKlass* mk = InstanceMirrorKlass::cast(mirror->klass()); assert(oop_size(mirror()) == mk->instance_size(k), "should have been set"); diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp index 8128674dc1e58..b9dcd7a3694e5 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp @@ -347,7 +347,7 @@ static void do_write_klass(JfrCheckpointWriter* writer, CldPtr cld, KlassPtr kla writer->write(cld != nullptr ? cld_id(cld, leakp) : 0); writer->write(mark_symbol(klass, leakp)); writer->write(package_id(klass, leakp)); - writer->write(klass->modifier_flags()); + writer->write(klass->compute_modifier_flags()); writer->write(klass->is_hidden()); if (leakp) { assert(IS_LEAKP(klass), "invariant"); diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp index d3f3b658e6521..9f166e13751a7 100644 --- a/src/hotspot/share/oops/klass.cpp +++ b/src/hotspot/share/oops/klass.cpp @@ -291,8 +291,7 @@ static markWord make_prototype(const Klass* kls) { return prototype; } -Klass::Klass() : _kind(UnknownKlassKind), - _cached_modifier_flags(max_jushort) { +Klass::Klass() : _kind(UnknownKlassKind) { assert(CDSConfig::is_dumping_static_archive() || CDSConfig::is_using_archive(), "only for cds"); } @@ -301,7 +300,6 @@ Klass::Klass() : _kind(UnknownKlassKind), // The constructor is also used from CppVtableCloner, // which doesn't zero out the memory before calling the constructor. Klass::Klass(KlassKind kind) : _kind(kind), - _cached_modifier_flags(max_jushort), _prototype_header(make_prototype(this)), _shared_class_path_index(-1) { CDS_ONLY(_shared_class_flags = 0;) @@ -330,15 +328,10 @@ jint Klass::array_layout_helper(BasicType etype) { return lh; } -void Klass::cache_modifier_flags(int flags) { - _cached_modifier_flags = checked_cast(flags); -} - int Klass::modifier_flags() const { - int flags = checked_cast(_cached_modifier_flags); - assert(flags != max_jushort, "should be initialized"); - assert(flags == compute_modifier_flags(), "should be same"); - return flags; + int mods = java_lang_Class::modifiers(java_mirror()); + assert(mods == compute_modifier_flags(), "should be same"); + return mods; } bool Klass::can_be_primary_super_slow() const { diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp index bddec085e5a29..5461fec0e3588 100644 --- a/src/hotspot/share/oops/klass.hpp +++ b/src/hotspot/share/oops/klass.hpp @@ -123,11 +123,6 @@ class Klass : public Metadata { AccessFlags _access_flags; // Access flags. The class/interface distinction is stored here. // Some flags created by the JVM, not in the class file itself, // are in _misc_flags below. - - // Java mirror carries the modifier flags. This cached field allows accessing - // modifiers when Java mirror is already dead, e.g. during class unloading. - u2 _cached_modifier_flags; - KlassFlags _misc_flags; // The fields _super_check_offset, _secondary_super_cache, _secondary_supers @@ -754,8 +749,10 @@ class Klass : public Metadata { virtual void release_C_heap_structures(bool release_constant_pool = true); public: + // Compute out modifier flags from the original data, instead of relying + // on Java mirror field. This is also allows accessing modifier flags + // when Java mirror is already dead, e.g. during class unloading. virtual u2 compute_modifier_flags() const = 0; - void cache_modifier_flags(int flags); int modifier_flags() const; // JVMTI support From f49438c8477ec2ff37c2a185676fb20e5a4e72b8 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 25 Feb 2025 21:07:54 +0100 Subject: [PATCH 4/6] Better comments --- src/hotspot/share/oops/klass.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp index 5461fec0e3588..7ceb6faea9ba1 100644 --- a/src/hotspot/share/oops/klass.hpp +++ b/src/hotspot/share/oops/klass.hpp @@ -749,12 +749,14 @@ class Klass : public Metadata { virtual void release_C_heap_structures(bool release_constant_pool = true); public: - // Compute out modifier flags from the original data, instead of relying - // on Java mirror field. This is also allows accessing modifier flags - // when Java mirror is already dead, e.g. during class unloading. - virtual u2 compute_modifier_flags() const = 0; + // Get modifier flags from Java mirror cache. int modifier_flags() const; + // Compute out modifier flags from the original data. This is also allows + // accessing modifier flags when Java mirror is already dead, e.g. during + // class unloading. + virtual u2 compute_modifier_flags() const = 0; + // JVMTI support virtual jint jvmti_class_status() const; From 8a77e58961fe7ed163e89f3bfd6a09a6d4c8db30 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 25 Feb 2025 21:11:48 +0100 Subject: [PATCH 5/6] More comment polishing, getting too late here for doing this without three commits in the row --- src/hotspot/share/oops/klass.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp index 7ceb6faea9ba1..4d940dabbf50c 100644 --- a/src/hotspot/share/oops/klass.hpp +++ b/src/hotspot/share/oops/klass.hpp @@ -752,9 +752,9 @@ class Klass : public Metadata { // Get modifier flags from Java mirror cache. int modifier_flags() const; - // Compute out modifier flags from the original data. This is also allows - // accessing modifier flags when Java mirror is already dead, e.g. during - // class unloading. + // Compute modifier flags from the original data. This is also allows + // accessing flags when Java mirror is already dead, e.g. during class + // unloading. virtual u2 compute_modifier_flags() const = 0; // JVMTI support From 0275ed1fa97e63a12e76754655fa1e142c397054 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 26 Feb 2025 08:31:17 +0100 Subject: [PATCH 6/6] Drop "is" --- src/hotspot/share/oops/klass.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp index 4d940dabbf50c..ca322ddff0a8d 100644 --- a/src/hotspot/share/oops/klass.hpp +++ b/src/hotspot/share/oops/klass.hpp @@ -752,7 +752,7 @@ class Klass : public Metadata { // Get modifier flags from Java mirror cache. int modifier_flags() const; - // Compute modifier flags from the original data. This is also allows + // Compute modifier flags from the original data. This also allows // accessing flags when Java mirror is already dead, e.g. during class // unloading. virtual u2 compute_modifier_flags() const = 0;