On Mon, 15 Mar 2021 12:35:47 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
> JavaValue is a small wrapper class that wraps values used to pass arguments > and results between native and Java. > > When JavaCalls::call returns an object, the value stored in the JavaValue is > not a handliezed jobject. Instead it's a raw oop. So, most of the code > handling the `result`, fetches the result as a jobject, and then immediately > casts it to an oop. For example: > oop res = (oop)result.get_jobject(); > > I'd like to change this code to be: > oop res = result.get_oop(); > > The motivations for this patch is: > > 1) Minimize the places where we pass around oops in jobject variables. Maybe > at some point we'll have converted the JVM to only use the jobject type when > passing around JNI handle. We need to be stricter with the types when we > continue develop our GCs and their barriers. > > 2) Limit the number of places in the code where we perform raw oop casts. We > have a helper cast function for that, cast_to_oop, but not all code use it. I > have future patches where the compiler will completely forbid raw cast to > oops (in fastdebug builds). With that in place, I can then add more stricter > oop verification code when oops are created. This helps catching bugs earlier. > > --- > > When reviewing this patch, take an extra look at the change to > oopsHierarchy.hpp. This was done to support jvmciEnv.cpp code: > JVMCIObject wrap(oop obj)... > JVMCIObjectArray wrap(objArrayOop obj)... > JVMCIPrimitiveArray wrap(typeArrayOop obj) ... > Previously, `wrap((oop)result.get_jobject())` called the first function. When > the code was changed to `wrap(result.get_oop())`, where `get_oop()` returns a > `oopDesc*`, the compiler didn't know what conversion in oopsHierarchy.hpp to > use. Therefore, I replaced the overly permissive `void*` constructor with a > constructor that only takes the corresponding `type##OopDesc*`. > > An alternative would be to let get_oop() return an oop, but then that would > add an unwanted a dependency between globalDefinitions.hpp and > oopsHierarchy.hpp. An earlier version of this patch did return an oop instead > of oopDesc*, but it also moved entire JavaValue class out of > globalDefinitions.hpp into a new javaValue.hpp file, and had a corresponding > javaValue.inline.hpp file. > > Even if we end up using the proposed `oopDesc* get_oop()` version, maybe > moving the class to javaValues.hpp would still makes sense? This pull request has now been integrated. Changeset: a1f6591f Author: Stefan Karlsson <stef...@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/a1f6591f Stats: 72 lines in 26 files changed: 5 ins; 0 del; 67 mod 8263589: Introduce JavaValue::get_oop/set_oop Reviewed-by: coleenp, sspitsyn ------------- PR: https://git.openjdk.java.net/jdk/pull/3013
