Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Ioi Lam
On Tue, 2 Feb 2021 16:00:43 GMT, Gerard Ziemski  wrote:

>> I am not sure if jni_utils.c is the right file (it defines the `JNU_XXX` 
>> functions that are used by other shared libraries).
>> 
>> There are other .c files that have trivial `DEF_JNI_OnLoad` functions (e.g., 
>> java.base/share/native/libnio/nio_util.c).
>> 
>> @AlanBateman do you have any suggestions?
>
> I'm fine with the way it is, just thought we might want to consider cleaning 
> up a bit more, since it's a cleanup task itself.

Thanks Gerard. The main purpose of this PR is to clean up the JVM side. I'll 
leave the refactoring of check_version.c to the core-lib team.

-

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Ioi Lam
On Tue, 2 Feb 2021 15:59:47 GMT, Gerard Ziemski  wrote:

>> Ioi Lam has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixed macos build
>
> Marked as reviewed by gziemski (Committer).

Thanks @gerard-ziemski @magicus @AlanBateman @lfoltan  for the review.

-

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Gerard Ziemski
On Mon, 1 Feb 2021 20:10:58 GMT, Ioi Lam  wrote:

>> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which 
>> allowed the same JDK library to use different version of HotSpot. However, 
>> HSX is no longer supported so this API should be removed.
>> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
>> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
>> declarations should be removed from jvm.h
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fixed macos build

Marked as reviewed by gziemski (Committer).

-

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Gerard Ziemski
On Mon, 1 Feb 2021 20:54:42 GMT, Ioi Lam  wrote:

>> src/java.base/share/native/libjava/check_version.c line 33:
>> 
>>> 31: DEF_JNI_OnLoad(JavaVM *vm, void *reserved)
>>> 32: {
>>> 33: return JNI_VERSION_1_2;
>> 
>> This leaves an entire file with one trivial function implementation. Can we 
>> remove the file and implement  `DEF_JNI_OnLoad()` in `jni_util.h` (or some 
>> other existing suitable file) ?
>
> I am not sure if jni_utils.c is the right file (it defines the `JNU_XXX` 
> functions that are used by other shared libraries).
> 
> There are other .c files that have trivial `DEF_JNI_OnLoad` functions (e.g., 
> java.base/share/native/libnio/nio_util.c).
> 
> @AlanBateman do you have any suggestions?

I'm fine with the way it is, just thought we might want to consider cleaning up 
a bit more, since it's a cleanup task itself.

-

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Magnus Ihse Bursie
On Mon, 1 Feb 2021 20:10:58 GMT, Ioi Lam  wrote:

>> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which 
>> allowed the same JDK library to use different version of HotSpot. However, 
>> HSX is no longer supported so this API should be removed.
>> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
>> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
>> declarations should be removed from jvm.h
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fixed macos build

"Build" change looks good.

-

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-01 Thread Ioi Lam
On Mon, 1 Feb 2021 20:29:10 GMT, Gerard Ziemski  wrote:

>> Ioi Lam has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixed macos build
>
> src/java.base/share/native/libjava/check_version.c line 33:
> 
>> 31: DEF_JNI_OnLoad(JavaVM *vm, void *reserved)
>> 32: {
>> 33: return JNI_VERSION_1_2;
> 
> This leaves an entire file with one trivial function implementation. Can we 
> remove the file and implement  `DEF_JNI_OnLoad()` in `jni_util.h` (or some 
> other existing suitable file) ?

I am not sure if jni_utils.c is the right file (it defines the `JNU_XXX` 
functions that are used by other shared libraries).

There are other .c files that have trivial `DEF_JNI_OnLoad` functions (e.g., 
java.base/share/native/libnio/nio_util.c).

@AlanBateman do you have any suggestions?

-

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-01 Thread Gerard Ziemski
On Mon, 1 Feb 2021 20:10:58 GMT, Ioi Lam  wrote:

>> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which 
>> allowed the same JDK library to use different version of HotSpot. However, 
>> HSX is no longer supported so this API should be removed.
>> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
>> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
>> declarations should be removed from jvm.h
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fixed macos build

Changes requested by gziemski (Committer).

src/java.base/share/native/libjava/check_version.c line 33:

> 31: DEF_JNI_OnLoad(JavaVM *vm, void *reserved)
> 32: {
> 33: return JNI_VERSION_1_2;

This leaves an entire file with one trivial function implementation. Can we 
remove the file and implement  `DEF_JNI_OnLoad()` in `jni_util.h` (or some 
other existing suitable file) ?

-

PR: https://git.openjdk.java.net/jdk/pull/2338


Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-01 Thread Ioi Lam
> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which allowed 
> the same JDK library to use different version of HotSpot. However, HSX is no 
> longer supported so this API should be removed.
> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
> declarations should be removed from jvm.h

Ioi Lam has updated the pull request incrementally with one additional commit 
since the last revision:

  fixed macos build

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2338/files
  - new: https://git.openjdk.java.net/jdk/pull/2338/files/c0307e7d..3a6415eb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2338&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2338&range=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2338.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2338/head:pull/2338

PR: https://git.openjdk.java.net/jdk/pull/2338