On Tue, 16 Sep 2025 02:44:26 GMT, Ioi Lam <[email protected]> wrote:
> Simplify the boilerplate code in jvm.cpp that calls
> `JvmtiThreadState::class_to_verify_considering_redefinition()`, and reduce
> the number of `InstanceKlass::cast()` calls.
>
> I also changed a few fields/arguments from `Klass*` to `InstanceKlass*` as
> these are used exclusively with `InstanceKlass*`.
Looks good. Just some minor nits.
Thanks
src/hotspot/share/prims/jvm.cpp line 2263:
> 2261: // Please, refer to the description in the jvmtiThreadState.hpp.
> 2262:
> 2263: inline Klass* get_klass_considering_redefinition(jclass cls, JavaThread
> *thread) {
Suggestion:
inline Klass* get_klass_considering_redefinition(jclass cls, JavaThread*
thread) {
src/hotspot/share/prims/jvm.cpp line 2269:
> 2267: }
> 2268:
> 2269: inline InstanceKlass*
> get_instance_klass_considering_redefinition(jclass cls, JavaThread *thread) {
Suggestion:
inline InstanceKlass* get_instance_klass_considering_redefinition(jclass cls,
JavaThread* thread) {
src/hotspot/share/prims/jvmtiThreadState.hpp line 31:
> 29: #include "memory/allocation.hpp"
> 30: #include "oops/oopHandle.hpp"
> 31: #include "oops/instanceKlass.hpp"
Suggestion:
#include "oops/instanceKlass.hpp"
#include "oops/oopHandle.hpp"
Include order is wrong.
src/hotspot/share/prims/jvmtiThreadState.hpp line 438:
> 436: static inline
> 437: Klass* class_to_verify_considering_redefinition(Klass* klass,
> 438: JavaThread *thread) {
Suggestion:
JavaThread* thread) {
src/hotspot/share/prims/jvmtiThreadState.hpp line 445:
> 443: } else {
> 444: return
> class_to_verify_considering_redefinition(InstanceKlass::cast(klass),
> 445: thread);
Suggestion:
return
class_to_verify_considering_redefinition(InstanceKlass::cast(klass), thread);
src/hotspot/share/prims/jvmtiThreadState.hpp line 451:
> 449: static inline
> 450: InstanceKlass* class_to_verify_considering_redefinition(InstanceKlass*
> klass,
> 451: JavaThread
> *thread) {
Suggestion:
JavaThread* thread) {
-------------
Changes requested by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/27303#pullrequestreview-3227749202
PR Review Comment: https://git.openjdk.org/jdk/pull/27303#discussion_r2351033232
PR Review Comment: https://git.openjdk.org/jdk/pull/27303#discussion_r2351035033
PR Review Comment: https://git.openjdk.org/jdk/pull/27303#discussion_r2351044566
PR Review Comment: https://git.openjdk.org/jdk/pull/27303#discussion_r2351048289
PR Review Comment: https://git.openjdk.org/jdk/pull/27303#discussion_r2351050636
PR Review Comment: https://git.openjdk.org/jdk/pull/27303#discussion_r2351051532
