RFR (L, tedious again, sorry) 8189610: Reconcile jvm.h and all jvm_md.h between java.base and hotspot

[Daniel D. Daugherty]

Just one comment about the location of "jvm.h".


On 10/30/17 8:07 AM, coleen.phillimore at 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.phillimore at 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.

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
>