RFR: 8263589: Introduce JavaValue::get_oop/set_oop
Stefan Karlsson
stefank at openjdk.java.net
Mon Mar 15 12:43:26 UTC 2021
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?
-------------
Commit messages:
- Commit unstaged changes
- 8263589: Introduce JavaValue::get_oop/set_oop
Changes: https://git.openjdk.java.net/jdk/pull/3013/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3013&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8263589
Stats: 72 lines in 26 files changed: 5 ins; 0 del; 67 mod
Patch: https://git.openjdk.java.net/jdk/pull/3013.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3013/head:pull/3013
PR: https://git.openjdk.java.net/jdk/pull/3013
More information about the serviceability-dev
mailing list