On 11/1/17 12:51 PM, Daniel D. Daugherty wrote:
Just one comment about the location of "jvm.h".
On 10/30/17 8:07 AM, coleen.phillim...@oracle.com wrote:
On 10/28/17 3:46 AM, David Holmes wrote:
On 28/10/2017 3:47 AM, 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).
Keeping it in prims avoids the need to touch many hotspot files, and
with no changes needed on the JDK side because we use a -I directive
to set the include path anyway. This is the exported VM interface so
it makes sense to me for it to be located in the VM sources.
But I'm not going to oppose this either way so it's up to Coleen.
I've already disagreed that this file belongs in
src/hotspot/share/prims, so the include directive without prims is
preferred. This allows putting jvm.h in a new place if/when that is
agreed upon.
The jvm.h file describes the internal JVM_* API implemented by
prims/jvm.cpp.
Because this is an internal interface, the jvm.h file would
traditionally be
co-located with the implementation (jvm.cpp) (and not in an include
directory).
So I disagree with the proposal to move jvm.h and believe the single copy
should be in prims/jvm.h.
So, we have a variety of opinions. Please open another ticket to
discuss this.
thanks,
Coleen
Dan
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
Unfortunately we did not do this though - hence the divergence
between the two. The use of int versus long for jint is causing a
real problem.
Coleen also hit the other issue on the head. The JNI and JVM
interfaces are C interfaces, not C++. The JDK code that uses them is
compiled as C - so all good. But the JVM code that implements them
is compiled as C++, and that is why we are getting issues with
differing linkage directives.
Well, there is now one source file for jvm.h and jni.h and their
machine dependent counterparts and 2500 lines of duplicated code is
removed with this change. The issues with jint and linkages are
resolved and tested as well with this changeset.
Thanks,
Coleen
David
-----
interface. This change will work with -I directive setting to the
new location, if changed later.
What do you think?
Mandy
