Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]
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]
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]
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]
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]
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]
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]
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]
> - 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
