Re: RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
On Tue, 11 Jun 2024 13:09:21 GMT, Jaikiran Pai wrote: > Can I please get a review of this test-only change which proposes to address > intermittent failures in > `test/jdk/java/io/Serializable/concurrentClassDescLookup/ConcurrentClassDescLookup.java`? > > This addresses https://bugs.openjdk.org/browse/JDK-8222884. As noted in that > issue, the test in its current form has a potential race. The test main > thread launches 50 "successful lookup" tasks and 50 "failed lookup" tasks. > Each task is a `Thread`. The test passes an `Object` instance shared by 50 of > these tasks and each task when it starts execution in its `run()` method does > a `wait()` on that `Object` instance. The test main thread then after > launching 50 tasks, sleeps for a second (to let the 50 `Thread`s start > execution and arrive at the `Object.wait()`) and then after waking up the > main thread does a `notifyAll()` on that `Object` instance. In theory and > likely in practice too, it's possible that (especially) for the last batch of > 50 tasks (the "failing lookup" tasks), the `notifyAll()` might end up getting > invoked before one or more task Thread(s) have actually done a `wait()`. That > can then mean that one or more task `Thread`(s) will keep `wait()`ing > indefinitely with no on e `notify()`ing the `Object` instance to proceed with the task execution. > > The commit in this PR removes the use of `Object.wait()/notifyAll()` and > instead uses a `CountdownLatch` to allow for a more deterministic barrier. > The test continues to pass with this change. Additionally, this change was > run against a CI setup, by Matthias, where it was previously failing > intermittently. Matthias reports that it hasn't failed after this change. Marked as reviewed by mbaesken (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19659#pullrequestreview-2110799797
Integrated: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null
On Tue, 21 May 2024 14:28:38 GMT, Matthias Baesken wrote: > When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing > jtreg tests afterwards I run into this error : > > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: > null pointer passed as argument 2, which is declared to never be null > #0 0x7fd95bec78d8 in spawnChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 > #1 0x7fd95bec78d8 in startChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 > #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 > #3 0x7fd93797a06d () > > this is the memcpy call getting an unexpected null pointer : > memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. > Something similar was discussed and fixed here > https://bugs.python.org/issue27570 for Python . > > Similar issue in OpenJDK _ > https://bugs.openjdk.org/browse/JDK-8332473 > 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed > as argument 1, which is declared to never be null This pull request has now been integrated. Changeset: 16dba04e Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/16dba04e8dfa871f8056480a42a9baeb24a2fb24 Stats: 12 lines in 1 file changed: 9 ins; 0 del; 3 mod 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null Reviewed-by: rriggs, mdoerr - PR: https://git.openjdk.org/jdk/pull/19329
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null [v2]
On Thu, 23 May 2024 07:26:14 GMT, Matthias Baesken wrote: >> When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing >> jtreg tests afterwards I run into this error : >> >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime >> error: null pointer passed as argument 2, which is declared to never be null >> #0 0x7fd95bec78d8 in spawnChild >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 >> #1 0x7fd95bec78d8 in startChild >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 >> #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 >> #3 0x7fd93797a06d () >> >> this is the memcpy call getting an unexpected null pointer : >> memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. >> Something similar was discussed and fixed here >> https://bugs.python.org/issue27570 for Python . >> >> Similar issue in OpenJDK _ >> https://bugs.openjdk.org/browse/JDK-8332473 >> 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed >> as argument 1, which is declared to never be null > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remarks Roger Riggs Hi Magnus, I added handling of the special case that memcpy src is a null pointer and non-0 length was given . - PR Comment: https://git.openjdk.org/jdk/pull/19329#issuecomment-2128779448
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null [v3]
> When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing > jtreg tests afterwards I run into this error : > > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: > null pointer passed as argument 2, which is declared to never be null > #0 0x7fd95bec78d8 in spawnChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 > #1 0x7fd95bec78d8 in startChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 > #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 > #3 0x7fd93797a06d () > > this is the memcpy call getting an unexpected null pointer : > memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. > Something similar was discussed and fixed here > https://bugs.python.org/issue27570 for Python . > > Similar issue in OpenJDK _ > https://bugs.openjdk.org/browse/JDK-8332473 > 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed > as argument 1, which is declared to never be null Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: handle special case that memcpy src is NULL but a len larger than 0 was given - Changes: - all: https://git.openjdk.org/jdk/pull/19329/files - new: https://git.openjdk.org/jdk/pull/19329/files/b03ff0c5..aad00366 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19329=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19329=01-02 Stats: 10 lines in 1 file changed: 7 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19329/head:pull/19329 PR: https://git.openjdk.org/jdk/pull/19329
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null [v2]
On Thu, 23 May 2024 07:26:14 GMT, Matthias Baesken wrote: >> When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing >> jtreg tests afterwards I run into this error : >> >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime >> error: null pointer passed as argument 2, which is declared to never be null >> #0 0x7fd95bec78d8 in spawnChild >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 >> #1 0x7fd95bec78d8 in startChild >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 >> #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec >> /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 >> #3 0x7fd93797a06d () >> >> this is the memcpy call getting an unexpected null pointer : >> memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. >> Something similar was discussed and fixed here >> https://bugs.python.org/issue27570 for Python . >> >> Similar issue in OpenJDK _ >> https://bugs.openjdk.org/browse/JDK-8332473 >> 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed >> as argument 1, which is declared to never be null > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remarks Roger Riggs Roger/Magnus, should I still add some handling for Magnus' concern : "However, if we ever call this with a non-zero sp.dirlen and a null c->pdir, we'd be in trouble." E.g. check for this and return -1 ? - PR Comment: https://git.openjdk.org/jdk/pull/19329#issuecomment-2127284704
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null [v2]
On Thu, 23 May 2024 10:57:28 GMT, Magnus Ihse Bursie wrote: >However, if we ever call this with a non-zero sp.dirlen and a null c->pdir, >we'd be in trouble. In the old code, we would have > crashed. Now, we will just silently ignore this, and God knows what will > happen after that part. In this special case we can return -1 ; the spawnChild function already returns -1 in some other error cases. - PR Comment: https://git.openjdk.org/jdk/pull/19329#issuecomment-2126888052
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null
On Tue, 21 May 2024 14:28:38 GMT, Matthias Baesken wrote: > When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing > jtreg tests afterwards I run into this error : > > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: > null pointer passed as argument 2, which is declared to never be null > #0 0x7fd95bec78d8 in spawnChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 > #1 0x7fd95bec78d8 in startChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 > #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 > #3 0x7fd93797a06d () > > this is the memcpy call getting an unexpected null pointer : > memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. > Something similar was discussed and fixed here > https://bugs.python.org/issue27570 for Python . > > Similar issue in OpenJDK _ > https://bugs.openjdk.org/browse/JDK-8332473 > 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed > as argument 1, which is declared to never be null Hi Roger, thanks for the hint ! I adjusted the coding. - PR Comment: https://git.openjdk.org/jdk/pull/19329#issuecomment-2126408877
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null [v2]
> When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing > jtreg tests afterwards I run into this error : > > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: > null pointer passed as argument 2, which is declared to never be null > #0 0x7fd95bec78d8 in spawnChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 > #1 0x7fd95bec78d8 in startChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 > #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 > #3 0x7fd93797a06d () > > this is the memcpy call getting an unexpected null pointer : > memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. > Something similar was discussed and fixed here > https://bugs.python.org/issue27570 for Python . > > Similar issue in OpenJDK _ > https://bugs.openjdk.org/browse/JDK-8332473 > 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed > as argument 1, which is declared to never be null Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remarks Roger Riggs - Changes: - all: https://git.openjdk.org/jdk/pull/19329/files - new: https://git.openjdk.org/jdk/pull/19329/files/f2bebda2..b03ff0c5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19329=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19329=00-01 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19329/head:pull/19329 PR: https://git.openjdk.org/jdk/pull/19329
Re: RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null
On Wed, 22 May 2024 07:38:07 GMT, Magnus Ihse Bursie wrote: > How come `c->pdir` can be null? Is it if `sp.dirlen` is 0? I added a bit of tracing output and get Attention c->pdir is null; sp.dirlen is 0 So yes sp.dirlen is also 0 . - PR Comment: https://git.openjdk.org/jdk/pull/19329#issuecomment-2124271807
RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null
When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing jtreg tests afterwards I run into this error : /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null #0 0x7fd95bec78d8 in spawnChild /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 #1 0x7fd95bec78d8 in startChild /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 #3 0x7fd93797a06d () this is the memcpy call getting an unexpected null pointer : memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. Something similar was discussed and fixed here https://bugs.python.org/issue27570 for Python . Similar issue in OpenJDK _ https://bugs.openjdk.org/browse/JDK-8332473 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed as argument 1, which is declared to never be null - Commit messages: - JDK-8332589 Changes: https://git.openjdk.org/jdk/pull/19329/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19329=00 Issue: https://bugs.openjdk.org/browse/JDK-8332589 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19329/head:pull/19329 PR: https://git.openjdk.org/jdk/pull/19329
Re: RFR: 8329653: JLILaunchTest fails on AIX after JDK-8329131
On Fri, 3 May 2024 11:31:35 GMT, Alan Bateman wrote: > > I was busy with other things and missed JoKern65's comment. I would be less > risky to limit this to AIX for now. Thanks for your comment; we see the issue only on AIX so it makes sense to limit it to this platform. - PR Comment: https://git.openjdk.org/jdk/pull/19000#issuecomment-2092825223
Re: RFR: 8329653: JLILaunchTest fails on AIX after JDK-8329131
On Mon, 29 Apr 2024 14:45:07 GMT, Joachim Kern wrote: > Since ~ end of March, after > [JDK-8329131](https://bugs.openjdk.org/browse/JDK-8329131), > tools/launcher/JliLaunchTest.java fails on AIX. Failure is : > > stdout: []; > stderr: [Error: could not find libjava.so > Error: Could not find Java SE Runtime Environment. > ] > exitValue = 2 > > java.lang.RuntimeException: Expected to get exit value of [0], exit value is: > [2] > at > jdk.test.lib.process.OutputAnalyzer.shouldHaveExitValue(OutputAnalyzer.java:521) > at JliLaunchTest.main(JliLaunchTest.java:58) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > at > com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333) > at java.base/java.lang.Thread.run(Thread.java:1575) > > Maybe we need to do further adjustments to > BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeJliLaunchTest / > BUILD_JDK_JTREG_EXECUTABLES_LDFLAGS_exeJliLaunchTest on AIX ? > Or somehow adjust the coding that attempts to find parts of "JRE" > (libjava.so) ? The libjli.so is gone on AIX after > [JDK-8329131](https://bugs.openjdk.org/browse/JDK-8329131), and with this > also the image discovery based on GetApplicationHomeFromDll which uses > libjli.so . > > Without libjli.so we have to analyze the LD-LIBRARY_PATH / LIBPATH envvar. > There is no other methos available on AIX, because it lacks the $ORIGIN > feature of the rpath. The naming GetApplicationHomeFromLD_LIBRARY_PATH looks a bit unconventional ; maybe adjust this ? Regarding if the code should be added for all platforms or just AIX, let's hear what Alan and others have to say. On AIX we have the bad situation that GetApplicationHomeFromDll stopped working after JDK-8329131. - PR Comment: https://git.openjdk.org/jdk/pull/19000#issuecomment-2092530647
Re: RFR: 8329862: libjli GetApplicationHome cleanups and enhance jli tracing [v6]
On Thu, 25 Apr 2024 13:22:01 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove /jre path check Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2078867092
Integrated: 8329862: libjli GetApplicationHome cleanups and enhance jli tracing
On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken wrote: > We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. This pull request has now been integrated. Changeset: 377f2e53 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/377f2e538ae0fc94fc5483700a3ae70175017741 Stats: 46 lines in 2 files changed: 8 ins; 36 del; 2 mod 8329862: libjli GetApplicationHome cleanups and enhance jli tracing Reviewed-by: clanger, stuefe - PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: libjli GetApplicationHome cleanups and enhance jli tracing [v6]
On Fri, 26 Apr 2024 07:00:07 GMT, Thomas Stuefe wrote: > Still looks good. We should maybe change the synopsis (title) of the bug to > something like "libjli GetApplicationHome cleanups"? I changed the synopsis because it was indeed now not really reflecting any more what the change does. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2078821181
Integrated: 8330615: avoid signed integer overflows in zip_util.c readCen / hashN
On Tue, 23 Apr 2024 07:51:28 GMT, Matthias Baesken wrote: > In the hashN usages of readCen from zip_util.c we see a lot of signed integer > overflows. > For example in the java/util jtreg tests those are easily reproducable when > compiling with -ftrapv (clang/gcc toolchains). > While those overflows never seem to cause crashes or similar errors, they are > unwanted because > signed integer overflows in C cause undefined behavior. > See > https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Signed-Overflow.html >> >> For signed integers, the result of overflow in C is in principle undefined, >> meaning that anything whatsoever could happen. >> Therefore, C compilers can do optimizations that treat the overflow case >> with total unconcern. > > So we might still get unwanted results (maybe bad/strange hashing, depending > on compiler and optimization level). > > Compilation with -ftrapv causes/triggers this SIGILL on macOS showing the > issue : > # Problematic frame: > # C [libzip.dylib+0x6362] hashN+0x32 > # > > Stack: [0x7c496000,0x7c596000], sp=0x7c5957e0, free > space=1021k > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native > code) > C [libzip.dylib+0x6362] hashN+0x32 > C [libzip.dylib+0x5d5e] readCEN+0xd2e > C [libzip.dylib+0x4ee0] ZIP_Put_In_Cache0+0x160 > V [libjvm.dylib+0x544b1e] ClassLoader::open_zip_file(char const*, char**, > JavaThread*)+0x3e > V [libjvm.dylib+0x543fec] ClassLoader::create_class_path_entry(JavaThread*, > char const*, stat const*, bool, bool)+0x6c > V [libjvm.dylib+0x543833] > ClassLoader::setup_bootstrap_search_path_impl(JavaThread*, char const*)+0xf3 > V [libjvm.dylib+0x54819b] classLoader_init1()+0x1b > V [libjvm.dylib+0x92602a] init_globals()+0x3a > V [libjvm.dylib+0x12b3b74] Threads::create_vm(JavaVMInitArgs*, bool*)+0x314 > V [libjvm.dylib+0xa848f4] JNI_CreateJavaVM+0x64 > C [libjli.dylib+0x4483] JavaMain+0x123 > C [libjli.dylib+0x7529] ThreadJavaMain+0x9 > C [libsystem_pthread.dylib+0x68fc] _pthread_start+0xe0 > C [libsystem_pthread.dylib+0x2443] thread_start+0xf This pull request has now been integrated. Changeset: 5af6b45e Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/5af6b45eefd227e3e046ca22a404ae8a23174160 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8330615: avoid signed integer overflows in zip_util.c readCen / hashN Reviewed-by: lucy, mdoerr - PR: https://git.openjdk.org/jdk/pull/18908
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v6]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove /jre path check - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/4d998244..4d8b6802 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=04-05 Stats: 24 lines in 2 files changed: 0 ins; 24 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
On Tue, 23 Apr 2024 14:31:44 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust output I removed the mentioned 'private JRE' check and also the related size check. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2077160938
Re: RFR: 8330615: avoid signed integer overflows in zip_util.c readCen / hashN
On Tue, 23 Apr 2024 07:51:28 GMT, Matthias Baesken wrote: > In the hashN usages of readCen from zip_util.c we see a lot of signed integer > overflows. > For example in the java/util jtreg tests those are easily reproducable when > compiling with -ftrapv (clang/gcc toolchains). > While those overflows never seem to cause crashes or similar errors, they are > unwanted because > signed integer overflows in C cause undefined behavior. > See > https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Signed-Overflow.html >> >> For signed integers, the result of overflow in C is in principle undefined, >> meaning that anything whatsoever could happen. >> Therefore, C compilers can do optimizations that treat the overflow case >> with total unconcern. > > So we might still get unwanted results (maybe bad/strange hashing, depending > on compiler and optimization level). > > Compilation with -ftrapv causes/triggers this SIGILL on macOS showing the > issue : > # Problematic frame: > # C [libzip.dylib+0x6362] hashN+0x32 > # > > Stack: [0x7c496000,0x7c596000], sp=0x7c5957e0, free > space=1021k > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native > code) > C [libzip.dylib+0x6362] hashN+0x32 > C [libzip.dylib+0x5d5e] readCEN+0xd2e > C [libzip.dylib+0x4ee0] ZIP_Put_In_Cache0+0x160 > V [libjvm.dylib+0x544b1e] ClassLoader::open_zip_file(char const*, char**, > JavaThread*)+0x3e > V [libjvm.dylib+0x543fec] ClassLoader::create_class_path_entry(JavaThread*, > char const*, stat const*, bool, bool)+0x6c > V [libjvm.dylib+0x543833] > ClassLoader::setup_bootstrap_search_path_impl(JavaThread*, char const*)+0xf3 > V [libjvm.dylib+0x54819b] classLoader_init1()+0x1b > V [libjvm.dylib+0x92602a] init_globals()+0x3a > V [libjvm.dylib+0x12b3b74] Threads::create_vm(JavaVMInitArgs*, bool*)+0x314 > V [libjvm.dylib+0xa848f4] JNI_CreateJavaVM+0x64 > C [libjli.dylib+0x4483] JavaMain+0x123 > C [libjli.dylib+0x7529] ThreadJavaMain+0x9 > C [libsystem_pthread.dylib+0x68fc] _pthread_start+0xe0 > C [libsystem_pthread.dylib+0x2443] thread_start+0xf Hi Lutz and Martin, thanks for the reviews! Jaikiran, are you done with your additional tests ? - PR Comment: https://git.openjdk.org/jdk/pull/18908#issuecomment-2077026022
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
On Tue, 23 Apr 2024 14:31:44 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust output I changed the added output to 'JRE path' to have more consistency with the existing JLI trace messages. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2075012914
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust output - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/8b7a9d58..4d998244 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=03-04 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output I adjusted the output ot 'JRE path' . > The GetJREPath function has no business looking for jre/lib as that location > does not exist since JDK 9. Is the at/below `/* Does the app ship a private JRE in /jre directory? */` part meant? This looks indeed obsolete . - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072472623
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Mon, 22 Apr 2024 17:19:14 GMT, Magnus Ihse Bursie wrote: > But that sounds like a very special case. Not sure if it is really such a special case. Currently both modes (getting the image path from launcher binary path , and getting the image path from 'the dll' / GetApplicationHomeFromDll (libjli.so on Linux) exist and are to my knowledge supported. At least one jtreg test fails when the GetApplicationHomeFromDll does not work any more. So I think it is natural that both modes are also supported/augmented by some tracing. We could probably reduce the trace message(s) added to just one if this is desired . - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072090837
RFR: 8330615: avoid signed integer overflows in zip_util.c readCen / hashN
In the hashN usages of readCen from zip_util.c we see a lot of signed integer overflows. For example in the java/util jtreg tests those are easily reproducable when compiling with -ftrapv (clang/gcc toolchains). While those overflows never seem to cause crashes or similar errors, they are unwanted because signed integer overflows in C cause undefined behavior. See https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Signed-Overflow.html > > For signed integers, the result of overflow in C is in principle undefined, > meaning that anything whatsoever could happen. > Therefore, C compilers can do optimizations that treat the overflow case with > total unconcern. So we might still get unwanted results (maybe bad/strange hashing, depending on compiler and optimization level). Compilation with -ftrapv causes/triggers this SIGILL on macOS showing the issue : # Problematic frame: # C [libzip.dylib+0x6362] hashN+0x32 # Stack: [0x7c496000,0x7c596000], sp=0x7c5957e0, free space=1021k Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) C [libzip.dylib+0x6362] hashN+0x32 C [libzip.dylib+0x5d5e] readCEN+0xd2e C [libzip.dylib+0x4ee0] ZIP_Put_In_Cache0+0x160 V [libjvm.dylib+0x544b1e] ClassLoader::open_zip_file(char const*, char**, JavaThread*)+0x3e V [libjvm.dylib+0x543fec] ClassLoader::create_class_path_entry(JavaThread*, char const*, stat const*, bool, bool)+0x6c V [libjvm.dylib+0x543833] ClassLoader::setup_bootstrap_search_path_impl(JavaThread*, char const*)+0xf3 V [libjvm.dylib+0x54819b] classLoader_init1()+0x1b V [libjvm.dylib+0x92602a] init_globals()+0x3a V [libjvm.dylib+0x12b3b74] Threads::create_vm(JavaVMInitArgs*, bool*)+0x314 V [libjvm.dylib+0xa848f4] JNI_CreateJavaVM+0x64 C [libjli.dylib+0x4483] JavaMain+0x123 C [libjli.dylib+0x7529] ThreadJavaMain+0x9 C [libsystem_pthread.dylib+0x68fc] _pthread_start+0xe0 C [libsystem_pthread.dylib+0x2443] thread_start+0xf - Commit messages: - JDK-8330615 Changes: https://git.openjdk.org/jdk/pull/18908/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18908=00 Issue: https://bugs.openjdk.org/browse/JDK-8330615 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18908.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18908/head:pull/18908 PR: https://git.openjdk.org/jdk/pull/18908
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output Hi, any additional comments / reviews ? Thanks Matthias - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2069158463
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
On Fri, 19 Apr 2024 09:20:27 GMT, Christoph Langer wrote: > This trace seems a bit unsymmetric to its Windows counterpart. Maybe it > should be left out here, too, since there is tracing in GetJREPath. I removed the JLI trace output for the launcher exe path. - PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1572140833
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove launcher executable path trace output - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/ddb3e0fe..8b7a9d58 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=02-03 Stats: 4 lines in 2 files changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
On Thu, 18 Apr 2024 06:31:03 GMT, Alan Bateman wrote: >> Hi Christoph, seems the USE_REGISTRY_LOOKUP is currently unused (at least >> without additional defines that are not present usually) >> >> src/java.base/windows/native/libjli/java_md.c:52:#ifdef USE_REGISTRY_LOOKUP >> src/java.base/windows/native/libjli/java_md.c:333:#ifdef USE_REGISTRY_LOOKUP >> >> >> I am not sure if this even works any more. Maybe Alan could comment on this >> ? > > @MBaesken I checked into the building with -DUSE_REGISTRY_LOOKUP as that > compiled in code that the Oracle installer needed when it copied java.exe to > somewhere. This is indeed left over code and can be removed from java_md.c. Hi Alan, thanks for checking this ! I removed the USE_REGISTRY_LOOKUP usages. - PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1570109527
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove obsolete USE_REGISTRY_LOOKUP usages - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/344d1b89..ddb3e0fe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=01-02 Stats: 12 lines in 1 file changed: 0 ins; 12 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v2]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust trace messages - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/128300a0..344d1b89 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=00-01 Stats: 10 lines in 3 files changed: 0 ins; 4 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken wrote:
> We have already good JLI tracing capabilities. But GetApplicationHome and
> GetApplicationHomeFromDll lack some tracing and should be enhanced.
I adjusted the trace messages a bit to make the coding more consistent to the
existing Jli trace code,
I removed the print of the function name because this is usually avoided in the
existing cases.
I removed the 'did not succeed' Jli trace messages in GetJREPath because in
case of success we would getJli trace output ('JRE path is ') so
missing success output means it failed.
-
PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2061561008
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 10:20:23 GMT, Alan Bateman wrote: > I think this is way too ad hoc and looks like lefts over from a debugging > session. So I don't think it should be integrated without stepping back and > thinking more about what this tracing option is intended for. Currently there seem to be two (with the unused registry stuff 3, if it is still valid ?) approaches to find the 'JRE' location (should this better be named image location?) - using the launcher exe location and using the location of the libjli shared lib. Unfortunately those approaches are not well supported by JLI-traces , this is what the change is about. We could probably adjust/reduce the added tracing code but I would like to know a bit more about the concerns raised. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2059252352
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 09:09:00 GMT, Christoph Langer wrote:
>> We have already good JLI tracing capabilities. But GetApplicationHome and
>> GetApplicationHomeFromDll lack some tracing and should be enhanced.
>
> src/java.base/windows/native/libjli/java_md.c line 326:
>
>> 324: }
>> 325:
>> 326: JLI_TraceLauncher("GetJREPath - attempt to get JRE location from
>> shared lib of the image\n");
>
> Maybe add a trace also in the USE_REGISTRY_LOOKUP section
Hi Christoph, seems the USE_REGISTRY_LOOKUP is currently unused (at least
without additional defines that are not present usually)
src/java.base/windows/native/libjli/java_md.c:52:#ifdef USE_REGISTRY_LOOKUP
src/java.base/windows/native/libjli/java_md.c:333:#ifdef USE_REGISTRY_LOOKUP
I am not sure if this even works any more. Maybe Alan could comment on this ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1567466784
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 09:14:50 GMT, Christoph Langer wrote: > > What exactly do you see as inconsistent ? > > Maybe the output of the tracing should look similar to other traces done > through `JLI_TraceLauncher`? E.g. not mention method names but just tell what > the program is doing... ? Okay, makes sense ! I will change that to make the output similar to existing traces. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2058627174
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Wed, 10 Apr 2024 07:16:49 GMT, Matthias Baesken wrote: > If we expand the tracing then I think it should be consistent with the > existing tracing. What exactly do you see as inconsistent ? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2056772309
Re: RFR: JDK-8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 9 Apr 2024 17:26:46 GMT, Alan Bateman wrote: > If we expand the tracing then I think it should be consistent with the > existing tracing. I just added some tracing calls where I missed them. Do you think I should do some more adjustments ? Btw. is the `GetApplicationHomeFromDll` (which in fact is getting the image location from the jli shared lib location) approach for the launcher documented somewhere or is this something that 'just works' for a log time ? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2046694106
Re: RFR: JDK-8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken wrote: > We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Btw. I noticed that GetModuleFileName return value is not checked, should this be done (at other locations in the JDk codebase it is done) ? See https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-getmodulefilenamea - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2045514750
RFR: JDK-8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
We have already good JLI tracing capabilities. But GetApplicationHome and GetApplicationHomeFromDll lack some tracing and should be enhanced. - Commit messages: - JDK-8329862 Changes: https://git.openjdk.org/jdk/pull/18699/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18699=00 Issue: https://bugs.openjdk.org/browse/JDK-8329862 Stats: 18 lines in 3 files changed: 15 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Integrated: JDK-8329074: AIX build fails after JDK-8328824
On Tue, 26 Mar 2024 09:21:20 GMT, Matthias Baesken wrote:
> After [JDK-8328824](https://bugs.openjdk.org/browse/JDK-8328824), we run in
> the AIX build into this failure :
>
> /opt/freeware/bin/bash: -c: line 1: syntax error near unexpected token `('
> gmake[3]: *** [lib/CoreLibraries.gmk:194:
> /openjdk/nb/aix_ppc64/jdk-dev-opt/support/native/java.base/libjli_static/args.o]
> Error 2
> gmake[3]: *** Waiting for unfinished jobs
>
> Looks like an addprefix usage is wrong, a '$' is missing.
This pull request has now been integrated.
Changeset: b9c76ded
Author:Matthias Baesken
URL:
https://git.openjdk.org/jdk/commit/b9c76dedf4aa2248a5e561a535c9e3e181f7836a
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
8329074: AIX build fails after JDK-8328824
Reviewed-by: clanger, goetz
-
PR: https://git.openjdk.org/jdk/pull/18484
Re: RFR: JDK-8329074: AIX build fails after JDK-8328824
On Tue, 26 Mar 2024 09:21:20 GMT, Matthias Baesken wrote:
> After [JDK-8328824](https://bugs.openjdk.org/browse/JDK-8328824), we run in
> the AIX build into this failure :
>
> /opt/freeware/bin/bash: -c: line 1: syntax error near unexpected token `('
> gmake[3]: *** [lib/CoreLibraries.gmk:194:
> /openjdk/nb/aix_ppc64/jdk-dev-opt/support/native/java.base/libjli_static/args.o]
> Error 2
> gmake[3]: *** Waiting for unfinished jobs
>
> Looks like an addprefix usage is wrong, a '$' is missing.
windows aarch64 failure is unrelated, fails with some download
[build.sh][INFO] Downloading
https://archive.apache.org/dist/ant/binaries/apache-ant-1.10.8-bin.zip to
/d/a/jdk/jdk/jtreg/src/make/../build/deps/apache-ant-1.10.8-bin.zip
Error: sh][ERROR] wget exited with exit code 4
Thanks for the reviews !
-
PR Comment: https://git.openjdk.org/jdk/pull/18484#issuecomment-2019994055
PR Comment: https://git.openjdk.org/jdk/pull/18484#issuecomment-2019997165
RFR: JDK-8329074: AIX build fails after JDK-8328824
After [JDK-8328824](https://bugs.openjdk.org/browse/JDK-8328824), we run in the
AIX build into this failure :
/opt/freeware/bin/bash: -c: line 1: syntax error near unexpected token `('
gmake[3]: *** [lib/CoreLibraries.gmk:194:
/openjdk/nb/aix_ppc64/jdk-dev-opt/support/native/java.base/libjli_static/args.o]
Error 2
gmake[3]: *** Waiting for unfinished jobs
Looks like an addprefix usage is wrong, a '$' is missing.
-
Commit messages:
- JDK-8329074
Changes: https://git.openjdk.org/jdk/pull/18484/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=18484=00
Issue: https://bugs.openjdk.org/browse/JDK-8329074
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/18484.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18484/head:pull/18484
PR: https://git.openjdk.org/jdk/pull/18484
Withdrawn: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/17625
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. With some rebalancing/adjustments to our test landscape the issues are gone. Unfortunately there was not much interest in the resource related discussion on jtreg-dev https://mail.openjdk.org/pipermail/jtreg-dev/2024-February/001926.html closing for now because the issues are currently not seen any more on our side. - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1996885622 PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1996886625
Re: RFR: 8327701: Remove the xlc toolchain [v4]
On Tue, 12 Mar 2024 15:27:29 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Replace CC_VERSION_OUTPUT with CC_VERSION_STRING > - We need CC_VERSION_OUTPUT before we can check it Seems `HOTSPOT_TOOLCHAIN_TYPE=xlc ` and `TARGET_COMPILER_xlc` macros stay, is this intended ? (not saying this is a wrong thing to do, but I believed first that all the xlc toolchain references are replaced by clang?) - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1993844670
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Tue, 12 Mar 2024 16:02:41 GMT, Matthias Baesken wrote: > > Please re-test. > > Hi Magnus, thanks for the adjustments. I reenabled your patch in our > build/test queue . Builds and test results on AIX (product and fastdebug) are fine with the latest version of the PR . - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1993808324
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Tue, 12 Mar 2024 15:24:21 GMT, Magnus Ihse Bursie wrote: > Please re-test. Hi Magnus, thanks for the adjustments. I reenabled your patch in our build/test queue . - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1992009593
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes With this change added, currently configure fails checking for ibm-llvm-cxxfilt... /opt/IBM/openxlC/17.1.1/tools/ibm-llvm-cxxfilt configure: error: ibm-clang_r version output check failed, output: configure exiting with result code maybe related to what Joachim pointed out ? - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1988358518
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes Hi, thanks for doing this cleanup change. I put it into our build/test queue to see the results on AIX. - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1987807579
Integrated: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase
On Wed, 6 Mar 2024 09:26:25 GMT, Matthias Baesken wrote: > We define the RESTARTABLE macro again and again at a lot of places in the JDK > native codebase. This could be centralized to avoid repeating it again and > again ! This pull request has now been integrated. Changeset: fb4610e6 Author: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/fb4610e6b7a339d0a95a99d6e113e3ddda291561 Stats: 185 lines in 18 files changed: 69 ins; 106 del; 10 mod 8327444: simplify RESTARTABLE macro usage in JDK codebase Reviewed-by: gli, clanger, alanb, dholmes, bpb - PR: https://git.openjdk.org/jdk/pull/18132
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v3]
On Thu, 7 Mar 2024 08:08:45 GMT, Alan Bateman wrote: > Latest version looks good to me. As have been pointed out, there at least 2 > files where the copyright header was updated but there are no changes, I > assume left over from previous iterations. I assume the RESTARTABLE-close in > libattach/VirtualMachineImpl.c will be tracked as a separate issue. Yes this was from the commit iterations. The RESTARTABLE-close issue will be tracked here https://bugs.openjdk.org/browse/JDK-8327468 8327468: Do not restart close if errno is EINTR [macOS/linux] Thanks for the review! - PR Comment: https://git.openjdk.org/jdk/pull/18132#issuecomment-1982996784
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v4]
On Wed, 6 Mar 2024 17:16:03 GMT, Christoph Langer wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> adjust COPYRIGHT year info > > src/java.base/unix/native/libjava/childproc.h line 75: > >> 73: #define FAIL_FILENO (STDERR_FILENO + 1) >> 74: >> 75: /* TODO: Refactor. */ > > Copyright year update missing. Thanks, I updated this too. - PR Review Comment: https://git.openjdk.org/jdk/pull/18132#discussion_r1515719859
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v4]
> We define the RESTARTABLE macro again and again at a lot of places in the JDK > native codebase. This could be centralized to avoid repeating it again and > again ! Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust COPYRIGHT year info - Changes: - all: https://git.openjdk.org/jdk/pull/18132/files - new: https://git.openjdk.org/jdk/pull/18132/files/2930075d..25a65a71 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18132=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18132=02-03 Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/18132.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18132/head:pull/18132 PR: https://git.openjdk.org/jdk/pull/18132
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v3]
On Wed, 6 Mar 2024 17:14:25 GMT, Christoph Langer wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Introduce windows jni_util_md.h > > src/java.base/share/native/libjava/jni_util.h line 30: > >> 28: >> 29: #include "jni.h" >> 30: #include "jni_util_md.h" > > This file needs copyright year update Agree, updated ! - PR Review Comment: https://git.openjdk.org/jdk/pull/18132#discussion_r1515719458
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v3]
> We define the RESTARTABLE macro again and again at a lot of places in the JDK > native codebase. This could be centralized to avoid repeating it again and > again ! Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Introduce windows jni_util_md.h - Changes: - all: https://git.openjdk.org/jdk/pull/18132/files - new: https://git.openjdk.org/jdk/pull/18132/files/f30a189d..2930075d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18132=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18132=01-02 Stats: 24 lines in 19 files changed: 1 ins; 22 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18132.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18132/head:pull/18132 PR: https://git.openjdk.org/jdk/pull/18132
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v2]
On Wed, 6 Mar 2024 15:49:05 GMT, Alan Bateman wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> introduce unix jni_util_md.h > > src/java.base/aix/native/libnio/ch/Pollset.c line 29: > >> 27: #include "jni.h" >> 28: #include "jni_util.h" >> 29: #include "jni_util_md.h" > > When I suggested jni_util_md.h then I meant create one for Unix and another > for Windows, then have jni_util.h include it. This will avoid needing to add > an include to all these files. I considered this too, but the one on Windows would be empty for now -> looks a bit strange . But I can do it why not . - PR Review Comment: https://git.openjdk.org/jdk/pull/18132#discussion_r1514752727
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v2]
> We define the RESTARTABLE macro again and again at a lot of places in the JDK > native codebase. This could be centralized to avoid repeating it again and > again ! Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: introduce unix jni_util_md.h - Changes: - all: https://git.openjdk.org/jdk/pull/18132/files - new: https://git.openjdk.org/jdk/pull/18132/files/014ab1fd..f30a189d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18132=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18132=00-01 Stats: 68 lines in 19 files changed: 52 ins; 7 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/18132.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18132/head:pull/18132 PR: https://git.openjdk.org/jdk/pull/18132
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase
On Wed, 6 Mar 2024 14:13:26 GMT, Alan Bateman wrote: >> We define the RESTARTABLE macro again and again at a lot of places in the >> JDK native codebase. This could be centralized to avoid repeating it again >> and again ! > > src/java.base/share/native/libjava/jni_util.h line 243: > >> 241: } while((_result == -1) && (errno == EINTR)); \ >> 242: } while(0) >> 243: > > jni_util.h is for all platforms so not the right place for a Unix specific > macro. We need think where the right place for this, might have to introduce > a jni_util_md.h. We could maybe also use the existing https://github.com/openjdk/jdk/blob/master/src/java.base/unix/native/include/jni_md.h - what do you think ? - PR Review Comment: https://git.openjdk.org/jdk/pull/18132#discussion_r1514574059
RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase
We define the RESTARTABLE macro again and again at a lot of places in the JDK native codebase. This could be centralized to avoid repeating it again and again ! - Commit messages: - JDK-8327444 Changes: https://git.openjdk.org/jdk/pull/18132/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18132=00 Issue: https://bugs.openjdk.org/browse/JDK-8327444 Stats: 113 lines in 16 files changed: 8 ins; 105 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18132.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18132/head:pull/18132 PR: https://git.openjdk.org/jdk/pull/18132
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v3]
On Mon, 26 Feb 2024 06:57:49 GMT, Jaikiran Pai wrote: > Hello David, the updated text that I proposed to Matthias, of the form > "expected: ... but was: ..." was borrowed from what junit5 Unfortunately we get now error messages like this java.lang.RuntimeException: VM output should contain exactly one rtm locking statistics entry for method compiler.testlibrary.rtm.XAbortProvoker::forceAbort expected: 0 but was: 1 It should be ... expected: 1 but was: 0 ; the assertEquals has this interface `assertEquals(Object lhs, Object rhs, String msg)` so we have a left hand (lhs) and a right hand side (rhs) of a comparison but was is expected and what is what we got is not defined . - PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1973131407
Re: RFR: 8326591: New test JmodExcludedFiles.java fails on Windows when --with-external-symbols-in-bundles=public is used
On Fri, 23 Feb 2024 19:18:46 GMT, Christoph Langer wrote: > The new test JmodExcludedFiles.java checks that no debug symbol files are > contained in the jmod files. It does not take into account that with > configure option --with-external-symbols-in-bundles=public we want to have > stripped pdb files, also in jmods, to get native callstacks with line number. > > This modifies the test and checks if equivalent *stripped.pdb files exist > when .pdb files are encountered inside jmods. In that case one can assume > that --with-external-symbols-in-bundles=public was chosen as configure option. Marked as reviewed by mbaesken (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17990#pullrequestreview-1910541971
Re: RFR: 8326591: New test JmodExcludedFiles.java fails on Windows when --with-external-symbols-in-bundles=public is used
On Fri, 23 Feb 2024 19:18:46 GMT, Christoph Langer wrote: > The new test JmodExcludedFiles.java checks that no debug symbol files are > contained in the jmod files. It does not take into account that with > configure option --with-external-symbols-in-bundles=public we want to have > stripped pdb files, also in jmods, to get native callstacks with line number. > > This modifies the test and checks if equivalent *stripped.pdb files exist > when .pdb files are encountered inside jmods. In that case one can assume > that --with-external-symbols-in-bundles=public was chosen as configure option. Looks okay to me, but could we print here `RuntimeException(jmodFile + " is expected not to include native debug symbols` not only the jmod but also the unwanted file(s) ? - PR Review: https://git.openjdk.org/jdk/pull/17990#pullrequestreview-1906139346
Integrated: JDK-8326389: [test] improve assertEquals failure output
On Wed, 21 Feb 2024 15:25:36 GMT, Matthias Baesken wrote: > Currently assertEquals has in the failure case sometimes confusing output > like : > > java.lang.RuntimeException: VM output should contain exactly one RTM locking > statistics entry for method > compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >at jdk.test.lib.Asserts.fail(Asserts.java:634) >at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) > > (I don't think we really expected that for some reason 0 equals 1) > This should be improved. This pull request has now been integrated. Changeset: 9b1f1e52 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/9b1f1e5294e130ec444b08af1f73d08e86fd91ee Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod 8326389: [test] improve assertEquals failure output Reviewed-by: clanger, jpai - PR: https://git.openjdk.org/jdk/pull/17952
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v4]
On Tue, 27 Feb 2024 16:48:05 GMT, Matthias Baesken wrote: >> Currently assertEquals has in the failure case sometimes confusing output >> like : >> >> java.lang.RuntimeException: VM output should contain exactly one RTM locking >> statistics entry for method >> compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >>at jdk.test.lib.Asserts.fail(Asserts.java:634) >>at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) >> >> (I don't think we really expected that for some reason 0 equals 1) >> This should be improved. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust ms Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1968478987
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v3]
On Mon, 26 Feb 2024 01:17:23 GMT, Jaikiran Pai wrote: > Hello Christoph, yes that looks fine to me. I adjusted it to the given suggestion. - PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1967070481
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v4]
> Currently assertEquals has in the failure case sometimes confusing output > like : > > java.lang.RuntimeException: VM output should contain exactly one RTM locking > statistics entry for method > compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >at jdk.test.lib.Asserts.fail(Asserts.java:634) >at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) > > (I don't think we really expected that for some reason 0 equals 1) > This should be improved. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust ms - Changes: - all: https://git.openjdk.org/jdk/pull/17952/files - new: https://git.openjdk.org/jdk/pull/17952/files/03cf3a82..a89742ff Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17952=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17952=02-03 Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17952.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17952/head:pull/17952 PR: https://git.openjdk.org/jdk/pull/17952
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v3]
On Thu, 22 Feb 2024 16:01:19 GMT, Matthias Baesken wrote: >> Currently assertEquals has in the failure case sometimes confusing output >> like : >> >> java.lang.RuntimeException: VM output should contain exactly one RTM locking >> statistics entry for method >> compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >>at jdk.test.lib.Asserts.fail(Asserts.java:634) >>at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) >> >> (I don't think we really expected that for some reason 0 equals 1) >> This should be improved. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > assertEquals adjust output Hi Christoph, thanks for the review ! Any other opinions/comments/reviews ? - PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1961045316
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v3]
On Thu, 22 Feb 2024 16:01:19 GMT, Matthias Baesken wrote: >> Currently assertEquals has in the failure case sometimes confusing output >> like : >> >> java.lang.RuntimeException: VM output should contain exactly one RTM locking >> statistics entry for method >> compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >>at jdk.test.lib.Asserts.fail(Asserts.java:634) >>at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) >> >> (I don't think we really expected that for some reason 0 equals 1) >> This should be improved. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > assertEquals adjust output now the error output is like java.lang.RuntimeException: VM output should contain two rtm locking statistics entries for method compiler.testlibrary.rtm.XAbortProvoker::forceAbort: object "0" is not equal to "2" looks much better to me. - PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1960895518
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v3]
> Currently assertEquals has in the failure case sometimes confusing output > like : > > java.lang.RuntimeException: VM output should contain exactly one RTM locking > statistics entry for method > compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >at jdk.test.lib.Asserts.fail(Asserts.java:634) >at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) > > (I don't think we really expected that for some reason 0 equals 1) > This should be improved. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: assertEquals adjust output - Changes: - all: https://git.openjdk.org/jdk/pull/17952/files - new: https://git.openjdk.org/jdk/pull/17952/files/09989d25..03cf3a82 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17952=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17952=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17952.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17952/head:pull/17952 PR: https://git.openjdk.org/jdk/pull/17952
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v2]
On Thu, 22 Feb 2024 15:23:56 GMT, Christoph Langer wrote: > I think it is a good idea to improve this. I was irritated by that output > more than once. > > Maybe a better message would be ... _"..." is not equal to "..."_ ? Thanks , I adjusted the output . - PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1959752228
Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v2]
> Currently assertEquals has in the failure case sometimes confusing output > like : > > java.lang.RuntimeException: VM output should contain exactly one RTM locking > statistics entry for method > compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 >at jdk.test.lib.Asserts.fail(Asserts.java:634) >at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) > > (I don't think we really expected that for some reason 0 equals 1) > This should be improved. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust COPYRIGHT year info - Changes: - all: https://git.openjdk.org/jdk/pull/17952/files - new: https://git.openjdk.org/jdk/pull/17952/files/d7b4de35..09989d25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17952=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17952=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17952.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17952/head:pull/17952 PR: https://git.openjdk.org/jdk/pull/17952
RFR: JDK-8326389: [test] improve assertEquals failure output
Currently assertEquals has in the failure case sometimes confusing output like : java.lang.RuntimeException: VM output should contain exactly one RTM locking statistics entry for method compiler.rtm.locking.TestRTMTotalCountIncrRate$Test: expected 0 to equal 1 at jdk.test.lib.Asserts.fail(Asserts.java:634) at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) (I don't think we really expected that for some reason 0 equals 1) This should be improved. - Commit messages: - JDK-8326389 Changes: https://git.openjdk.org/jdk/pull/17952/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17952=00 Issue: https://bugs.openjdk.org/browse/JDK-8326389 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17952.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17952/head:pull/17952 PR: https://git.openjdk.org/jdk/pull/17952
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. Hi [~[jaikiran] the exclude and match files sound promising, this could be helpful to achieve what we need/want . - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1954373348
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. It happens on various machines, two for example Windows Server 2022 Standard 16 cores 32G RAM Windows Server 2019 Standard 16 cores 32G RAM On both machines we run :tier1 -avm with -conc:15 (concurrency jtreg flag) . > The other unanswered question is - why is this happening now? I filed the issue this year but there are a couple of occurrences also from last year. I find also similar older failures from 2022 of java/lang/StringBuilder/HugeCapacity.java because of resource shortages (but those did not generate a hserr file for some reasons just some text output). So the issue is there for months already (maybe years?) . - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1952490909
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Wed, 31 Jan 2024 08:13:25 GMT, Matthias Baesken wrote: > Can we maybe see if we can fix these tests without exclusive-accessing them? > I find it surprising that `java/lang/StringBuilder` tests are problematic, > but `java/lang/StringBuffer` tests are not. Which tests fail? What do you think about marking jtreg tests with higher memory requirements with a jtreg key like highmemusage ? This way we do not need to put these tests into the _exclusiveAccess.dirs_ group, but get a way (only if needed) to execute those with high memory usage separately e.g. with lower concurrency. - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1951934706
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. I started a discussion on jtreg-dev https://mail.openjdk.org/pipermail/jtreg-dev/2024-February/001926.html but not much response so far. Adding a jtreg test key for tests with higher memory requirement (like HugeCapacity) would probably help to solve these resource issues . - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1943742596
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Mon, 12 Feb 2024 10:47:56 GMT, Jaikiran Pai wrote: > What seems to be happening is that the system where this run appears to be > launching too many tests concurrently. Sure, that's why I want to limit the concurrency *for certain tests/ test groups* . Limiting it for the whole tier1 would slow down tests that are absolutely fine with the concurrency we set. - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1940741219
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Tue, 6 Feb 2024 08:05:14 GMT, Matthias Baesken wrote: >>> I hope finally the AIX part of this PR is done. >> >> Thanks for the AIX related effort ; I put it again into our internal >> build/test queue. > >> >> Thanks for the AIX related effort ; I put it again into our internal >> build/test queue. > > With the latest commit the build again fails on AIX with this error > > /jdk/src/java.base/unix/native/libnio/ch/UnixFileDispatcherImpl.c:381:27: > error: incompatible pointer types passing 'struct statvfs64 *' to parameter > of type 'struct statvfs *' [-Werror,-Wincompatible-pointer-types] > result = fstatvfs(fd, _stat); > ^~ > /usr/include/sys/statvfs.h:102:42: note: passing argument to parameter here > extern int fstatvfs(int, struct statvfs *); > Well, well... The code at least looks cleaner without these AIX defines, so I > really hope that this is the end of the AIX saga, at the `n+1`th time. > @MBaesken Can you rerun AIX testing with the latest commit? Latest commit looks still good on AIX. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1933652124
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. Hello, any further comments on this ? Or should we carry the discussion to jtreg on how to work with resource (in this case memory) issues in case of concurrent runs ? Can we **_configure_** to execute some jtreg tests with higher mem requirements with less concurrency ? - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1933560174
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v9]
On Tue, 6 Feb 2024 08:18:14 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Also fix fstatvfs on AIX AIX build is fixed now after latest commit, thanks for handling the AIX special cases. - Marked as reviewed by mbaesken (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17538#pullrequestreview-1865270571
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Mon, 5 Feb 2024 14:15:44 GMT, Matthias Baesken wrote: > > Thanks for the AIX related effort ; I put it again into our internal > build/test queue. With the latest commit the build again fails on AIX with this error /jdk/src/java.base/unix/native/libnio/ch/UnixFileDispatcherImpl.c:381:27: error: incompatible pointer types passing 'struct statvfs64 *' to parameter of type 'struct statvfs *' [-Werror,-Wincompatible-pointer-types] result = fstatvfs(fd, _stat); ^~ /usr/include/sys/statvfs.h:102:42: note: passing argument to parameter here extern int fstatvfs(int, struct statvfs *); - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1928972961
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in > the exclusiveAccess.dirs group so that they are not executed concurrently, > which helps to mitigate the resource shortages. > Of course this has the downside that on very large machines the concurrent > execution is not done any more. This is what the thread stack looks like in hs_err for example for java\lang\StringBuilder\Insert\hs_err_pid910208.log we had on Sun Jan 07 20:32:56 CET 2024 such an hs err file with thread stack : # # There is insufficient memory for the Java Runtime Environment to continue. # Native memory allocation (mmap) failed to map 536870912 bytes. Error detail: G1 virtual space # Possible reasons: # The system is out of physical RAM or swap space # This process is running with CompressedOops enabled, and the Java Heap may be blocking the growth of the native heap # Possible solutions: # Reduce memory load on the system # Increase physical memory or swap space # Check if swap backing store is full # Decrease Java heap size (-Xmx/-Xms) # Decrease number of Java threads # Decrease Java thread stack sizes (-Xss) # Set larger code cache with -XX:ReservedCodeCacheSize= # JVM is running with Unscaled Compressed Oops mode in which the Java heap is # placed in the first 4GB address space. The Java Heap base address is the # maximum limit for the native heap growth. Please use -XX:HeapBaseMinAddress # to set the Java Heap base and to place the Java Heap above 4GB virtual address. # This output file may be truncated or incomplete. # # Out of Memory Error (c:\openjdk-jdk-dev-windows_x86_64-dbg\jdk\src\hotspot\os\windows\os_windows.cpp:3627), pid=910208, tid=910648 # # JRE version: (23.0) (fastdebug build ) # Java VM: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.GLOBALsapmachine.jdk, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, windows-amd64) # CreateCoredumpOnCrash turned off, no core file dumped # ... --- T H R E A D --- Current thread (0x02178c5a44c0): JavaThread "Unknown thread" [_thread_in_vm, id=910648, stack(0x00eeec50,0x00eeec60) (1024K)] Stack: [0x00eeec50,0x00eeec60] Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) V [jvm.dll+0xc96581] os::win32::platform_print_native_stack+0x101 (os_windows_x86.cpp:236) V [jvm.dll+0xfe7b31] VMError::report+0x1491 (vmError.cpp:1005) V [jvm.dll+0xfea055] VMError::report_and_die+0x645 (vmError.cpp:1834) V [jvm.dll+0xfea7cf] VMError::report_and_die+0x5f (vmError.cpp:1604) V [jvm.dll+0x559d4f] report_vm_out_of_memory+0x5f (debug.cpp:225) V [jvm.dll+0xc91c5d] os::pd_commit_memory_or_exit+0xad (os_windows.cpp:3635) V [jvm.dll+0xc82a2e] os::commit_memory_or_exit+0x6e (os.cpp:2051) V [jvm.dll+0x6de800] G1PageBasedVirtualSpace::commit+0x100 (g1PageBasedVirtualSpace.cpp:192) V [jvm.dll+0x6f0aff] G1RegionsLargerThanCommitSizeMapper::commit_regions+0x7f (g1RegionToSpaceMapper.cpp:100) V [jvm.dll+0x7806da] HeapRegionManager::expand+0x8a (heapRegionManager.cpp:164) V [jvm.dll+0x780be6] HeapRegionManager::expand_by+0xf6 (heapRegionManager.cpp:361) V [jvm.dll+0x6812e4] G1CollectedHeap::expand+0xf4 (g1CollectedHeap.cpp:1014) V [jvm.dll+0x682dc6] G1CollectedHeap::initialize+0x596 (g1CollectedHeap.cpp:1389) V [jvm.dll+0xf823e0] universe_init+0x140 (universe.cpp:794) V [jvm.dll+0x79c8c1] init_globals+0x31 (init.cpp:126) V [jvm.dll+0xf5c20e] Threads::create_vm+0x2ae (threads.cpp:552) V [jvm.dll+0x8c17b2] JNI_CreateJavaVM_inner+0x82 (jni.cpp:3576) V [jvm.dll+0x8c5d9f] JNI_CreateJavaVM+0x1f (jni.cpp:3667) C [jli.dll+0x539f] JavaMain+0x113 (java.c:491) C [ucrtbase.dll+0x2268a] (no source info available) C [KERNEL32.DLL+0x17ac4] (no source info available) C [ntdll.dll+0x5a4e1] (no source info available) - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1927319309
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Mon, 5 Feb 2024 14:03:45 GMT, Magnus Ihse Bursie wrote: > I hope finally the AIX part of this PR is done. Thanks for the AIX related effort ; I put it again into our internal build/test queue. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1927105669
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Mon, 5 Feb 2024 12:38:03 GMT, Magnus Ihse Bursie wrote: > It seems like the statvfs64 is a pre-existing bug in AIX in that case. I have > not removed any statvfs64 for AIX; all such changes are guarded by `#ifdef > _ALLBSD_SOURCE`, which I presume is not defined on AIX. > > I recommend that I push this PR as-is first, and then you can do a follow-up > fix to define statvfs to statvfs64 on AIX. Java_sun_nio_fs_UnixNativeDispatcher_statvfs0 is changed from statvfs64 to statvfs, did I miss something ? - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1926925394
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Mon, 5 Feb 2024 12:17:33 GMT, Joachim Kern wrote: > Yes, if statvfs64() is replaced by statvfs() in the code, we will fallback on > AIX to 32-Bit. _LARGE_FILES really does not help in this case! Thanks for confirming. I think we do not want to fallback on AIX, so the <*>64 variant needs to stay on AIX. Seems the other symbols are covered by _LARGE_FILES according to the table in the linked IBM AIX doc (table in https://www.ibm.com/docs/en/aix/7.1?topic=volumes-writing-programs-that-access-large-files ) , is that correct (Joachim?) or did I miss something ? - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1926874075
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Fri, 2 Feb 2024 06:55:19 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add kludge to BufferedRenderPipe.c for AIX Current commit compiles nicely on AIX. One issue we might still have statvfs/statvfs64 is not mentioned here in the table of functions/structs redefined on AIX https://www.ibm.com/docs/en/aix/7.1?topic=volumes-writing-programs-that-access-large-files so would we fall back to statvfs from the *64 - variant ? The define _LARGE_FILES might not help in this case on AIX . - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1926848063
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Fri, 2 Feb 2024 06:55:19 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add kludge to BufferedRenderPipe.c for AIX I noticed that in the file src/java.base/share/native/libzip/zlib/zconf.h we seem to still use `off64_t` , is this okay (at most other locations it was replaced) https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libzip/zlib/zconf.h#L541 - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1926448109
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v4]
On Thu, 1 Feb 2024 13:47:45 GMT, Matthias Baesken wrote: >> After adding this additional patch I fully build fastdebug on AIX (hav to >> admit it does not look very nice). >> >> >> diff --git >> a/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c >> b/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c >> index 823475b0a23..ee0109b6806 100644 >> --- a/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c >> +++ b/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c >> @@ -31,6 +31,10 @@ >> #include "SpanIterator.h" >> #include "Trace.h" >> >> +#if defined(_AIX) && defined(open) >> +#undef open >> +#endif >> + >> /* The "header" consists of a jint opcode and a jint span count value */ >> #define INTS_PER_HEADER 2 >> #define BYTES_PER_HEADER 8 > >> @MBaesken So my fix in >> [25c691d](https://github.com/openjdk/jdk/pull/17538/commits/25c691df823eb9d9db1451637f28d59dd9508386) >> did not help? Maybe then it is some other system library that drags in >> `fcntl.h`; I assumed it was stdlib or stdio. That header file includes way >> too much that it does not need, so we can surely strip it of even more >> standard includes if that is what is required to fix this. > > > Unfortunately it did not help. > @MBaesken How annoying. :( I have now tried to remove _all_ system includes > from `debug_util.h`. Can you please try again building debug on AIX, to see > if it works without the `#undef` in `BufferedRenderPipe.c`? The AIX (fast)debug build still fails . - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1921645170
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v4]
On Wed, 31 Jan 2024 09:19:39 GMT, Matthias Baesken wrote: >> Magnus Ihse Bursie has updated the pull request with a new target base due >> to a merge or a rebase. The incremental webrev excludes the unrelated >> changes brought in by the merge/rebase. The pull request contains seven >> additional commits since the last revision: >> >> - Merge branch 'master' into jdk-FOB64 >> - Move #include out of debug_util.h >> - Restore AIX dirent64 et al defines >> - Rollback AIX changes since they are now tracked in JDK-8324834 >> - Remove superfluous setting of FOB64 >> - Replace all foo64() with foo() for large-file functions in the JDK >> - 8324539: Do not use LFS64 symbols in JDK libs > > After adding this additional patch I fully build fastdebug on AIX (hav to > admit it does not look very nice). > > > diff --git > a/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c > b/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c > index 823475b0a23..ee0109b6806 100644 > --- a/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c > +++ b/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c > @@ -31,6 +31,10 @@ > #include "SpanIterator.h" > #include "Trace.h" > > +#if defined(_AIX) && defined(open) > +#undef open > +#endif > + > /* The "header" consists of a jint opcode and a jint span count value */ > #define INTS_PER_HEADER 2 > #define BYTES_PER_HEADER 8 > @MBaesken So my fix in > [25c691d](https://github.com/openjdk/jdk/pull/17538/commits/25c691df823eb9d9db1451637f28d59dd9508386) > did not help? Maybe then it is some other system library that drags in > `fcntl.h`; I assumed it was stdlib or stdio. That header file includes way > too much that it does not need, so we can surely strip it of even more > standard includes if that is what is required to fix this. Unfortunately it did not help. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1921367368
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v4]
On Tue, 30 Jan 2024 14:15:57 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains seven additional > commits since the last revision: > > - Merge branch 'master' into jdk-FOB64 > - Move #include out of debug_util.h > - Restore AIX dirent64 et al defines > - Rollback AIX changes since they are now tracked in JDK-8324834 > - Remove superfluous setting of FOB64 > - Replace all foo64() with foo() for large-file functions in the JDK > - 8324539: Do not use LFS64 symbols in JDK libs After adding this additional patch I fully build fastdebug on AIX (hav to admit it does not look very nice). diff --git a/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c b/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c index 823475b0a23..ee0109b6806 100644 --- a/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c +++ b/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c @@ -31,6 +31,10 @@ #include "SpanIterator.h" #include "Trace.h" +#if defined(_AIX) && defined(open) +#undef open +#endif + /* The "header" consists of a jint opcode and a jint span count value */ #define INTS_PER_HEADER 2 #define BYTES_PER_HEADER 8 - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1918699480
Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On Wed, 31 Jan 2024 00:48:35 GMT, Joe Darcy wrote: > Can we maybe see if we can fix these tests without exclusive-accessing them? > I find it surprising that `java/lang/StringBuilder` tests are problematic, > but `java/lang/StringBuffer` tests are not. Which tests fail? It is a bit arbitrary which tests fail. one day : java/lang/StringBuilder/StringBufferRepeat.java java/lang/StringBuilder/CompactStringBuilderSerialization.java java/lang/StringBuilder/Insert.java other day: java/lang/StringBuilder/HugeCapacity.java next day it might differ a bit. Maybe it would be sufficient to execute only the HugeCapacity test in a non concurrent way because this one seems to be especially resource hungry, but I am not aware how this would work in jtreg (I can only set whole directories). Currently we run with this patch and the issues are gone. Is there a way to balance resource usage in jtreg runs? - PR Comment: https://git.openjdk.org/jdk/pull/17625#issuecomment-1918595685
RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs
On some Windows machines we see sometimes OOM errors because of high resource (memory/swap) consumption. This is especially seen when the jtreg runs have higher concurrency. A solution is to put the java/lang/StringBuilder tests in the exclusiveAccess.dirs group so that they are not executed concurrently, which helps to mitigate the resource shortages. Of course this has the downside that on very large machines the concurrent execution is not done any more. - Commit messages: - JDK-8324930 Changes: https://git.openjdk.org/jdk/pull/17625/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17625=00 Issue: https://bugs.openjdk.org/browse/JDK-8324930 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17625.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17625/head:pull/17625 PR: https://git.openjdk.org/jdk/pull/17625
Re: RFR: JDK-8324598: use mem_unit when working with sysinfo memory and swap related information [v2]
On Thu, 25 Jan 2024 09:15:38 GMT, Matthias Baesken wrote: >> According to the sysinfo manpage ( >> https://man7.org/linux/man-pages/man2/sysinfo.2.html ) the memory and swap >> related entries in the struct sysinfo are given as multiples of mem_unit >> bytes. >> "In the above structure, sizes of the memory and swap fields are given as >> multiples of mem_unit bytes." >> >> This is considered in most parts of the OpenJDK codebase when sysinfo is >> used but not all; should be added where it is missing. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > add parentheses Hi Martin, thanks for the review ! - PR Comment: https://git.openjdk.org/jdk/pull/17550#issuecomment-1909863447
Integrated: JDK-8324598: use mem_unit when working with sysinfo memory and swap related information
On Wed, 24 Jan 2024 10:07:17 GMT, Matthias Baesken wrote: > According to the sysinfo manpage ( > https://man7.org/linux/man-pages/man2/sysinfo.2.html ) the memory and swap > related entries in the struct sysinfo are given as multiples of mem_unit > bytes. > "In the above structure, sizes of the memory and swap fields are given as > multiples of mem_unit bytes." > > This is considered in most parts of the OpenJDK codebase when sysinfo is used > but not all; should be added where it is missing. This pull request has now been integrated. Changeset: 7a798d3c Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/7a798d3cebea0915f8a73af57333b3488c2091af Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod 8324598: use mem_unit when working with sysinfo memory and swap related information Reviewed-by: dholmes, mdoerr - PR: https://git.openjdk.org/jdk/pull/17550
Re: RFR: JDK-8324598: use mem_unit when working with sysinfo memory and swap related information [v2]
> According to the sysinfo manpage ( > https://man7.org/linux/man-pages/man2/sysinfo.2.html ) the memory and swap > related entries in the struct sysinfo are given as multiples of mem_unit > bytes. > "In the above structure, sizes of the memory and swap fields are given as > multiples of mem_unit bytes." > > This is considered in most parts of the OpenJDK codebase when sysinfo is used > but not all; should be added where it is missing. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: add parentheses - Changes: - all: https://git.openjdk.org/jdk/pull/17550/files - new: https://git.openjdk.org/jdk/pull/17550/files/c46b804f..77b2d44e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17550=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17550=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17550.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17550/head:pull/17550 PR: https://git.openjdk.org/jdk/pull/17550
Re: RFR: JDK-8324598: use mem_unit when working with sysinfo memory and swap related information
On Wed, 24 Jan 2024 10:07:17 GMT, Matthias Baesken wrote: > According to the sysinfo manpage ( > https://man7.org/linux/man-pages/man2/sysinfo.2.html ) the memory and swap > related entries in the struct sysinfo are given as multiples of mem_unit > bytes. > "In the above structure, sizes of the memory and swap fields are given as > multiples of mem_unit bytes." > > This is considered in most parts of the OpenJDK codebase when sysinfo is used > but not all; should be added where it is missing. Thanks for the review! I followed your suggestions regarding parenthesis. - PR Comment: https://git.openjdk.org/jdk/pull/17550#issuecomment-1909712920
RFR: JDK-8324598: use mem_unit when working with sysinfo memory and swap related information
According to the sysinfo manpage ( https://man7.org/linux/man-pages/man2/sysinfo.2.html ) the memory and swap related entries in the struct sysinfo are given as multiples of mem_unit bytes. "In the above structure, sizes of the memory and swap fields are given as multiples of mem_unit bytes." This is considered in most parts of the OpenJDK codebase when sysinfo is used but not all; should be added where it is missing. - Commit messages: - JDK-8324598 Changes: https://git.openjdk.org/jdk/pull/17550/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17550=00 Issue: https://bugs.openjdk.org/browse/JDK-8324598 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17550.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17550/head:pull/17550 PR: https://git.openjdk.org/jdk/pull/17550
Re: RFR: JDK-8322782: Clean up usages of unnecessary fully qualified class name "java.util.Arrays" [v3]
On Wed, 3 Jan 2024 13:55:22 GMT, Matthias Baesken wrote: >> In [JDK-8322772](https://bugs.openjdk.org/browse/JDK-8322772) one similar >> cleanup has been proposed before (and was done in the change). But there are >> a number of other places in the codebase where the import is done and still >> the unneeded fully qualified class name "java.util.Arrays" is used so more >> cleanups can be done. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Invokers.java too Hi Alexey, thanks for the review ! - PR Comment: https://git.openjdk.org/jdk/pull/17241#issuecomment-1876673066
Integrated: JDK-8322782: Clean up usages of unnecessary fully qualified class name "java.util.Arrays"
On Wed, 3 Jan 2024 11:41:20 GMT, Matthias Baesken wrote: > In [JDK-8322772](https://bugs.openjdk.org/browse/JDK-8322772) one similar > cleanup has been proposed before (and was done in the change). But there are > a number of other places in the codebase where the import is done and still > the unneeded fully qualified class name "java.util.Arrays" is used so more > cleanups can be done. This pull request has now been integrated. Changeset: 1369c545 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/1369c545ac51d7b5ff623d486e28c939869fecb8 Stats: 29 lines in 9 files changed: 0 ins; 0 del; 29 mod 8322782: Clean up usages of unnecessary fully qualified class name "java.util.Arrays" Reviewed-by: alanb, aivanov - PR: https://git.openjdk.org/jdk/pull/17241
Re: RFR: JDK-8322782: Clean up usages of unnecessary fully qualified class name "java.util.Arrays" [v3]
On Wed, 3 Jan 2024 13:55:22 GMT, Matthias Baesken wrote: >> In [JDK-8322772](https://bugs.openjdk.org/browse/JDK-8322772) one similar >> cleanup has been proposed before (and was done in the change). But there are >> a number of other places in the codebase where the import is done and still >> the unneeded fully qualified class name "java.util.Arrays" is used so more >> cleanups can be done. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Invokers.java too Hi Alan, thanks for the review ! I adjust the copyright year info and also some more places in Invokers.java . - PR Comment: https://git.openjdk.org/jdk/pull/17241#issuecomment-1875416193
Re: RFR: JDK-8322782: Clean up usages of unnecessary fully qualified class name "java.util.Arrays" [v3]
> In [JDK-8322772](https://bugs.openjdk.org/browse/JDK-8322772) one similar > cleanup has been proposed before (and was done in the change). But there are > a number of other places in the codebase where the import is done and still > the unneeded fully qualified class name "java.util.Arrays" is used so more > cleanups can be done. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust Invokers.java too - Changes: - all: https://git.openjdk.org/jdk/pull/17241/files - new: https://git.openjdk.org/jdk/pull/17241/files/c06f35b5..eac9ca69 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17241=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17241=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17241.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17241/head:pull/17241 PR: https://git.openjdk.org/jdk/pull/17241
