Withdrawn: 8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set
On Mon, 11 Mar 2024 11:57:04 GMT, Matthias Baesken wrote: > Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/18190
Integrated: 8335710: serviceability/dcmd/vm/SystemDumpMapTest.java and SystemMapTest.java fail on Linux Alpine after 8322475
On Mon, 8 Jul 2024 11:06:45 GMT, Matthias Baesken wrote:
> Unfortunately those 2 tests fail now on Linux Alpine (x86_64) :
> serviceability/dcmd/vm/SystemDumpMapTest.java
>
> Missing patterns in dump:
> 0x\\p{XDigit}+-0x\\p{XDigit}+ +\\d+ +[rwsxp-]+ +\\d+ +\\d+
> +(4K|8K|16K|64K|2M|16M|64M) +com.*\[vdso\]
> test SystemDumpMapTest.jmx(): failure
> java.lang.RuntimeException: java.lang.RuntimeException: Missing patterns
> at SystemDumpMapTest.run_test(SystemDumpMapTest.java:100)
> at SystemDumpMapTest.run(SystemDumpMapTest.java:106)
> at SystemDumpMapTest.jmx(SystemDumpMapTest.java:112)
> at
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at
> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
> at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
> at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
> at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
> at
> org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
> at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
> at
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
> at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
> at java.base/java.util.ArrayList.forEach(ArrayList.java:1597)
>
>
> Reason is that the vdso lib is not there on all Linux flavors; e.g. we do not
> seem to have it on our Alpine Linux test system.
> So better avoid the check because it is based on false assumptions.
This pull request has now been integrated.
Changeset: c703d290
Author:Matthias Baesken
URL:
https://git.openjdk.org/jdk/commit/c703d290425f85a06e61d72c9672ac2adac92db9
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
8335710: serviceability/dcmd/vm/SystemDumpMapTest.java and SystemMapTest.java
fail on Linux Alpine after 8322475
Reviewed-by: stuefe, lucy
-
PR: https://git.openjdk.org/jdk/pull/20072
Re: RFR: 8335710: serviceability/dcmd/vm/SystemDumpMapTest.java and SystemMapTest.java fail on Linux Alpine after 8322475
On Mon, 8 Jul 2024 11:06:45 GMT, Matthias Baesken wrote:
> Unfortunately those 2 tests fail now on Linux Alpine (x86_64) :
> serviceability/dcmd/vm/SystemDumpMapTest.java
>
> Missing patterns in dump:
> 0x\\p{XDigit}+-0x\\p{XDigit}+ +\\d+ +[rwsxp-]+ +\\d+ +\\d+
> +(4K|8K|16K|64K|2M|16M|64M) +com.*\[vdso\]
> test SystemDumpMapTest.jmx(): failure
> java.lang.RuntimeException: java.lang.RuntimeException: Missing patterns
> at SystemDumpMapTest.run_test(SystemDumpMapTest.java:100)
> at SystemDumpMapTest.run(SystemDumpMapTest.java:106)
> at SystemDumpMapTest.jmx(SystemDumpMapTest.java:112)
> at
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at
> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
> at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
> at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
> at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
> at
> org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
> at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
> at
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
> at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
> at java.base/java.util.ArrayList.forEach(ArrayList.java:1597)
>
>
> Reason is that the vdso lib is not there on all Linux flavors; e.g. we do not
> seem to have it on our Alpine Linux test system.
> So better avoid the check because it is based on false assumptions.
Hi Thomas, I added a check for the [heap] segment .
-
PR Comment: https://git.openjdk.org/jdk/pull/20072#issuecomment-804204
Re: RFR: 8335710: serviceability/dcmd/vm/SystemDumpMapTest.java and SystemMapTest.java fail on Linux Alpine after 8322475 [v2]
> Unfortunately those 2 tests fail now on Linux Alpine (x86_64) :
> serviceability/dcmd/vm/SystemDumpMapTest.java
>
> Missing patterns in dump:
> 0x\\p{XDigit}+-0x\\p{XDigit}+ +\\d+ +[rwsxp-]+ +\\d+ +\\d+
> +(4K|8K|16K|64K|2M|16M|64M) +com.*\[vdso\]
> test SystemDumpMapTest.jmx(): failure
> java.lang.RuntimeException: java.lang.RuntimeException: Missing patterns
> at SystemDumpMapTest.run_test(SystemDumpMapTest.java:100)
> at SystemDumpMapTest.run(SystemDumpMapTest.java:106)
> at SystemDumpMapTest.jmx(SystemDumpMapTest.java:112)
> at
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at
> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
> at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
> at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
> at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
> at
> org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
> at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
> at
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
> at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
> at java.base/java.util.ArrayList.forEach(ArrayList.java:1597)
>
>
> Reason is that the vdso lib is not there on all Linux flavors; e.g. we do not
> seem to have it on our Alpine Linux test system.
> So better avoid the check because it is based on false assumptions.
Matthias Baesken has updated the pull request incrementally with one additional
commit since the last revision:
test for heap segment
-
Changes:
- all: https://git.openjdk.org/jdk/pull/20072/files
- new: https://git.openjdk.org/jdk/pull/20072/files/3c9d1b9e..c379a443
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=20072=01
- incr: https://webrevs.openjdk.org/?repo=jdk=20072=00-01
Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/20072.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/20072/head:pull/20072
PR: https://git.openjdk.org/jdk/pull/20072
Re: RFR: 8335533: OutOfMemoryError: Metaspace observed again on AIX in test RedefineLeakThrowable.java after JDK-8294960
On Wed, 10 Jul 2024 09:14:11 GMT, Christoph Langer wrote: > The change of JDK-8294960 has brought an increase of required metaspace for > this test on AIX which seems to go beyond 23m in most cases. So I propose > another slight increment. > > Why AIX needs more metaspace compared to other platforms is probably a > different topic. LGTM ; I observed a very small increase on Linux x86_64 too but not enough to 'break' the metaspace test settings on this platform. - Marked as reviewed by mbaesken (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20106#pullrequestreview-2168788816
Re: RFR: 8335710: serviceability/dcmd/vm/SystemDumpMapTest.java and SystemMapTest.java fail on Linux Alpine after 8322475
On Tue, 9 Jul 2024 12:52:34 GMT, Thomas Stuefe wrote: > Checked and Alpine processes have a [heap] segment. Could you try that? Yes there is an entry [heap] in the file column of the lengthy table. Do you think this is present on all Linux platforms? If I replace the vdso with [heap] maybe it breaks elsewhere ? - PR Comment: https://git.openjdk.org/jdk/pull/20072#issuecomment-2219736851
Re: RFR: 8335882: platform/cgroup/TestSystemSettings.java fails on Alpine Linux
On Mon, 8 Jul 2024 14:19:21 GMT, Severin Gehwolf wrote: > Please review this simple test fix to exclude the test from being run on an > Alpine Linux system. Apparently default Alpine Linux systems don't have > cgroups set up by default the way other distros do. More info on the bug. I > propose to not run the test on musl systems. Marked as reviewed by mbaesken (Reviewer). The issue is gone in our tests, with your patch added. - PR Review: https://git.openjdk.org/jdk/pull/20076#pullrequestreview-2165630718 PR Comment: https://git.openjdk.org/jdk/pull/20076#issuecomment-2217120494
Re: RFR: 8335882: platform/cgroup/TestSystemSettings.java fails on Alpine Linux
On Mon, 8 Jul 2024 14:24:12 GMT, Severin Gehwolf wrote: > @MBaesken Since you've noticed the failing test in your CI could you please > verify the issue is gone with this? I don't have an Alpine Linux setup to > easily test this exclusion. Thanks! Hi Severin, sure ! I put it into our build/test setup . - PR Comment: https://git.openjdk.org/jdk/pull/20076#issuecomment-2214228730
RFR: 8335710: serviceability/dcmd/vm/SystemDumpMapTest.java and SystemMapTest.java fail on Linux Alpine after 8322475
Unfortunately those 2 tests fail now on Linux Alpine (x86_64) :
serviceability/dcmd/vm/SystemDumpMapTest.java
Missing patterns in dump:
0x\\p{XDigit}+-0x\\p{XDigit}+ +\\d+ +[rwsxp-]+ +\\d+ +\\d+
+(4K|8K|16K|64K|2M|16M|64M) +com.*\[vdso\]
test SystemDumpMapTest.jmx(): failure
java.lang.RuntimeException: java.lang.RuntimeException: Missing patterns
at SystemDumpMapTest.run_test(SystemDumpMapTest.java:100)
at SystemDumpMapTest.run(SystemDumpMapTest.java:106)
at SystemDumpMapTest.jmx(SystemDumpMapTest.java:112)
at
java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at
org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
at
org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
at
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1597)
Reason is that the vdso lib is not there on all Linux flavors; e.g. we do not
seem to have it on our Alpine Linux test system.
So better avoid the check because it is based on false assumptions.
-
Commit messages:
- JDK-8335710
Changes: https://git.openjdk.org/jdk/pull/20072/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=20072=00
Issue: https://bugs.openjdk.org/browse/JDK-8335710
Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/20072.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/20072/head:pull/20072
PR: https://git.openjdk.org/jdk/pull/20072
Re: RFR: 8333361: ubsan, test : libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null
On Thu, 20 Jun 2024 11:58:23 GMT, Matthias Baesken wrote: > The following issue has been observed when running > serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest (and some > other :tier1 tests) > on Linux with ubsan enabled binaries : > > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518:9: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x80388020 in event_storage_augment_storage > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518 > #1 0x80388020 in event_storage_add > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:557 > #2 0x85e695fc in JvmtiExport::post_sampled_object_alloc(JavaThread*, > oopDesc*) src/hotspot/share/prims/jvmtiExport.cpp:2926 > #3 0x85e558b8 in > JvmtiObjectAllocEventCollector::generate_call_for_allocated() > src/hotspot/share/prims/jvmtiExport.cpp:3074 > #4 0x85e56c14 in > JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() > src/hotspot/share/prims/jvmtiExport.cpp:3171 > #5 0x85e56c14 in > JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() > src/hotspot/share/prims/jvmtiExport.cpp:3166 > #6 0x862ace34 in > MemAllocator::Allocation::notify_allocation_jvmti_sampler() > src/hotspot/share/gc/shared/memAllocator.cpp:196 > #7 0x862af7a4 in MemAllocator::Allocation::~Allocation() > src/hotspot/share/gc/shared/memAllocator.cpp:87 > #8 0x862af7a4 in MemAllocator::allocate() const > src/hotspot/share/gc/shared/memAllocator.cpp:356 > #9 0x86dca4dc in CollectedHeap::array_allocate(Klass*, unsigned long, > int, bool, JavaThread*) > src/hotspot/share/gc/shared/collectedHeap.inline.hpp:41 > #10 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, > JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:93 > #11 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, > JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:89 > #12 0x857f35c8 in InterpreterRuntime::newarray(JavaThread*, > BasicType, int) src/hotspot/share/interpreter/interpreterRuntime.cpp:232 > #13 0x6b094cf4 () Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/19804#issuecomment-2182284335
Integrated: 8333361: ubsan,test : libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null
On Thu, 20 Jun 2024 11:58:23 GMT, Matthias Baesken wrote: > The following issue has been observed when running > serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest (and some > other :tier1 tests) > on Linux with ubsan enabled binaries : > > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518:9: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x80388020 in event_storage_augment_storage > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518 > #1 0x80388020 in event_storage_add > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:557 > #2 0x85e695fc in JvmtiExport::post_sampled_object_alloc(JavaThread*, > oopDesc*) src/hotspot/share/prims/jvmtiExport.cpp:2926 > #3 0x85e558b8 in > JvmtiObjectAllocEventCollector::generate_call_for_allocated() > src/hotspot/share/prims/jvmtiExport.cpp:3074 > #4 0x85e56c14 in > JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() > src/hotspot/share/prims/jvmtiExport.cpp:3171 > #5 0x85e56c14 in > JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() > src/hotspot/share/prims/jvmtiExport.cpp:3166 > #6 0x862ace34 in > MemAllocator::Allocation::notify_allocation_jvmti_sampler() > src/hotspot/share/gc/shared/memAllocator.cpp:196 > #7 0x862af7a4 in MemAllocator::Allocation::~Allocation() > src/hotspot/share/gc/shared/memAllocator.cpp:87 > #8 0x862af7a4 in MemAllocator::allocate() const > src/hotspot/share/gc/shared/memAllocator.cpp:356 > #9 0x86dca4dc in CollectedHeap::array_allocate(Klass*, unsigned long, > int, bool, JavaThread*) > src/hotspot/share/gc/shared/collectedHeap.inline.hpp:41 > #10 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, > JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:93 > #11 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, > JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:89 > #12 0x857f35c8 in InterpreterRuntime::newarray(JavaThread*, > BasicType, int) src/hotspot/share/interpreter/interpreterRuntime.cpp:232 > #13 0x6b094cf4 () This pull request has now been integrated. Changeset: ed149062 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/ed149062d0e8407710f083aa85d28d27c4a45ecc Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 861: ubsan,test : libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null Reviewed-by: asteiner, lucy, amenkov - PR: https://git.openjdk.org/jdk/pull/19804
RFR: 8333361: ubsan,test : libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null
The following issue has been observed when running serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest (and some other :tier1 tests) on Linux with ubsan enabled binaries : test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null #0 0x80388020 in event_storage_augment_storage test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518 #1 0x80388020 in event_storage_add test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:557 #2 0x85e695fc in JvmtiExport::post_sampled_object_alloc(JavaThread*, oopDesc*) src/hotspot/share/prims/jvmtiExport.cpp:2926 #3 0x85e558b8 in JvmtiObjectAllocEventCollector::generate_call_for_allocated() src/hotspot/share/prims/jvmtiExport.cpp:3074 #4 0x85e56c14 in JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() src/hotspot/share/prims/jvmtiExport.cpp:3171 #5 0x85e56c14 in JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() src/hotspot/share/prims/jvmtiExport.cpp:3166 #6 0x862ace34 in MemAllocator::Allocation::notify_allocation_jvmti_sampler() src/hotspot/share/gc/shared/memAllocator.cpp:196 #7 0x862af7a4 in MemAllocator::Allocation::~Allocation() src/hotspot/share/gc/shared/memAllocator.cpp:87 #8 0x862af7a4 in MemAllocator::allocate() const src/hotspot/share/gc/shared/memAllocator.cpp:356 #9 0x86dca4dc in CollectedHeap::array_allocate(Klass*, unsigned long, int, bool, JavaThread*) src/hotspot/share/gc/shared/collectedHeap.inline.hpp:41 #10 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:93 #11 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:89 #12 0x857f35c8 in InterpreterRuntime::newarray(JavaThread*, BasicType, int) src/hotspot/share/interpreter/interpreterRuntime.cpp:232 #13 0x6b094cf4 () - Commit messages: - JDK-861 Changes: https://git.openjdk.org/jdk/pull/19804/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19804=00 Issue: https://bugs.openjdk.org/browse/JDK-861 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19804.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19804/head:pull/19804 PR: https://git.openjdk.org/jdk/pull/19804
Integrated: 8333730: ubsan: FieldIndices/libFieldIndicesTest.cpp:276:11: runtime error: null pointer passed as argument 2, which is declared to never be null
On Tue, 11 Jun 2024 13:00:44 GMT, Matthias Baesken wrote: > When running HS :tier1 tests with ubsan-enabled binaries, the following issue > is reported : > test > serviceability/jvmti/FollowReferences/FieldIndices/FieldIndicesTest.jtr > > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:276:11: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x7efea442e379 in Klass::explore_interfaces(JNIEnv_*) > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:276 > #1 0x7efea443280d in Klass::explore(JNIEnv_*, _jclass*) > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:322 > #2 0x7efea4433adf in Object::explore(JNIEnv_*, _jobject*) > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:349 > #3 0x7efea443443b in Java_FieldIndicesTest_prepare > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:489 > #4 0x7efe7f8d1e7b () This pull request has now been integrated. Changeset: a7e4ab93 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/a7e4ab9300730c32f6cf0dafd48f5e093f4ac0be Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8333730: ubsan: FieldIndices/libFieldIndicesTest.cpp:276:11: runtime error: null pointer passed as argument 2, which is declared to never be null Reviewed-by: cjplummer, amenkov - PR: https://git.openjdk.org/jdk/pull/19657
Re: RFR: 8333730: ubsan: FieldIndices/libFieldIndicesTest.cpp:276:11: runtime error: null pointer passed as argument 2, which is declared to never be null
On Tue, 11 Jun 2024 13:00:44 GMT, Matthias Baesken wrote: > When running HS :tier1 tests with ubsan-enabled binaries, the following issue > is reported : > test > serviceability/jvmti/FollowReferences/FieldIndices/FieldIndicesTest.jtr > > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:276:11: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x7efea442e379 in Klass::explore_interfaces(JNIEnv_*) > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:276 > #1 0x7efea443280d in Klass::explore(JNIEnv_*, _jclass*) > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:322 > #2 0x7efea4433adf in Object::explore(JNIEnv_*, _jobject*) > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:349 > #3 0x7efea443443b in Java_FieldIndicesTest_prepare > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:489 > #4 0x7efe7f8d1e7b () Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/19657#issuecomment-2162264557
RFR: 8333730: ubsan: FieldIndices/libFieldIndicesTest.cpp:276:11: runtime error: null pointer passed as argument 2, which is declared to never be null
When running HS :tier1 tests with ubsan-enabled binaries, the following issue is reported : test serviceability/jvmti/FollowReferences/FieldIndices/FieldIndicesTest.jtr test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:276:11: runtime error: null pointer passed as argument 2, which is declared to never be null #0 0x7efea442e379 in Klass::explore_interfaces(JNIEnv_*) test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:276 #1 0x7efea443280d in Klass::explore(JNIEnv_*, _jclass*) test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:322 #2 0x7efea4433adf in Object::explore(JNIEnv_*, _jobject*) test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:349 #3 0x7efea443443b in Java_FieldIndicesTest_prepare test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp:489 #4 0x7efe7f8d1e7b () - Commit messages: - JDK-8333730 Changes: https://git.openjdk.org/jdk/pull/19657/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19657=00 Issue: https://bugs.openjdk.org/browse/JDK-8333730 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19657.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19657/head:pull/19657 PR: https://git.openjdk.org/jdk/pull/19657
Integrated: 8333178: ubsan: jvmti_tools.cpp:149:16: runtime error: null pointer passed as argument 2, which is declared to never be null
On Wed, 5 Jun 2024 13:04:03 GMT, Matthias Baesken wrote: > With ubsan enabled binaries we run into the following issue in HS :tier4 > tests : > e.g. > vmTestbase/nsk/jvmti/unit/FollowReferences/followref006/TestDescription.jtr > > /jdk/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:149:16: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x7fa7a1049482 in add_option > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:149 > #1 0x7fa7a1049482 in nsk_jvmti_parseOptions > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:242 > #2 0x7fa7a10634cd in Agent_Initialize > test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref006/followref006.cpp:216 > #3 0x7fa79a9dbb36 in invoke_Agent_OnLoad > src/hotspot/share/prims/jvmtiAgent.cpp:609 > #4 0x7fa79a9dbb36 in JvmtiAgent::load(outputStream*) > src/hotspot/share/prims/jvmtiAgent.cpp:623 > #5 0x7fa79a9defd4 in load_agents > src/hotspot/share/prims/jvmtiAgentList.cpp:179 > #6 0x7fa79a9defd4 in JvmtiAgentList::load_agents() > src/hotspot/share/prims/jvmtiAgentList.cpp:190 > #7 0x7fa79bdad503 in Threads::create_vm(JavaVMInitArgs*, bool*) > src/hotspot/share/runtime/threads.cpp:505 > #8 0x7fa79a6e531f in JNI_CreateJavaVM_inner > src/hotspot/share/prims/jni.cpp:3581 > #9 0x7fa79a6e531f in JNI_CreateJavaVM src/hotspot/share/prims/jni.cpp:3672 > #10 0x7fa7a11277d5 in InitializeJVM > src/java.base/share/native/libjli/java.c:1550 > #11 0x7fa7a11277d5 in JavaMain > src/java.base/share/native/libjli/java.c:491 > #12 0x7fa7a1130f68 in ThreadJavaMain > src/java.base/unix/native/libjli/java_md.c:653 > #13 0x7fa7a10df6e9 in start_thread (/lib64/libpthread.so.0+0xa6e9) > (BuildId: 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) > #14 0x7fa7a071550e in clone (/lib64/libc.so.6+0x11850e) (BuildId: > f732026552f6adff988b338e92d466bc81a01c37) This pull request has now been integrated. Changeset: 880c6b42 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/880c6b42ba74884690daa5c23f6605876f29aece Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8333178: ubsan: jvmti_tools.cpp:149:16: runtime error: null pointer passed as argument 2, which is declared to never be null Reviewed-by: cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/19557
Re: RFR: 8333178: ubsan: jvmti_tools.cpp:149:16: runtime error: null pointer passed as argument 2, which is declared to never be null
On Wed, 5 Jun 2024 13:04:03 GMT, Matthias Baesken wrote: > With ubsan enabled binaries we run into the following issue in HS :tier4 > tests : > e.g. > vmTestbase/nsk/jvmti/unit/FollowReferences/followref006/TestDescription.jtr > > /jdk/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:149:16: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x7fa7a1049482 in add_option > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:149 > #1 0x7fa7a1049482 in nsk_jvmti_parseOptions > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:242 > #2 0x7fa7a10634cd in Agent_Initialize > test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref006/followref006.cpp:216 > #3 0x7fa79a9dbb36 in invoke_Agent_OnLoad > src/hotspot/share/prims/jvmtiAgent.cpp:609 > #4 0x7fa79a9dbb36 in JvmtiAgent::load(outputStream*) > src/hotspot/share/prims/jvmtiAgent.cpp:623 > #5 0x7fa79a9defd4 in load_agents > src/hotspot/share/prims/jvmtiAgentList.cpp:179 > #6 0x7fa79a9defd4 in JvmtiAgentList::load_agents() > src/hotspot/share/prims/jvmtiAgentList.cpp:190 > #7 0x7fa79bdad503 in Threads::create_vm(JavaVMInitArgs*, bool*) > src/hotspot/share/runtime/threads.cpp:505 > #8 0x7fa79a6e531f in JNI_CreateJavaVM_inner > src/hotspot/share/prims/jni.cpp:3581 > #9 0x7fa79a6e531f in JNI_CreateJavaVM src/hotspot/share/prims/jni.cpp:3672 > #10 0x7fa7a11277d5 in InitializeJVM > src/java.base/share/native/libjli/java.c:1550 > #11 0x7fa7a11277d5 in JavaMain > src/java.base/share/native/libjli/java.c:491 > #12 0x7fa7a1130f68 in ThreadJavaMain > src/java.base/unix/native/libjli/java_md.c:653 > #13 0x7fa7a10df6e9 in start_thread (/lib64/libpthread.so.0+0xa6e9) > (BuildId: 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) > #14 0x7fa7a071550e in clone (/lib64/libc.so.6+0x11850e) (BuildId: > f732026552f6adff988b338e92d466bc81a01c37) Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/19557#issuecomment-2151620741
RFR: 8333178: ubsan: jvmti_tools.cpp:149:16: runtime error: null pointer passed as argument 2, which is declared to never be null
With ubsan enabled binaries we run into the following issue in HS :tier4 tests : e.g. vmTestbase/nsk/jvmti/unit/FollowReferences/followref006/TestDescription.jtr /jdk/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:149:16: runtime error: null pointer passed as argument 2, which is declared to never be null #0 0x7fa7a1049482 in add_option test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:149 #1 0x7fa7a1049482 in nsk_jvmti_parseOptions test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp:242 #2 0x7fa7a10634cd in Agent_Initialize test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref006/followref006.cpp:216 #3 0x7fa79a9dbb36 in invoke_Agent_OnLoad src/hotspot/share/prims/jvmtiAgent.cpp:609 #4 0x7fa79a9dbb36 in JvmtiAgent::load(outputStream*) src/hotspot/share/prims/jvmtiAgent.cpp:623 #5 0x7fa79a9defd4 in load_agents src/hotspot/share/prims/jvmtiAgentList.cpp:179 #6 0x7fa79a9defd4 in JvmtiAgentList::load_agents() src/hotspot/share/prims/jvmtiAgentList.cpp:190 #7 0x7fa79bdad503 in Threads::create_vm(JavaVMInitArgs*, bool*) src/hotspot/share/runtime/threads.cpp:505 #8 0x7fa79a6e531f in JNI_CreateJavaVM_inner src/hotspot/share/prims/jni.cpp:3581 #9 0x7fa79a6e531f in JNI_CreateJavaVM src/hotspot/share/prims/jni.cpp:3672 #10 0x7fa7a11277d5 in InitializeJVM src/java.base/share/native/libjli/java.c:1550 #11 0x7fa7a11277d5 in JavaMain src/java.base/share/native/libjli/java.c:491 #12 0x7fa7a1130f68 in ThreadJavaMain src/java.base/unix/native/libjli/java_md.c:653 #13 0x7fa7a10df6e9 in start_thread (/lib64/libpthread.so.0+0xa6e9) (BuildId: 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) #14 0x7fa7a071550e in clone (/lib64/libc.so.6+0x11850e) (BuildId: f732026552f6adff988b338e92d466bc81a01c37) - Commit messages: - JDK-8333178 Changes: https://git.openjdk.org/jdk/pull/19557/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19557=00 Issue: https://bugs.openjdk.org/browse/JDK-8333178 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19557.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19557/head:pull/19557 PR: https://git.openjdk.org/jdk/pull/19557
Integrated: 8333149: ubsan : memset on nullptr target detected in jvmtiEnvBase.cpp get_object_monitor_usage
On Wed, 29 May 2024 09:09:16 GMT, Matthias Baesken wrote: > When running with ubsan - enabled binaries (--enable-ubsan), > in the vmTestbase/nsk/jdi tests some cases of memset on nullptr destinations > are detected in get_object_monitor_usage . > > // null out memory for robustness > memset(ret.waiters, 0, ret.waiter_count * sizeof(jthread *)); > memset(ret.notify_waiters, 0, ret.notify_waiter_count * sizeof(jthread *)); > > probably we should add checks there. > Example : > vmTestbase/nsk/jdi/ObjectReference/entryCount/entrycount002/TestDescription.jtr > > debugee.stderr> /src/hotspot/share/prims/jvmtiEnvBase.cpp:1560:11: runtime > error: null pointer passed as argument 1, which is declared to never be null > debugee.stderr> #0 0x7ffb2568559c in > JvmtiEnvBase::get_object_monitor_usage(JavaThread*, _jobject*, > jvmtiMonitorUsage*) src/hotspot/share/prims/jvmtiEnvBase.cpp:1560 > debugee.stderr> #1 0x7ffb27987bd7 in VM_GetObjectMonitorUsage::doit() > src/hotspot/share/prims/jvmtiEnvBase.hpp:594 > debugee.stderr> #2 0x7ffb28ddc2dd in VM_Operation::evaluate() > src/hotspot/share/runtime/vmOperations.cpp:75 > debugee.stderr> #3 0x7ffb28deac41 in > VMThread::evaluate_operation(VM_Operation*) > src/hotspot/share/runtime/vmThread.cpp:283 > debugee.stderr> #4 0x7ffb28decc4f in VMThread::inner_execute(VM_Operation*) > src/hotspot/share/runtime/vmThread.cpp:427 > debugee.stderr> #5 0x7ffb28ded7b9 in VMThread::loop() > src/hotspot/share/runtime/vmThread.cpp:493 > debugee.stderr> #6 0x7ffb28ded8a7 in VMThread::run() > src/hotspot/share/runtime/vmThread.cpp:177 > debugee.stderr> #7 0x7ffb28b7e31a in Thread::call_run() > src/hotspot/share/runtime/thread.cpp:225 > debugee.stderr> #8 0x7ffb281c4971 in thread_native_entry > src/hotspot/os/linux/os_linux.cpp:846 > debugee.stderr> #9 0x7ffb2df416e9 in start_thread > (/lib64/libpthread.so.0+0xa6e9) (BuildId: > 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) > debugee.stderr> #10 0x7ffb2d51550e in clone (/lib64/libc.so.6+0x11850e) > (BuildId: f732026552f6adff988b338e92d466bc81a01c37) > > vmTestbase/nsk/jdi/ObjectReference/owningThread/owningthread002/TestDescription.jtr > > debugee.stderr> /src/hotspot/share/prims/jvmtiEnvBase.cpp:1561:11: runtime > error: null pointer passed as argument 1, which is declared to never be null > debugee.stderr> #0 0x7f1e070855bb in > JvmtiEnvBase::get_object_monitor_usage(JavaThread*, _jobject*, > jvmtiMonitorUsage*) src/hotspot/share/prims/jvmtiEnvBase.cpp:1561 > debugee.stderr> #1 0x7f1e09387bd7 in VM_GetObjectMonitorUsage::doit() > src/hotspot/share/prims/jvmtiEnvBase.hpp:594 > debugee.stderr> #2 0x7f1e0a7dc2dd in VM_Operation::evaluate() src/hotsp... This pull request has now been integrated. Changeset: 43a2f173 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/43a2f17342af8f5bf1f5823df9fa0bf0bdfdfce2 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod 8333149: ubsan : memset on nullptr target detected in jvmtiEnvBase.cpp get_object_monitor_usage Reviewed-by: sspitsyn, mdoerr - PR: https://git.openjdk.org/jdk/pull/19450
Re: RFR: 8333149: ubsan : memset on nullptr target detected in jvmtiEnvBase.cpp get_object_monitor_usage
On Wed, 29 May 2024 09:09:16 GMT, Matthias Baesken wrote: > When running with ubsan - enabled binaries (--enable-ubsan), > in the vmTestbase/nsk/jdi tests some cases of memset on nullptr destinations > are detected in get_object_monitor_usage . > > // null out memory for robustness > memset(ret.waiters, 0, ret.waiter_count * sizeof(jthread *)); > memset(ret.notify_waiters, 0, ret.notify_waiter_count * sizeof(jthread *)); > > probably we should add checks there. > Example : > vmTestbase/nsk/jdi/ObjectReference/entryCount/entrycount002/TestDescription.jtr > > debugee.stderr> /src/hotspot/share/prims/jvmtiEnvBase.cpp:1560:11: runtime > error: null pointer passed as argument 1, which is declared to never be null > debugee.stderr> #0 0x7ffb2568559c in > JvmtiEnvBase::get_object_monitor_usage(JavaThread*, _jobject*, > jvmtiMonitorUsage*) src/hotspot/share/prims/jvmtiEnvBase.cpp:1560 > debugee.stderr> #1 0x7ffb27987bd7 in VM_GetObjectMonitorUsage::doit() > src/hotspot/share/prims/jvmtiEnvBase.hpp:594 > debugee.stderr> #2 0x7ffb28ddc2dd in VM_Operation::evaluate() > src/hotspot/share/runtime/vmOperations.cpp:75 > debugee.stderr> #3 0x7ffb28deac41 in > VMThread::evaluate_operation(VM_Operation*) > src/hotspot/share/runtime/vmThread.cpp:283 > debugee.stderr> #4 0x7ffb28decc4f in VMThread::inner_execute(VM_Operation*) > src/hotspot/share/runtime/vmThread.cpp:427 > debugee.stderr> #5 0x7ffb28ded7b9 in VMThread::loop() > src/hotspot/share/runtime/vmThread.cpp:493 > debugee.stderr> #6 0x7ffb28ded8a7 in VMThread::run() > src/hotspot/share/runtime/vmThread.cpp:177 > debugee.stderr> #7 0x7ffb28b7e31a in Thread::call_run() > src/hotspot/share/runtime/thread.cpp:225 > debugee.stderr> #8 0x7ffb281c4971 in thread_native_entry > src/hotspot/os/linux/os_linux.cpp:846 > debugee.stderr> #9 0x7ffb2df416e9 in start_thread > (/lib64/libpthread.so.0+0xa6e9) (BuildId: > 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) > debugee.stderr> #10 0x7ffb2d51550e in clone (/lib64/libc.so.6+0x11850e) > (BuildId: f732026552f6adff988b338e92d466bc81a01c37) > > vmTestbase/nsk/jdi/ObjectReference/owningThread/owningthread002/TestDescription.jtr > > debugee.stderr> /src/hotspot/share/prims/jvmtiEnvBase.cpp:1561:11: runtime > error: null pointer passed as argument 1, which is declared to never be null > debugee.stderr> #0 0x7f1e070855bb in > JvmtiEnvBase::get_object_monitor_usage(JavaThread*, _jobject*, > jvmtiMonitorUsage*) src/hotspot/share/prims/jvmtiEnvBase.cpp:1561 > debugee.stderr> #1 0x7f1e09387bd7 in VM_GetObjectMonitorUsage::doit() > src/hotspot/share/prims/jvmtiEnvBase.hpp:594 > debugee.stderr> #2 0x7f1e0a7dc2dd in VM_Operation::evaluate() src/hotsp... Hi Martin and Serguei, thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/19450#issuecomment-2137313538
RFR: 8333149: ubsan : memset on nullptr target detected in jvmtiEnvBase.cpp get_object_monitor_usage
When running with ubsan - enabled binaries (--enable-ubsan), in the vmTestbase/nsk/jdi tests some cases of memset on nullptr destinations are detected in get_object_monitor_usage . // null out memory for robustness memset(ret.waiters, 0, ret.waiter_count * sizeof(jthread *)); memset(ret.notify_waiters, 0, ret.notify_waiter_count * sizeof(jthread *)); probably we should add checks there. Example : vmTestbase/nsk/jdi/ObjectReference/entryCount/entrycount002/TestDescription.jtr debugee.stderr> /src/hotspot/share/prims/jvmtiEnvBase.cpp:1560:11: runtime error: null pointer passed as argument 1, which is declared to never be null debugee.stderr> #0 0x7ffb2568559c in JvmtiEnvBase::get_object_monitor_usage(JavaThread*, _jobject*, jvmtiMonitorUsage*) src/hotspot/share/prims/jvmtiEnvBase.cpp:1560 debugee.stderr> #1 0x7ffb27987bd7 in VM_GetObjectMonitorUsage::doit() src/hotspot/share/prims/jvmtiEnvBase.hpp:594 debugee.stderr> #2 0x7ffb28ddc2dd in VM_Operation::evaluate() src/hotspot/share/runtime/vmOperations.cpp:75 debugee.stderr> #3 0x7ffb28deac41 in VMThread::evaluate_operation(VM_Operation*) src/hotspot/share/runtime/vmThread.cpp:283 debugee.stderr> #4 0x7ffb28decc4f in VMThread::inner_execute(VM_Operation*) src/hotspot/share/runtime/vmThread.cpp:427 debugee.stderr> #5 0x7ffb28ded7b9 in VMThread::loop() src/hotspot/share/runtime/vmThread.cpp:493 debugee.stderr> #6 0x7ffb28ded8a7 in VMThread::run() src/hotspot/share/runtime/vmThread.cpp:177 debugee.stderr> #7 0x7ffb28b7e31a in Thread::call_run() src/hotspot/share/runtime/thread.cpp:225 debugee.stderr> #8 0x7ffb281c4971 in thread_native_entry src/hotspot/os/linux/os_linux.cpp:846 debugee.stderr> #9 0x7ffb2df416e9 in start_thread (/lib64/libpthread.so.0+0xa6e9) (BuildId: 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) debugee.stderr> #10 0x7ffb2d51550e in clone (/lib64/libc.so.6+0x11850e) (BuildId: f732026552f6adff988b338e92d466bc81a01c37) vmTestbase/nsk/jdi/ObjectReference/owningThread/owningthread002/TestDescription.jtr debugee.stderr> /src/hotspot/share/prims/jvmtiEnvBase.cpp:1561:11: runtime error: null pointer passed as argument 1, which is declared to never be null debugee.stderr> #0 0x7f1e070855bb in JvmtiEnvBase::get_object_monitor_usage(JavaThread*, _jobject*, jvmtiMonitorUsage*) src/hotspot/share/prims/jvmtiEnvBase.cpp:1561 debugee.stderr> #1 0x7f1e09387bd7 in VM_GetObjectMonitorUsage::doit() src/hotspot/share/prims/jvmtiEnvBase.hpp:594 debugee.stderr> #2 0x7f1e0a7dc2dd in VM_Operation::evaluate() src/hotspot/share/runtime/vmOperations.cpp:75 debugee.stderr> #3 0x7f1e0a7eac41 in VMThread::evaluate_operation(VM_Operation*) src/hotspot/share/runtime/vmThread.cpp:283 debugee.stderr> #4 0x7f1e0a7ecc4f in VMThread::inner_execute(VM_Operation*) src/hotspot/share/runtime/vmThread.cpp:427 debugee.stderr> #5 0x7f1e0a7ed7b9 in VMThread::loop() src/hotspot/share/runtime/vmThread.cpp:493 debugee.stderr> #6 0x7f1e0a7ed8a7 in VMThread::run() src/hotspot/share/runtime/vmThread.cpp:177 debugee.stderr> #7 0x7f1e0a57e31a in Thread::call_run() src/hotspot/share/runtime/thread.cpp:225 debugee.stderr> #8 0x7f1e09bc4971 in thread_native_entry src/hotspot/os/linux/os_linux.cpp:846 debugee.stderr> #9 0x7f1e0f9bf6e9 in start_thread (/lib64/libpthread.so.0+0xa6e9) (BuildId: 2f8d3c2d0f4d7888c2598d2ff6356537f5708a73) debugee.stderr> #10 0x7f1e0ef1550e in clone (/lib64/libc.so.6+0x11850e) (BuildId: f732026552f6adff988b338e92d466bc81a01c37) - Commit messages: - JDK-8333149 Changes: https://git.openjdk.org/jdk/pull/19450/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19450=00 Issue: https://bugs.openjdk.org/browse/JDK-8333149 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19450.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19450/head:pull/19450 PR: https://git.openjdk.org/jdk/pull/19450
Re: RFR: 8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Wed, 27 Mar 2024 13:44:42 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust java.1 man page Another option would be to implement an additional jcmd command where jcmd GC.heap_dump_other_command_name without options writes to location given by -XX:HeapDumpPath, if set. This would avoid all the compatibility concerns raised by changing jcmd GC.heap_dump; would this be a welcome change ? - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-210731
Re: RFR: 8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Wed, 27 Mar 2024 13:44:42 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust java.1 man page > It may be desirable to have VM.heap_dump support not specifying a filename > and instead just have it choose a default name > and path, probably the current working directory with a name something like > java_pid.hprof. So is there some intention to decide to have such a default now or in the near future ? Otherwise it is a rather theoretical discussion . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2077099599
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Wed, 10 Apr 2024 04:34:53 GMT, Chris Plummer wrote: > One other thing I just realized is that if we go through with this change, > then we can't in the future change VM.heap_dump so it can be used without any > filename (make it use a default filename when no filename is specified). > Setting HeapDumpPath would override this default filename, and it would be > really confusing to the user when suddenly heap dumps are not appearing in > the default location with the default name because someone decided to specify > HeapDumpPath when launching the JVM. Isn't this the case already for the -XX:+HeapDumpBeforeFullGC, -XX:+HeapDumpAfterFullGC scenarios? But probably it should be decided first if such a default is planned for VM.heap_dump . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2047487391
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Mon, 8 Apr 2024 06:47:09 GMT, Matthias Baesken wrote: > If (3) has defaults, why (4) and (maybe) (5) don't have the same defaults? I could open a separate JBS issue (or issues) for scenario 4 and 5 , if this is desired. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2042690069
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Fri, 5 Apr 2024 22:47:55 GMT, Alex Menkov wrote: > I don't have strong opinion if it's good to have default file path for `jcmd > GC.heap_dump`, just some thoughts. We have several ways to heap dump: > > 1. `-XX:+HeapDumpOnOutOfMemoryError` > 2. `-XX:+HeapDumpBeforeFullGC`, `-XX:+HeapDumpAfterFullGC` >in the case `HeapDumpPath` and `HeapDumpGzipLevel` are used (this is not > mentioned in the options' description, need to fix it) Was that ever a problem raised by users that the OOME and the GC related heap dumps end up at the same location ? > 3. `jcmd GC.heap_dump` > 4. `jmap -dump` >it uses "dumpheap" Attach operation, implemented by AttachListener directly > 5. `HotSpotDiagnosticMXBean.dumpHeap()`; > > Current patch looks inconsistent: If HeapDumpPath is used as default, > HeapDumpGzipLevel should be used as default too (note that default path has > different extension depending on HeapDumpGzipLevel); If (3) has defaults, why > (4) and (maybe) (5) don't have the same defaults? Yes , probably with 1. + 2, and now (with the patch) 3. using HeapDumpPath the others should use it most likely too. (but our users just asked for 3. that's why the patch). About HeapDumpGzipLevel , I could add this (I think this was already mentioned), but before adding it I wanted to be sure that there the change is supported. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2041981493
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Fri, 5 Apr 2024 08:57:09 GMT, Sandra Payne wrote: > Many customers may see such a change as a regression, as they rely on > separation of location for heap dumps generated by the > JVM at OOME and heap > dumps manually pulled by various other processes attaching to the JVM. With this change it is still possible to have separation (nobody stops users from using jcmd with the filename option, this is still available). Or do you think that some tool/script developers would maybe (with this change) in future more use the -XX:HeapDumpPath for **_both_** OOME and jcmd, so that customers would run into this scenario ? - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2039511382
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Fri, 29 Mar 2024 04:15:24 GMT, Chris Plummer wrote: > There's still the question of whether or not it is even appropriate to have > -XX options taking the place of jcmd options. Some people (like our cloud support colleagues and also some who commented) would like this approach, some find it not very useful. Seems there are also already other globals flags playing a role in diagnosticCommand coding (e.g. RecordDynamicDumpInfo, maybe more) so it is not completely 'new' that the jcmd commands are influenced by globals flag. Looking at just the _names_ of HeapDumpPath and also HeapDumpGzipLevel I would think that they play a role for all heap dumps and not just some. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2033985633
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Fri, 29 Mar 2024 04:15:24 GMT, Chris Plummer wrote: > There's also a question of whether currently missing doc updates for > HeapDumpGzipLevel should be made part of this PR > because it complicates back porting. It should most likely be a separate PR (the title of this one does not match, and there are different backporting needs). - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2033834669
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Thu, 28 Mar 2024 07:02:16 GMT, Thomas Stuefe wrote: > Wouldn't this just be a case of changing a flag description? As luck has it, > the flag already has a generic name that is not tied to OOMs. Yes it is some work on documentation (but seems some doc work needs to be done anyway because it was forgotten when `HeapDumpGzipLevel` was introduced). And the current name is generic and does not mention the OOM case in the name itself, the current implementation would better match the name if it was `HeapDumpPathOnOom` or something like this. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2025456286
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
On Wed, 27 Mar 2024 23:33:18 GMT, Chris Plummer wrote: > I did get feedback from a couple of our support engineers. One felt it was of > no real benefit as it was just as easy to provide the > filename as an > argument to the jcmd. That might be true for this support engineer, but not for others with other setups where it is an issue to find out where to write in cloud/container scenarios . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2024704505
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]
On Wed, 27 Mar 2024 23:26:14 GMT, Chris Plummer wrote: > > Backporting also came to mind since the trouble shooting guide would need to > be updated for each Oracle version this PR is backported to. And yes, > HeapDumpGzipLevel docs need to match the actual implementation. I'm not sure > if HeapDumpGzipLevel has been backported to 8, 11, and 17 (in the open for > for the Oracle release). I'm inclined to say a separate jbs issue should > track updating docs for HeapDumpGzipLevel, but then we also have the question > of whether or not HeapDumpGzipLevel should impact the jcmd if this PR is to > be pushed. HeapDumpGzipLevel is in openjdk17 and openjdk21 . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2024698315
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v10]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust java.1 man page - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/b414b1be..8039b9a4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=09 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=08-09 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]
On Tue, 26 Mar 2024 21:42:15 GMT, Chris Plummer wrote: > A docs CR will need to be filed to get it updated (and I see it is already > appears out-of-date w.r.t. HeapDumpGzipLevel) Sorry maybe it is maybe obvious for you but where should I open a "docs CR", would that be a separate JBS issue with Component name docs ? Should I include the HeapDumpGzipLevel in the same "docs CR" (but this might be bad for potential backporting) ? - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2022660444
Integrated: JDK-8328930: [AIX] remove pase related coding
On Mon, 25 Mar 2024 13:26:12 GMT, Matthias Baesken wrote: > We still have quite a lot of OS400 / pase related coding in the hotspot AIX > codebase. But this does not work and was never supported in OpenJDK. So we > can remove this. This pull request has now been integrated. Changeset: 153410f4 Author: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/153410f480c372604e5825bfcd3a63f137e6a013 Stats: 416 lines in 8 files changed: 9 ins; 363 del; 44 mod 8328930: [AIX] remove pase related coding Reviewed-by: clanger, lucy - PR: https://git.openjdk.org/jdk/pull/18471
Re: RFR: JDK-8328930: [AIX] remove pase related coding [v2]
On Tue, 26 Mar 2024 14:18:34 GMT, Matthias Baesken wrote: >> We still have quite a lot of OS400 / pase related coding in the hotspot AIX >> codebase. But this does not work and was never supported in OpenJDK. So we >> can remove this. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust/remove some comments Hi Lutz and Christoph, thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/18471#issuecomment-2020898141
Re: RFR: JDK-8328930: [AIX] remove pase related coding
On Mon, 25 Mar 2024 13:26:12 GMT, Matthias Baesken wrote: > We still have quite a lot of OS400 / pase related coding in the hotspot AIX > codebase. But this does not work and was never supported in OpenJDK. So we > can remove this. Hi Christoph, I adjusted/removed the comments. - PR Comment: https://git.openjdk.org/jdk/pull/18471#issuecomment-2020550254
Re: RFR: JDK-8328930: [AIX] remove pase related coding [v2]
> We still have quite a lot of OS400 / pase related coding in the hotspot AIX > codebase. But this does not work and was never supported in OpenJDK. So we > can remove this. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust/remove some comments - Changes: - all: https://git.openjdk.org/jdk/pull/18471/files - new: https://git.openjdk.org/jdk/pull/18471/files/d1c4c7de..ba02f086 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18471=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18471=00-01 Stats: 10 lines in 1 file changed: 0 ins; 9 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18471.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18471/head:pull/18471 PR: https://git.openjdk.org/jdk/pull/18471
Re: RFR: JDK-8328930: [AIX] remove pase related coding
On Tue, 26 Mar 2024 09:27:01 GMT, Christoph Langer wrote: >> We still have quite a lot of OS400 / pase related coding in the hotspot AIX >> codebase. But this does not work and was never supported in OpenJDK. So we >> can remove this. > > src/hotspot/os/aix/os_aix.cpp line 626: > >> 624: >> 625: // excerpt from >> 626: // http://publib.boulder.ibm.com/infocenter/systems/index.jsp > > Same here. URL on one line. Does it still work? The link is dead, it does not work any more. I think we can remove it. - PR Review Comment: https://git.openjdk.org/jdk/pull/18471#discussion_r1539288321
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v9]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: suggestions from Chris for diagnosticCommand.cpp - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/8b48c911..b414b1be Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=08 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=07-08 Stats: 12 lines in 1 file changed: 0 ins; 3 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Fri, 15 Mar 2024 11:24:53 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust jcmd manpage, help and globals comment Hi Chris, thanks for the suggestions , I added them to the latest commit. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2020494383
RFR: JDK-8328930: [AIX] remove pase related coding
We still have quite a lot of OS400 / pase related coding in the hotspot AIX codebase. But this does not work and was never supported in OpenJDK. So we can remove this. - Commit messages: - JDK-8328930 Changes: https://git.openjdk.org/jdk/pull/18471/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18471=00 Issue: https://bugs.openjdk.org/browse/JDK-8328930 Stats: 419 lines in 8 files changed: 12 ins; 357 del; 50 mod Patch: https://git.openjdk.org/jdk/pull/18471.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18471/head:pull/18471 PR: https://git.openjdk.org/jdk/pull/18471
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Thu, 21 Mar 2024 16:44:29 GMT, Chris Plummer wrote: > I'd like to consult with a couple of people at Oracle who unfortunately are > out of town until Monday. Can this PR wait until next week? That's fine, no need to rush it into jdk-head this week. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2014480135
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Fri, 15 Mar 2024 11:24:53 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust jcmd manpage, help and globals comment > In a cloud environment using containers, the HeapDumpPath is automatically > set to a file system service to persist the heapdump. However, if a support > engineer or DevOps detects high or increasing memory usage and wants to > trigger a heapdump via jcmd, they would need to specify the filename. This > requires retrieving the set HeapDumpPath from the app environment and copying > it to the jcmd to use the persistent file system. This change can help avoid > the need for an additional copy and paste step, which is prone to errors. Hi Andreas , thanks for the details . Chris, is this understable for you? We indeed had quite a few user complains by cloud env users, that the HeapDumpPath is currently not evaluated in the jcmd case/scenario . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-2012910338
Re: RFR: 8327990: [macosx-aarch64] Various tests fail with -XX:+AssertWXAtThreadSync [v3]
On Tue, 19 Mar 2024 12:17:22 GMT, David Holmes wrote: > IIUC no actual failures are observed other than those pertaining to > AssertWXAtThreadSync We saw sporadic crashes in our jtreg (maybe also jck?) runs; only **_later_** we enabled AssertWXAtThreadSync for more checking. - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-2007159560
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Tue, 19 Mar 2024 10:13:16 GMT, Matthias Baesken wrote: >> So far we only use the gzip level setting from jcmd, not HeapDumpGzipLevel . >> See the `level` variable in `HeapDumpDCmd::execute` . Yeah you are probably >> right we should make it consistent. > >> If -XX:HeapDumpPath is specified, then it is used as the default > > No, the filename set with jcmd GC:heamp_dump filename has priority. So we > should better keep the current description. So should I also use HeapDumpGzipLevel the same way as HeapDumpPath ? Tehn we have to change the text in globals.hpp for HeapDumpGzipLevel as well because it mentions only the HeapDumpOnOutOfmemoryError case and not the jcmd case . - PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1530082635
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Sat, 16 Mar 2024 12:50:05 GMT, Matthias Baesken wrote:
>> src/hotspot/share/services/diagnosticCommand.cpp line 475:
>>
>>> 473: HeapDumpDCmd::HeapDumpDCmd(outputStream* output, bool heap) :
>>> 474:DCmdWithParser(output, heap),
>>> 475: _filename("filename","Name of the dump file", "STRING", false, "if
>>> no filename was specified, but -XX:HeapDumpPath=hdp is set, path hdp is
>>> taken"),
>>
>> This seems clumsy, but I'm having a hard time coming up with something
>> better.
>>
>> "the filename specified by -XX:HeapDumpPath, if specified"
>> "If -XX:HeapDumpPath is specified, then it is used as the default"
>>
>> Makes me wonder if this approach is wrong since it is hard to document
>> clearly. Maybe we should go back to not having the jcmd use HeapDumpPath. I
>> understand the argument for having the jcmd use the HeapDumpPath setting,
>> but it might not be worth the documentation confusion it introduces. You can
>> argue that HeapDumpPath really is just intended to help support
>> HeapDumpOnOutOfMemoryError. Does the jcmd also use HeapDumpGzipLevel? I'm
>> not sure, but we should be consistent with the application of HeapDumpPath
>> and HeapDumpGzipLevel to the jcmd.
>
> So far we only use the gzip level setting from jcmd, not HeapDumpGzipLevel .
> See the `level` variable in `HeapDumpDCmd::execute` . Yeah you are probably
> right we should make it consistent.
> If -XX:HeapDumpPath is specified, then it is used as the default
No, the filename set with jcmd GC:heamp_dump filename has priority. So we
should better keep the current description.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1530077879
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Fri, 15 Mar 2024 19:22:58 GMT, Chris Plummer wrote:
>> Matthias Baesken has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Adjust jcmd manpage, help and globals comment
>
> src/hotspot/share/services/diagnosticCommand.cpp line 475:
>
>> 473: HeapDumpDCmd::HeapDumpDCmd(outputStream* output, bool heap) :
>> 474:DCmdWithParser(output, heap),
>> 475: _filename("filename","Name of the dump file", "STRING", false, "if no
>> filename was specified, but -XX:HeapDumpPath=hdp is set, path hdp is taken"),
>
> This seems clumsy, but I'm having a hard time coming up with something better.
>
> "the filename specified by -XX:HeapDumpPath, if specified"
> "If -XX:HeapDumpPath is specified, then it is used as the default"
>
> Makes me wonder if this approach is wrong since it is hard to document
> clearly. Maybe we should go back to not having the jcmd use HeapDumpPath. I
> understand the argument for having the jcmd use the HeapDumpPath setting, but
> it might not be worth the documentation confusion it introduces. You can
> argue that HeapDumpPath really is just intended to help support
> HeapDumpOnOutOfMemoryError. Does the jcmd also use HeapDumpGzipLevel? I'm not
> sure, but we should be consistent with the application of HeapDumpPath and
> HeapDumpGzipLevel to the jcmd.
So far we only use the gzip level setting from jcmd, not HeapDumpGzipLevel .
See the `level` variable in `HeapDumpDCmd::execute` . Yeah you are probably
right we should make it consistent.
> src/jdk.jcmd/share/man/jcmd.1 line 340:
>
>> 338: \f[I]filename\f[R]: The name of the dump file; in case no file is
>> specified
>> 339: but -XX:HeapDumpPath=path is set, the path provided by HeapDumpPath is
>> used
>> 340: (STRING, no default value)
>
> Here we say "no default value" but the actual text of the help output says
> something different. But then what do we put in place of "no default value"
> here? Descriptive text (like in the help output) is not needed since we have
> the description here. I don't really have an answer for how to handle this.
Hi , 'no default' should be still correct. Both the old syntax and also the
additional optional new one (evaluating -XX:HeapDumpPath) do not set some
default string. The user has to specify something.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1527168801
PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1527167229
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust jcmd manpage, help and globals comment - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/a71f04b0..8b48c911 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=07 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=06-07 Stats: 7 lines in 3 files changed: 3 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v7]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: change is on to is enabled - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/b97921d5..a71f04b0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=06 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=05-06 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v6]
On Fri, 15 Mar 2024 08:35:56 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > rename dump_to_heapdump_path Hi Chris, thanks for the comments- > Also, if you are cleaning up this text, I would suggest changing "is on" to > "is enabled". Same for HeapDumpGzipLevel below. I changed the two locations. btw. seems we have more of those in globals.hpp . See the comments related to PrintNMTStatistics and LogFile (where the "is on" is used as well). - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1999211132
Re: RFR: 8327990: [macosx-aarch64] JFR enters VM without WXWrite
On Tue, 12 Mar 2024 15:05:00 GMT, Richard Reingruber wrote: > This pr changes `JfrJvmtiAgent::retransform_classes()` and `jfr_set_enabled` > to switch to `WXWrite` before transitioning to the vm. > > Testing: > make test TEST=jdk/jfr/event/runtime/TestClassLoadEvent.java > TEST_VM_OPTS=-XX:+AssertWXAtThreadSync > make test TEST=compiler/intrinsics/klass/CastNullCheckDroppingsTest.java > TEST_VM_OPTS=-XX:+AssertWXAtThreadSync > > More tests are pending. `JfrRecorderService::emit_leakprofiler_events` (src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp ) and `JfrJavaEventWriter::flush` (src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp) might need adjustment too (see the other findings I posted yesterday). - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-1999185059
Re: RFR: 8327990: [macosx-aarch64] JFR enters VM without WXWrite
On Thu, 14 Mar 2024 14:49:12 GMT, Matthias Baesken wrote: >> This pr changes `JfrJvmtiAgent::retransform_classes()` and >> `jfr_set_enabled` to switch to `WXWrite` before transitioning to the vm. >> >> Testing: >> make test TEST=jdk/jfr/event/runtime/TestClassLoadEvent.java >> TEST_VM_OPTS=-XX:+AssertWXAtThreadSync >> make test TEST=compiler/intrinsics/klass/CastNullCheckDroppingsTest.java >> TEST_VM_OPTS=-XX:+AssertWXAtThreadSync >> >> More tests are pending. > > I noticed a few more asserts (assert(_wx_state == expected) failed: wrong > state) in the jfr area (jdk tier3 jfr tests). > E.g. > > > V [libjvm.dylib+0x8a5d94] JavaThread::check_for_valid_safepoint_state()+0x0 > V [libjvm.dylib+0x3e95b4] > ThreadStateTransition::transition_from_native(JavaThread*, JavaThreadState, > bool)+0x174 > V [libjvm.dylib+0x3e93d0] > ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*)+0x70 > V [libjvm.dylib+0x91a578] JfrRecorderService::emit_leakprofiler_events(long > long, bool, bool)+0xcc > > > > and > > > V [libjvm.dylib+0x8a5d94] JavaThread::check_for_valid_safepoint_state()+0x0 > V [libjvm.dylib+0x3e95b4] > ThreadStateTransition::transition_from_native(JavaThread*, JavaThreadState, > bool)+0x174 > V [libjvm.dylib+0x3e93d0] > ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*)+0x70 > V [libjvm.dylib+0x8d7f74] JfrJavaEventWriter::flush(_jobject*, int, int, > JavaThread*)+0xf8 > j jdk.jfr.internal.JVM.flush(Ljdk/jfr/internal/event/EventWriter;II)V+0 > jdk.jfr@23-internal > j jdk.jfr.internal.event.EventWriter.flush(II)V+3 jdk.jfr@23-internal > > > @MBaesken found 2 more locations in jvmti that need switching to `WXWrite` > > > ``` > > > JvmtiExport::get_jvmti_interface > > > GetCarrierThread > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Both use `ThreadInVMfromNative`. > > > > > > Should we address those 2 more findings in this PR ? Or open a separate JBS > > issue ? > > I'm leaning towards fixing all locations in this PR even though this will > prevent clean backports. Would that be ok? I think this is ok. - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-1999177986
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v2]
On Fri, 15 Mar 2024 08:00:50 GMT, Yi Yang wrote: > Maybe `dump` and `dump_to` Why not, I renamed the method to dump_to . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1999165924
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v6]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: rename dump_to_heapdump_path - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/daab7df9..b97921d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=04-05 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v5]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: fix COPYRIGHT info in HeapDumpJcmdPresetPathTest.java - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/61165f55..daab7df9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: 8327990: [macosx-aarch64] JFR enters VM without WXWrite
On Tue, 12 Mar 2024 15:05:00 GMT, Richard Reingruber wrote: > This pr changes `JfrJvmtiAgent::retransform_classes()` and `jfr_set_enabled` > to switch to `WXWrite` before transitioning to the vm. > > Testing: > make test TEST=jdk/jfr/event/runtime/TestClassLoadEvent.java > TEST_VM_OPTS=-XX:+AssertWXAtThreadSync > make test TEST=compiler/intrinsics/klass/CastNullCheckDroppingsTest.java > TEST_VM_OPTS=-XX:+AssertWXAtThreadSync > > More tests are pending. I noticed a few more asserts (assert(_wx_state == expected) failed: wrong state) in the jfr area (jdk tier3 jfr tests). E.g. V [libjvm.dylib+0x8a5d94] JavaThread::check_for_valid_safepoint_state()+0x0 V [libjvm.dylib+0x3e95b4] ThreadStateTransition::transition_from_native(JavaThread*, JavaThreadState, bool)+0x174 V [libjvm.dylib+0x3e93d0] ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*)+0x70 V [libjvm.dylib+0x91a578] JfrRecorderService::emit_leakprofiler_events(long long, bool, bool)+0xcc and V [libjvm.dylib+0x8a5d94] JavaThread::check_for_valid_safepoint_state()+0x0 V [libjvm.dylib+0x3e95b4] ThreadStateTransition::transition_from_native(JavaThread*, JavaThreadState, bool)+0x174 V [libjvm.dylib+0x3e93d0] ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*)+0x70 V [libjvm.dylib+0x8d7f74] JfrJavaEventWriter::flush(_jobject*, int, int, JavaThread*)+0xf8 j jdk.jfr.internal.JVM.flush(Ljdk/jfr/internal/event/EventWriter;II)V+0 jdk.jfr@23-internal j jdk.jfr.internal.event.EventWriter.flush(II)V+3 jdk.jfr@23-internal - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-1997635467
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
On Thu, 14 Mar 2024 13:43:09 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > add test HeapDumpJcmdPresetPathTest Hi Kevin, > (and btw regarding your comment on a test, yes I agree there should be a > separate test or at least an adjustment/addition to the existing tests) I added a test HeapDumpJcmdPresetPathTest.java . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1997514632
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: add test HeapDumpJcmdPresetPathTest - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/7420fb47..61165f55 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=02-03 Stats: 47 lines in 1 file changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: 8327990: [macosx-aarch64] JFR enters VM without WXWrite
On Wed, 13 Mar 2024 16:40:33 GMT, Richard Reingruber wrote: > @MBaesken found 2 more locations in jvmti that need switching to `WXWrite` > > JvmtiExport::get_jvmti_interface GetCarrierThread > > Both use `ThreadInVMfromNative`. Should we address those 2 more findings in this PR ? Or open a separate JBS issue ? btw those were the jtreg tests triggering the 2 additional findings / asserts runtime/Thread/AsyncExceptionOnMonitorEnter.java runtime/Thread/AsyncExceptionTest.java serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.java runtime/handshake/HandshakeDirectTest.java runtime/handshake/SuspendBlocked.java runtime/jni/terminatedThread/TestTerminatedThread.java runtime/lockStack/TestStackWalk.java serviceability/jvmti/vthread/GetThreadState/GetThreadStateTest.java#default serviceability/jvmti/vthread/GetThreadState/GetThreadStateTest.java#no-vmcontinuations serviceability/jvmti/vthread/GetThreadStateMountedTest/GetThreadStateMountedTest.java serviceability/jvmti/vthread/RawMonitorTest/RawMonitorTest.java serviceability/jvmti/vthread/SuspendWithInterruptLock/SuspendWithInterruptLock.java#default serviceability/jvmti/vthread/SuspendWithInterruptLock/SuspendWithInterruptLock.java#xint serviceability/jvmti/vthread/ThreadStateTest/ThreadStateTest.java serviceability/jvmti/vthread/WaitNotifySuspendedVThreadTest/WaitNotifySuspendedVThreadTest.java - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-1996996689
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v3]
On Thu, 14 Mar 2024 09:13:03 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust text related to HeapDumpPath in globals.hpp Hi Kevin, > globals.hpp documents HeapDumpPath as relating to HeapDumpOnOutOfMemoryError > (so that will need changing if this change is happening). I adjusted the related text in globals.hpp . Please check. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1996977356
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v3]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust text related to HeapDumpPath in globals.hpp - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/e335d9ee..7420fb47 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=01-02 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v2]
On Thu, 14 Mar 2024 02:39:28 GMT, Yi Yang wrote: > Looks reasonable, this is a harmless change, but the name > `dump_to_heapdump_path` looks too details(and somewhat strange) to me Do you maybe have a good suggestion for a new name ? - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1996891933
Integrated: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux]
On Fri, 8 Mar 2024 10:12:06 GMT, Matthias Baesken wrote: > There are a number of places remaining in the linux/macOS native JDK codebase > where we use the RESTARTABLE macro with close, but this is unwanted on > Linux/macOS. This pull request has now been integrated. Changeset: 481c866d Author: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/481c866df87c693a90a1da20e131e5654b084ddd Stats: 6 lines in 2 files changed: 0 ins; 3 del; 3 mod 8327468: Do not restart close if errno is EINTR [macOS/linux] Reviewed-by: dholmes, sspitsyn - PR: https://git.openjdk.org/jdk/pull/18164
Re: RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux] [v2]
On Mon, 11 Mar 2024 08:50:09 GMT, Matthias Baesken wrote: >> There are a number of places remaining in the linux/macOS native JDK >> codebase where we use the RESTARTABLE macro with close, but this is unwanted >> on Linux/macOS. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > do not throw exception on close error Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/18164#issuecomment-1996775638
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v2]
On Tue, 12 Mar 2024 13:27:13 GMT, Kevin Walls wrote: >That's not necessarily wrong, and may not be serious enough to refuse >-overwrite when using HeapDumpPath, but we want to >find a way of making things clear (I was not going to suggest separate >sequence numbers... 8-) ) Thanks for clarifying, maybe we have to describe/document this at some place (comment? or jcmd help ?) . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1991661956
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v2]
On Tue, 12 Mar 2024 11:08:01 GMT, Kevin Walls wrote:
>Does dump_to_heapdump_path() not print the ("Dumping heap to %s ...", path)
>message?
>That seems important when the user isn't specifying it directly.
The path is already printed, here is an example (the JVM with pid 219447 was
started with` -XX:HeapDumpPath=... `).
images/jdk/bin/jcmd 219447 GC.heap_dump
219447:
Dumping heap to /mydir/test/dumpdir/dydumpfile ...
Heap dump file created [3757116 bytes in 0.046 secs]
-
PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1991554323
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v2]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: alloc_and_create_heapdump_pathname adjust comment about freeing the returned pointer - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/116c0572..e335d9ee Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=00-01 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set
On Tue, 12 Mar 2024 11:08:01 GMT, Kevin Walls wrote: >jcmd GC.heap_dump has the overwrite flag. >OOM dumping in HeapDumper::dump_heap(bool oome) has a sequence number. There is (with this patch) still just one sequence number and it is incremented by all invocations of alloc_and_create_heapdump_pathname. >I like splitting out alloc_and_create_heapdump_pathname() as this is already a >large part of dump_heap. > Should the comment say, "caller must free the returned pointer". Agree, I will adjust the comment. (and btw regarding your comment on a test, yes I agree there should be a separate test or at least an adjustment/addition to the existing tests) - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1991510708 PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1991514153
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set
On Mon, 11 Mar 2024 11:57:04 GMT, Matthias Baesken wrote: > Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Hi Kevin, thanks for the comments. > globals.hpp documents HeapDumpPath as relating to HeapDumpOnOutOfMemoryError Yes true, probably we have to adjust the related text. What do you think about this old "When HeapDumpOnOutOfMemoryError is on," new "When HeapDumpOnOutOfMemoryError is on or a heap dump is trigger by jcmd GC.heap_dump without specifying a path," ? - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1991504498
Re: RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux] [v2]
On Tue, 12 Mar 2024 06:12:53 GMT, David Holmes wrote: > This seems fine to me, but please ensure someone from serviceability also > approves it. > Hi David, thanks for the review ! Someone from Serviceability Group interested to comment/review ? - PR Comment: https://git.openjdk.org/jdk/pull/18164#issuecomment-1990969018
Re: RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux] [v2]
On Mon, 11 Mar 2024 08:50:09 GMT, Matthias Baesken wrote: >> There are a number of places remaining in the linux/macOS native JDK >> codebase where we use the RESTARTABLE macro with close, but this is unwanted >> on Linux/macOS. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > do not throw exception on close error Looks like the return code of close is mostly (98-99 % ?) ignored in the JDK codebase. So probably we should follow for now the usual approach in this PR too. Btw. there are some places in the codebase where the return code of close is checked/handled e.g. https://github.com/openjdk/jdk/blob/f2b5ffdb8ea3c766f14bab1dfd7c3865cffa2ce8/src/java.base/unix/native/libnet/SdpSupport.c#L109 or https://github.com/openjdk/jdk/blob/f2b5ffdb8ea3c766f14bab1dfd7c3865cffa2ce8/src/java.base/unix/native/libnio/ch/UnixDispatcher.c#L37 - PR Comment: https://git.openjdk.org/jdk/pull/18164#issuecomment-1987915306
Re: RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux] [v2]
> There are a number of places remaining in the linux/macOS native JDK codebase > where we use the RESTARTABLE macro with close, but this is unwanted on > Linux/macOS. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: do not throw exception on close error - Changes: - all: https://git.openjdk.org/jdk/pull/18164/files - new: https://git.openjdk.org/jdk/pull/18164/files/32d05ccd..7ed2cc07 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18164=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18164=00-01 Stats: 19 lines in 2 files changed: 0 ins; 16 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18164.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18164/head:pull/18164 PR: https://git.openjdk.org/jdk/pull/18164
Re: RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux]
On Fri, 8 Mar 2024 10:24:54 GMT, Alan Bateman wrote:
>> There are a number of places remaining in the linux/macOS native JDK
>> codebase where we use the RESTARTABLE macro with close, but this is unwanted
>> on Linux/macOS.
>
> src/jdk.attach/linux/native/libattach/VirtualMachineImpl.c line 200:
>
>> 198: if (res == -1) {
>> 199: JNU_ThrowIOExceptionWithLastError(env, "close");
>> 200: }
>
> I assume it would be better to not throw when errno is EINTR. If there is a
> profiler or some other tool firing signals at threads then there isn't
> anything that can be done here.
Should I just ignore the result / return code of close and avoid throwing an
exception completely ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18164#discussion_r1517639119
RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux]
There are a number of places remaining in the linux/macOS native JDK codebase where we use the RESTARTABLE macro with close, but this is unwanted on Linux/macOS. - Commit messages: - JDK-8327468 Changes: https://git.openjdk.org/jdk/pull/18164/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18164=00 Issue: https://bugs.openjdk.org/browse/JDK-8327468 Stats: 16 lines in 2 files changed: 13 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18164.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18164/head:pull/18164 PR: https://git.openjdk.org/jdk/pull/18164
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 [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 [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]
> 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: 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: 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: 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
Integrated: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 12:30:15 GMT, Matthias Baesken wrote: > The get_total_or_available_swap_space_size coding misses AIX support, we only > return 0. This should be enhanced. > The perfstat API can be used, see > https://www.ibm.com/docs/pt/aix/7.2?topic=interfaces-perfstat-memory-total-interface > . This pull request has now been integrated. Changeset: 33324a59 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/33324a59ccdb220250cb74e15ce13af0e99dcb07 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod 8324637: [aix] Implement support for reporting swap space in jdk.management Reviewed-by: kevinw, stuefe - PR: https://git.openjdk.org/jdk/pull/17569
Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 14:25:51 GMT, Thomas Stuefe wrote: > Do we need the cast? perfstat_memory_total_t members are all 64-bit, no? > > Also, can we shorten this to: > > ``` > return (available ? memory_info.pgsp_free : memory_info.pgsp_total) * 4096; > ``` Hi Thomas, I see no types defined here https://www.ibm.com/docs/pt/aix/7.2?topic=interfaces-perfstat-memory-total-interface but most likely you are correct and they are 64bit. - PR Review Comment: https://git.openjdk.org/jdk/pull/17569#discussion_r1466552393
Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 14:11:35 GMT, Kevin Walls wrote: > I can't try this and don't use AIX, but it looks good. It follows the same > pattern as the other AIX cases in the file. > > Although the others (e.g. line 200) don't throw_internal_error if the call > returns -1, they just return -1. You might want to do that for consistency of > behaviour. Hi I wanted to be consistent to the other OS in get_total_or_available_swap_space_size, that#s why the throw_internal_error . - PR Comment: https://git.openjdk.org/jdk/pull/17569#issuecomment-1910449037
RFR: JDK-8324637: get_total_or_available_swap_space_size no support on AIX
The get_total_or_available_swap_space_size coding misses AIX support, we only return 0. This should be enhanced. The perfstat API can be used, see https://www.ibm.com/docs/pt/aix/7.2?topic=interfaces-perfstat-memory-total-interface . - Commit messages: - JDK-8324637 Changes: https://git.openjdk.org/jdk/pull/17569/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17569=00 Issue: https://bugs.openjdk.org/browse/JDK-8324637 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17569.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17569/head:pull/17569 PR: https://git.openjdk.org/jdk/pull/17569
