On Tue, 16 Sep 2025 06:41:52 GMT, David Holmes <[email protected]> wrote:
>> Ioi Lam has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains three additional commits >> since the last revision: >> >> - @coleenp and @dholmes-ora comments -- simplify implementation; fixed code >> formatting >> - Merge branch 'master' into >> 8367719-refactor-jni-code-that-uses-class_to_verify_considering_redefinition >> - 8367719: Refactor JNI code that uses >> class_to_verify_considering_redefinition() > > Looks good. Just some minor nits. > > Thanks Thanks for the comments @dholmes-ora and @coleenp. I've made the changes you suggested. > I sort of think there's no need for the Klass* version of > get_class_considering_redefinition since its callers seem to really want an > InstanceKlass. Since it's jvm.cpp, you can probably verify that it's never > Klass in these callers. I think it's valid for some of the functions to be called with a non-instance klass, so I use the `get_instance_klass_considering_redefinition()` version only when the original code was doing an `InstanceKlass::cast()`. For functions like this one, I am keeping the type checks in the original code: JVM_ENTRY(jint, JVM_GetClassMethodsCount(JNIEnv *env, jclass cls)) Klass* k = get_klass_considering_redefinition(cls, thread); return (!k->is_instance_klass()) ? 0 : InstanceKlass::cast(k)->methods()->length(); JVM_END I did remove the `Klass*` version from jvmThreadState.hpp. I also updated the comments to reflect the new naming. ------------- PR Comment: https://git.openjdk.org/jdk/pull/27303#issuecomment-3310354049
