On 10/27/17 1:47 PM, mandy chung wrote:
On 10/27/17 7:08 AM, coleen.phillim...@oracle.com wrote:
On 10/27/17 9:37 AM, David Holmes wrote:
The one file that is needed is a hotspot file - jvm.h defines the
interface that hotspot exports via jvm.cpp.
If you leave jvm.h in hotspot/prims then a very large chunk of your
boilerplate changes are not needed. The JDK code doesn't care what
the name of the directory is - whatever it is just gets added as a
-I directive (the JDK code will include "jvm.h" not "prims/jvm.h"
the way hotspot sources do.
This isn't something we want to change back or move again later.
Whatever we do now we live with.
I think it belongs with jni.h and I think the core libraries group
would agree. It seems more natural there than buried in the hotspot
prims directory. I guess this is on hold while we have this
debate. Sigh.
Actually with -I directives, changing to jvm.h from prims/jvm.h would
still work. Maybe we should change the name to jvm.hpp since it's
jvm.cpp though? Or maybe just have two divergent copies and close
this as WNF.
I also think hotspot/prims is not a good location.
src/java.base/share/include is a well-defined location for native
header files. Maybe internal header files could be placed in
include/internal but this is a separate issue . I should create an
issue for jvm.h and jmm.h (I looked at the files under the include
directory and jvm.h and jmm.h are the only two internal header files
in the include directory).
I do think removing the duplicated copy of jvm.h is a good change.
This is finally possible with the consolidated repository and we no
longer need to update two copies of jvm.h for any change to the JVM
interface. This change will work with -I directive setting to the
new location, if changed later.
What do you think?
I agree. I'm not really bothered by it being in
src/java/base/share/include in the first place though. Only jni.h and
jni_md.h are copied into the images, so this seems a bit pained to make
jvm.h be in some other directory. But your call, really.
Thanks,
Coleen
Mandy
