On Tue, 9 Sep 2025 05:58:32 GMT, David Holmes <[email protected]> wrote:
> Sorry but I think this PR is trying to do too many things at once. > > It is pushing JNI resolving inside internal JVM methods, which I think is a > bad thing - we resolve JNI references at the boundary to get the oop that the > VM wants to work with. Internal APIs should be oblivious to jobject and > friends IMO. Also there may be times that the JNI/JVM method needs get the > oop itself before extracting the klass. There are 92 header files that have the word `jobject` in them. There are 71 cpp/hpp files with the word `JNIHandles::resolve` in them. So I am not sure if we really have that separation anymore. There are 54 cases where we call `as_Klass(JNIHandles::resolve_non_null(x))`, so I think it would be nice to have a way to write less code. > You are converting klass to instanceKlass where it has to be instanceKlass > e.g. with redefinition. This is a good thing, but it is a very distinct thing > that deserves its own cleanup (as per previous changes in that area). I can move the considering_redefinition changes in a follow-up PR. > You are defining a helper `java_lang_Class::as_InstanceKlass` to internalize > the cast - this is fine but again a simple cleanup that would be better > standalone IMO. > > It is also not clear that JVM/JNI API's are properly checking that the > incoming jobject is in fact a class of the right kind (ie not an array class > object). I am just converting InstanceKlass* ik = InstanceKlass::cast(as_Klass(mirror)); to InstanceKlass* ik = as_InstanceKlass(mirror); The code already assumes that it has an InstanceKlass, and I am not changing that. ------------- PR Comment: https://git.openjdk.org/jdk/pull/27158#issuecomment-3269072414
