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

Reply via email to