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

[Daniel D. Daugherty]

> So, we have a variety of opinions.   Please open another ticket to 
> discuss this. 

Sorry, I didn't realize that this fix pushed yesterday.
I'm not completely caught up on all of the email. It's
not worth another bug report (and more changes).

Dan



On 11/1/17 2:09 PM, coleen.phillimore at oracle.com wrote:
>
>
> 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
>>>
>>
>
>