Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Thomas Schatzl
On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev wrote: > `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC > phase is running, while it actually only covers the STW GCs. It would be good > to rename it for clarity. The freed-up name, `is_gc_active` could then be >

Re: RFR: 8330146: assert(!_thread->is_in_any_VTMS_transition()) failed

2024-05-02 Thread Alan Bateman
On Thu, 2 May 2024 22:40:02 GMT, Chris Plummer wrote: > It seems maybe we should instead be trying to avoid these events by > preloading the classes as was suggested as an option in the CR. I don't think preloading PinnedThreadPrinter will solve it completely. First usage could potentially loa

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v6]

2024-05-02 Thread Thomas Schatzl
On Fri, 3 May 2024 06:21:54 GMT, Lei Zaakjyu wrote: >> And here also it seems you agreed with this suggestion but the change was >> never made. > > I think we can make these changes in later PRs in order to avoid making this > one even larger. As mentioned earlier, I also think the SA changes

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Alan Bateman
On Thu, 2 May 2024 21:44:43 GMT, Chris Plummer wrote: >> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review: Corrections in: 1) JVMTI/JDWP spec; 2) test vthread checks; 3) >> test comments > > src/hotspot/share/prims

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v6]

2024-05-02 Thread Lei Zaakjyu
On Thu, 2 May 2024 21:32:50 GMT, Chris Plummer wrote: >> test/hotspot/jtreg/serviceability/sa/TestG1HeapRegion.java line 62: >> >>> 60: agent.attach(Integer.parseInt(pid)); >>> 61: G1CollectedHeap heap = >>> (G1CollectedHeap)VM.getVM().getUniverse().heap(); >>> 62:

Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v4]

2024-05-02 Thread Alex Menkov
> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method > > Testing: tier1-6 Alex Menkov has updated the pull request incrementally with three additional commits since the last revision: - update - Revert "renamed current_thread to current" This reverts commit d5d61

Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v3]

2024-05-02 Thread Alex Menkov
On Tue, 30 Apr 2024 23:48:02 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > renamed current_thread

Re: RFR: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly [v3]

2024-05-02 Thread Chris Plummer
On Tue, 30 Apr 2024 23:48:02 GMT, Alex Menkov wrote: >> Some cleanup related to JvmtiEnvBase::get_threadOop_and_JavaThread method >> >> Testing: tier1-6 > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > renamed current_thread

Re: RFR: 8330146: assert(!_thread->is_in_any_VTMS_transition()) failed

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 10:07:35 GMT, Serguei Spitsyn wrote: > Any event posting code except CFLH, ClassPrepare and ClassLoad events has a > conditional return in case if the event is posted during a VTMS transition. > The CFLH, ClassPrepare and ClassLoad event posting code has just an assert > in

Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Laurence Cable
diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/V irtualMachineImpl.java index 81d4fd259ed..74bd60c791d 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux

Re: RFR: 8308033: The jcmd thread dump related tests should test virtual threads [v2]

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 03:57:42 GMT, Jaikiran Pai wrote: >> test/hotspot/jtreg/serviceability/tmtools/jstack/DaemonThreadTest.java line >> 77: >> >>> 75: t.setDaemon(false); >>> 76: } >>> 77: testThread(t, ""); >> >> I think the following is a bit clearer: >> >> >> //

Re: RFR: 8308033: The jcmd thread dump related tests should test virtual threads [v3]

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 03:57:25 GMT, Jaikiran Pai wrote: >> Can I please get a review of these test-only changes which proposes to >> remove the `test/jdk/sun/tools/jcmd/JcmdOutputEncodingTest.java` and >> `test/jdk/sun/tools/jstack/BasicJStackTest.java` tests from >> `ProblemList-Virtual.txt`? >>

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 21:36:27 GMT, Chris Plummer wrote: >> Thank you for the comment. In fact, I don't know how to fix it. >> Replacing of NUMBER_OF_ENTERING_THREADS/NUMBER_OF_WAITING_THREADS in >> comments with `expEnteringCount/expWaitingCount` does not make sense to me. >> The comments are abo

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 07:33:09 GMT, Serguei Spitsyn wrote: >> The fix is to degrade virtual threads support in the JVM TI >> `GetObjectMonitorUsage` function so that it is specified to only return an >> owner when the owner is a platform thread. Also, virtual threads are not >> listed in the both

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Chris Plummer
On Wed, 1 May 2024 20:42:16 GMT, Chris Plummer wrote: >> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review: Corrections in: 1) JVMTI/JDWP spec; 2) test vthread checks; 3) >> test comments > > src/java.se/share/data/

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 06:45:39 GMT, Serguei Spitsyn wrote: >> test/hotspot/jtreg/serviceability/jvmti/ObjectMonitorUsage/ObjectMonitorUsage.java >> line 257: >> >>> 255: // Correct the expected values for the virtual thread case. >>> 256: int expEnteringCount = isVirtual ?

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v6]

2024-05-02 Thread Chris Plummer
On Sat, 20 Apr 2024 03:04:23 GMT, Chris Plummer wrote: >> Lei Zaakjyu has updated the pull request incrementally with one additional >> commit since the last revision: >> >> review > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerLocation.java > line 131: > >> 129:

Re: RFR: 8328866: Add raw monitor rank support to the debug agent.

2024-05-02 Thread Chris Plummer
On Thu, 2 May 2024 02:40:36 GMT, Alex Menkov wrote: >> This PR adds ranked monitor support to the debug agent. The debug agent has >> a large number of monitors, and it's really hard to know which order to grab >> them in, and for that matter which monitors might already be held at any >> give

Re: RFR: 8322043: HeapDumper should use parallel dump by default [v6]

2024-05-02 Thread Alex Menkov
On Tue, 30 Apr 2024 18:54:09 GMT, Alex Menkov wrote: >> The fix makes VM heap dumping parallel by default. >> `jcmd GC.heap_dump` and `jmap -dump` had parallel dumping by default, the >> fix affects `HotSpotDiagnosticMXBean.dumpHeap()`, >> `-XX:+HeapDumpBeforeFullGC`, `-XX:+HeapDumpAfterFullGC`

Re: RFR: 8328866: Add raw monitor rank support to the debug agent.

2024-05-02 Thread Alex Menkov
On Wed, 1 May 2024 21:32:46 GMT, Chris Plummer wrote: > This PR adds ranked monitor support to the debug agent. The debug agent has a > large number of monitors, and it's really hard to know which order to grab > them in, and for that matter which monitors might already be held at any > given

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Serguei Spitsyn
On Thu, 2 May 2024 07:33:09 GMT, Serguei Spitsyn wrote: >> The fix is to degrade virtual threads support in the JVM TI >> `GetObjectMonitorUsage` function so that it is specified to only return an >> owner when the owner is a platform thread. Also, virtual threads are not >> listed in the both

Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Zhengyu Gu
On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev wrote: > `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC > phase is running, while it actually only covers the STW GCs. It would be good > to rename it for clarity. The freed-up name, `is_gc_active` could then be >

Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Stefan Karlsson
On Thu, 2 May 2024 17:23:21 GMT, Aleksey Shipilev wrote: >> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1270: >> >>> 1268: >>> 1269: ParallelScavengeHeap* heap = ParallelScavengeHeap::heap(); >>> 1270: assert(!heap->is_stw_gc_active(), "not reentrant"); >> >> While reading thi

Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Aleksey Shipilev
On Thu, 2 May 2024 17:04:44 GMT, Stefan Karlsson wrote: >> `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ >> GC phase is running, while it actually only covers the STW GCs. It would be >> good to rename it for clarity. The freed-up name, `is_gc_active` could then >>

Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Stefan Karlsson
On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev wrote: > `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC > phase is running, while it actually only covers the STW GCs. It would be good > to rename it for clarity. The freed-up name, `is_gc_active` could then be >

Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Aleksey Shipilev
On Thu, 2 May 2024 16:56:11 GMT, Stefan Karlsson wrote: >> `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ >> GC phase is running, while it actually only covers the STW GCs. It would be >> good to rename it for clarity. The freed-up name, `is_gc_active` could then >>

Re: RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Stefan Karlsson
On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev wrote: > `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC > phase is running, while it actually only covers the STW GCs. It would be good > to rename it for clarity. The freed-up name, `is_gc_active` could then be >

RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

2024-05-02 Thread Aleksey Shipilev
`CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ GC phase is running, while it actually only covers the STW GCs. It would be good to rename it for clarity. The freed-up name, `is_gc_active` could then be repurposed to track any (concurrent or STW) GC phase running. That

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v6]

2024-05-02 Thread Lei Zaakjyu
> follow up 8267941 Lei Zaakjyu has updated the pull request incrementally with one additional commit since the last revision: review - Changes: - all: https://git.openjdk.org/jdk/pull/18871/files - new: https://git.openjdk.org/jdk/pull/18871/files/f4e066c4..92d0df4d Webrevs

Re: RFR: 8330694: Rename 'HeapRegion' to 'G1HeapRegion' [v5]

2024-05-02 Thread Lei Zaakjyu
> follow up 8267941 Lei Zaakjyu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - Merge branch 'master' into JDK-8330694 - fix indentation - also tidy up - tidy up - rename - Changes: https://git.openj

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Alan Bateman
On Thu, 2 May 2024 07:33:09 GMT, Serguei Spitsyn wrote: >> The fix is to degrade virtual threads support in the JVM TI >> `GetObjectMonitorUsage` function so that it is specified to only return an >> owner when the owner is a platform thread. Also, virtual threads are not >> listed in the both

RE: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Doyle, James, K
Thank you, Larry - that /proc//ns/mnt test makes a lot of sense to me, and I learned something new today! Jim -Original Message- From: serviceability-dev On Behalf Of Laurence Cable Sent: Wednesday, May 1, 2024 8:44 PM To: serviceability-dev@openjdk.org Subject: Re: 8327114: Attach in

RFR: 8331557: Serial: Refactor SerialHeap::do_collection

2024-05-02 Thread Albert Mingkun Yang
It's probably easier to read the new code directly. The two classes in `serialVMOperations` serve as entrance points to invoke young/full GCs. Some previously hidden decisions are made more obvious, e.g. if a young-gc fails (or will probablly fail), fallback to full-gc. Additionally, `StatRecor

Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Sebastian Lövdahl
On Thu, 2 May 2024 10:13:51 GMT, Sebastian Lövdahl wrote: > 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid > (Kubernetes debug container) Ran the following tests locally: $ make test TEST="jtreg:test/hotspot/jtreg/containers" ... == Test sum

Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Laurence Cable
using pid to namespace comparison is IMO inappropriate/misleading what is being tested is the sharing of a common mount namespace, therefore the test should be comparing the "mnt" namespace ids. Rgds - Larry On 5/2/24 3:22 AM, Sebastian Lövdahl wrote: On Thu, 2 May 2024 10:13:51 GMT, Sebasti

Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Sebastian Lövdahl
On Thu, 2 May 2024 10:13:51 GMT, Sebastian Lövdahl wrote: > 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid > (Kubernetes debug container) This is a first stab at fixing the regression introduced in #17628. There has been a bit of discussion in https://mail.openjdk.org/pi

Re: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Laurence Cable
On 5/2/24 3:09 AM, Sebastian Lövdahl wrote: Interesting, TIL about /proc//ns. I tried to look for something like that but couldn't find anything relevant in /proc//status. ok So, a pixel perfect solution could compare these IDs to know whether /tmp or /proc//root/tmp should be used. >

RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Sebastian Lövdahl
8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) - Commit messages: - 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) Changes: https://git.openjdk.org/jdk/pull/19055/files Webrev:

RFR: 8330146: assert(!_thread->is_in_any_VTMS_transition()) failed

2024-05-02 Thread Serguei Spitsyn
Any event posting code except CFLH, ClassPrepare and ClassLoad events has a conditional return in case if the event is posted during a VTMS transition. The CFLH, ClassPrepare and ClassLoad event posting code has just an assert instead. The ClassPrepare and ClassLoad events also have a condition

Re: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-05-02 Thread Sebastian Lövdahl
Interesting, TIL about /proc//ns. I tried to look for something like that but couldn't find anything relevant in /proc//status. So, a pixel perfect solution could compare these IDs to know whether /tmp or /proc//root/tmp should be used. > 2. jcmd treats it as a heuristic and attempts each way

Re: RFR: 8322043: HeapDumper should use parallel dump by default [v6]

2024-05-02 Thread Aleksey Shipilev
On Tue, 30 Apr 2024 18:54:09 GMT, Alex Menkov wrote: >> The fix makes VM heap dumping parallel by default. >> `jcmd GC.heap_dump` and `jmap -dump` had parallel dumping by default, the >> fix affects `HotSpotDiagnosticMXBean.dumpHeap()`, >> `-XX:+HeapDumpBeforeFullGC`, `-XX:+HeapDumpAfterFullGC`

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v2]

2024-05-02 Thread Serguei Spitsyn
> The fix is to degrade virtual threads support in the JVM TI > `GetObjectMonitorUsage` function so that it is specified to only return an > owner when the owner is a platform thread. Also, virtual threads are not > listed in the both `waiters` and `notify_waiters` lists returned in the > `jvmt

Re: RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage

2024-05-02 Thread Serguei Spitsyn
On Wed, 1 May 2024 21:01:16 GMT, Chris Plummer wrote: >> The fix is to degrade virtual threads support in the JVM TI >> `GetObjectMonitorUsage` function so that it is specified to only return an >> owner when the owner is a platform thread. Also, virtual threads are not >> listed in the both `