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