RFR: JDK-8189607 Remove duplicated jvmticmlr.h

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8189607 Remove duplicated jvmticmlr.h

Magnus Ihse Bursie
The file jvmticmlr.h is stored twice in the repo, both in hotspot and in
java.base. They are both identical, and only the java.base version is
included in the final product. This might arguably have been useful in a
pre-consolidated world, but makes absolutely no sense now.

Bug: https://bugs.openjdk.java.net/browse/JDK-8189607
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8189607-remove-duplicated-jvmticmlr/webrev.01

/Magnus
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189607 Remove duplicated jvmticmlr.h

Erik Joelsson
On 2017-10-18 10:04, Magnus Ihse Bursie wrote:
> The file jvmticmlr.h is stored twice in the repo, both in hotspot and
> in java.base. They are both identical, and only the java.base version
> is included in the final product. This might arguably have been useful
> in a pre-consolidated world, but makes absolutely no sense now.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8189607
> WebRev:
> http://cr.openjdk.java.net/~ihse/JDK-8189607-remove-duplicated-jvmticmlr/webrev.01
>
The question is, which file location makes the most sense. I think your
pick of java.base/share/native/include probably makes more sense as that
makes it much clearer that this is an exported header file.

Looks good to me.

/Erik

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189607 Remove duplicated jvmticmlr.h

Magnus Ihse Bursie
On 2017-10-18 10:26, Erik Joelsson wrote:

> On 2017-10-18 10:04, Magnus Ihse Bursie wrote:
>> The file jvmticmlr.h is stored twice in the repo, both in hotspot and
>> in java.base. They are both identical, and only the java.base version
>> is included in the final product. This might arguably have been
>> useful in a pre-consolidated world, but makes absolutely no sense now.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189607
>> WebRev:
>> http://cr.openjdk.java.net/~ihse/JDK-8189607-remove-duplicated-jvmticmlr/webrev.01
>>
> The question is, which file location makes the most sense. I think
> your pick of java.base/share/native/include probably makes more sense
> as that makes it much clearer that this is an exported header file.
Yes, that was my reasoning. Also, the file is not really tied to hotspot
per se -- if you were to plug in another VM, you'd still need this file.
Combined with the fact that this was the file that was exported to the
world. (Which doesn't *really* make any difference in this case, since
the files were identical...)

> Looks good to me.
Thanks.

/Magnus
>
> /Erik
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189607 Remove duplicated jvmticmlr.h

serguei.spitsyn@oracle.com
In reply to this post by Magnus Ihse Bursie
Hi Magnus,

The fix looks good to me.
Thank you for doing this cleanup.

Thanks,
Serguei


On 10/18/17 01:04, Magnus Ihse Bursie wrote:

> The file jvmticmlr.h is stored twice in the repo, both in hotspot and
> in java.base. They are both identical, and only the java.base version
> is included in the final product. This might arguably have been useful
> in a pre-consolidated world, but makes absolutely no sense now.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8189607
> WebRev:
> http://cr.openjdk.java.net/~ihse/JDK-8189607-remove-duplicated-jvmticmlr/webrev.01
>
> /Magnus

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189607 Remove duplicated jvmticmlr.h

David Holmes
In reply to this post by Magnus Ihse Bursie
Hi Magnus,

This seems fine to me.

Sanity check: the various -Ixxx will be processed in order and the first
file found will be used - right? ie we won't unintentionally pick up the
java.base jni.h.

Thanks,
David

On 18/10/2017 6:04 PM, Magnus Ihse Bursie wrote:

> The file jvmticmlr.h is stored twice in the repo, both in hotspot and in
> java.base. They are both identical, and only the java.base version is
> included in the final product. This might arguably have been useful in a
> pre-consolidated world, but makes absolutely no sense now.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8189607
> WebRev:
> http://cr.openjdk.java.net/~ihse/JDK-8189607-remove-duplicated-jvmticmlr/webrev.01 
>
>
> /Magnus
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189607 Remove duplicated jvmticmlr.h

Erik Joelsson
Hello David,


On 2017-10-18 11:29, David Holmes wrote:
>
> Sanity check: the various -Ixxx will be processed in order and the
> first file found will be used - right? ie we won't unintentionally
> pick up the java.base jni.h.
>
Correct, the search order is the order in which the -I parameters are
listed on the command line.

/Erik

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189607 Remove duplicated jvmticmlr.h

Mandy Chung
In reply to this post by Erik Joelsson


On 10/18/17 1:26 AM, Erik Joelsson wrote:

> On 2017-10-18 10:04, Magnus Ihse Bursie wrote:
>> The file jvmticmlr.h is stored twice in the repo, both in hotspot and
>> in java.base. They are both identical, and only the java.base version
>> is included in the final product. This might arguably have been
>> useful in a pre-consolidated world, but makes absolutely no sense now.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189607
>> WebRev:
>> http://cr.openjdk.java.net/~ihse/JDK-8189607-remove-duplicated-jvmticmlr/webrev.01
>>
> The question is, which file location makes the most sense. I think
> your pick of java.base/share/native/include probably makes more sense
> as that makes it much clearer that this is an exported header file.
>

jvmticmlr.h is an exported header file and
java.base/share/native/include is a proper location as described in JEP
201 about the modular source layout.

The change looks good to me too.

Mandy