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

[coleen.phillimore at oracle.com]

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.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.

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
>>
>