Update jvmtiGetThreadGroupChildren for java 19 #15539

EricYangIBM · 2022-07-13T19:21:17Z

The ThreadGroup class does not store its threads anymore and child groups
are separated into "groups" and "weaks" ThreadGroup arrays.

The child ThreadGroups array is populated from the ThreadGroup's "groups"
and "weaks" arrays, which are the child ThreadGroups that are strongly
and weakly reachable from the parent.

The threads array is constructed by iterating over all J9VMThreads and
adding the threads that belong in the given ThreadGroup.

The returned arrays are constructed while locking on the ThreadGroup
object.

Fixes: #15244
Issue: #15183
Signed-off-by: Eric Yang eric.yang@ibm.com

EricYangIBM · 2022-07-19T16:37:14Z

Test in linked issue passes with these changes

babsingh

The commit message/description should include few lines on how the threads and groups arrays are created for Java 19+. It will make it easier to follow the source code.

runtime/jvmti/jvmtiThreadGroup.c

babsingh · 2022-07-20T14:28:10Z

runtime/jvmti/jvmtiThreadGroup.c

+		} while ((targetThread = targetThread->linkNext) != vm->mainThread);
+
+		*rv_thread_count = nLiveThreads;
+		*rv_threads = threads;


On return, the jthread* points to a newly allocated array of size*thread_count_ptr

threads array is of size totalThreadCount which is greater than nLiveThreads. Is it fine to return an array of larger size? We allow this in our current implementation; the answer may be "yes".

We return the actual number of threads in the array so it probably is fine

A lot of allocated memory will be unused if the filter excludes a lot of threads. Filter: include only platform threads which match the thread_group.

babsingh · 2022-07-20T14:30:31Z

runtime/jvmti/jvmtiThreadGroup.c

+		*rv_thread_count = nLiveThreads;
+		*rv_threads = threads;
+		*rv_group_count = nGroupsTotal;
+		*rv_groups = groups;


On return, the jthreadGroup* points to a newly allocated array of size*group_count_ptr

groups array is of size nGroups + nWeaks, which is greater than nGroupsTotal. Is it fine to return an array of larger size? We allow this in our current implementation; the answer may be "yes".

A lot of allocated memory will be unused if a lot of weak references are null.

babsingh · 2022-07-20T19:09:35Z

@EricYangIBM The commits can be squashed.

@tajila These changes should work fine. But, still doubtful about the memory allocated for threads and groups arrays which are returned from this method (see comments inlined in code). Should we create new arrays with smaller/required size and copy the elements from the bigger array and free the bigger array?

runtime/jvmti/jvmtiThreadGroup.c

tajila · 2022-08-04T18:46:22Z

But, still doubtful about the memory allocated for threads and groups arrays which are returned from this method (see comments inlined in code). Should we create new arrays with smaller/required size and copy the elements from the bigger array and free the bigger array?

This is not going to return vthreads correct? If so, then I dont think returning a potentially larger buffer is a big deal as platform threads arent created as frequently

tajila · 2022-08-04T18:46:33Z

It could be an optimization that is added later

babsingh · 2022-08-04T18:55:21Z

This is not going to return vthreads correct?

Vthreads will not be returned. See ref below. @EricYangIBM Can you add a comment in code and open an issue regarding this optimization for tracking purposes?

Ref: https://download.java.net/java/early_access/jdk19/docs/specs/jvmti.html#GetThreadGroupChildren

EricYangIBM · 2022-08-04T19:07:47Z

I will open the issue after this PR is merged (to link to the comment)

babsingh · 2022-08-04T19:13:31Z

LGTM. The commits need to be squashed before the PR builds are launched.

The ThreadGroup class does not store its threads anymore and child groups are separated into "groups" and "weaks" ThreadGroup arrays. The child ThreadGroups array is populated from the ThreadGroup's "groups" and "weaks" arrays, which are the child ThreadGroups that are strongly and weakly reachable from the parent. The threads array is constructed by iterating over all J9VMThreads and adding the threads that belong in the given ThreadGroup. The returned arrays are constructed while locking on the ThreadGroup object. Fixes: eclipse-openj9#15244 Issue: eclipse-openj9#15183 Signed-off-by: Eric Yang <eric.yang@ibm.com>

EricYangIBM · 2022-08-04T19:14:58Z

Squashed

babsingh · 2022-08-04T19:16:45Z

jenkins compile win jdk19,jdk17

babsingh · 2022-08-04T19:17:33Z

Does not work for me yet. Need to wait for @AdamBrousseau to add me.

babsingh · 2022-08-05T14:56:16Z

jenkins compile win jdk19,jdk17

babsingh · 2022-08-05T15:01:04Z

jenkins test sanity amac jdk19,jdk17

babsingh · 2022-08-05T17:06:34Z

These JVMTI changes should only impact the gtgc001 test, which has been enabled and passes for JDK17 and JDK19.

https://openj9-jenkins.osuosl.org/job/Test_openjdk17_j9_sanity.functional_aarch64_mac_Personal/75/

jit_vich_0_FAILED is unrelated to this PR. All tests in this suite pass but there is an error at the end (Invalid JIT return address).

===============================================
JIT_Test suite
Total tests run: 18, Failures: 0, Skips: 0
===============================================

*** Invalid JIT return address 000000037A4175E0 in 000000016FBDFD98

jit_vich_0_FAILED

babsingh

LGTM. Opened #15673 for the JIT failure.

babsingh · 2022-08-05T17:11:12Z

Still can't merge the PR. Need to wait for @AdamBrousseau to give the required permissions.

AdamBrousseau · 2022-08-05T17:18:51Z

Still can't merge the PR

This is an Eclipse Github team thing. It should be automatic. If it's not working then I would open an Eclipse ticket.
https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues?sort=created_date&state=opened

GetThreadGroupChildren was fixed by eclipse-openj9#15539. The above PR removed the exclude for JDK19. But, the exclude for JDK20 was not removed. Related: eclipse-openj9#15244 Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>

EricYangIBM force-pushed the getThreadGroupChildren branch 2 times, most recently from cab5162 to c41c9d9 Compare July 13, 2022 20:36

EricYangIBM marked this pull request as ready for review July 19, 2022 16:37

babsingh self-requested a review July 19, 2022 16:38

babsingh requested changes Jul 20, 2022

View reviewed changes

EricYangIBM force-pushed the getThreadGroupChildren branch from 5f21af4 to 7460947 Compare July 20, 2022 17:15

EricYangIBM force-pushed the getThreadGroupChildren branch from 7460947 to 7150903 Compare July 21, 2022 14:54

babsingh mentioned this pull request Jul 22, 2022

Loom: JVMTI support #15183

Closed

EricYangIBM commented Jul 27, 2022

View reviewed changes

runtime/jvmti/jvmtiThreadGroup.c Outdated Show resolved Hide resolved

tajila reviewed Aug 4, 2022

View reviewed changes

runtime/jvmti/jvmtiThreadGroup.c Outdated Show resolved Hide resolved

EricYangIBM force-pushed the getThreadGroupChildren branch from 50f8122 to 342dc03 Compare August 4, 2022 19:06

EricYangIBM force-pushed the getThreadGroupChildren branch from 342dc03 to 077da24 Compare August 4, 2022 19:14

babsingh approved these changes Aug 5, 2022

View reviewed changes

babsingh merged commit d31ce9c into eclipse-openj9:master Aug 10, 2022

EricYangIBM deleted the getThreadGroupChildren branch August 11, 2022 12:53

EricYangIBM mentioned this pull request Aug 12, 2022

Jvmti GetThreadGroupChildren allocates more memory than necessary #15706

Open

babsingh mentioned this pull request Nov 21, 2022

Re-enable gtgc001 on JDK20 #16345

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update jvmtiGetThreadGroupChildren for java 19 #15539

Update jvmtiGetThreadGroupChildren for java 19 #15539

EricYangIBM commented Jul 13, 2022 •

edited

Loading

EricYangIBM commented Jul 19, 2022

babsingh left a comment •

edited

Loading

babsingh Jul 20, 2022

EricYangIBM Jul 20, 2022

babsingh Jul 20, 2022

babsingh Jul 20, 2022

babsingh Jul 20, 2022

babsingh commented Jul 20, 2022

tajila commented Aug 4, 2022

tajila commented Aug 4, 2022

babsingh commented Aug 4, 2022

EricYangIBM commented Aug 4, 2022

babsingh commented Aug 4, 2022

EricYangIBM commented Aug 4, 2022

babsingh commented Aug 4, 2022

babsingh commented Aug 4, 2022

babsingh commented Aug 5, 2022

babsingh commented Aug 5, 2022

babsingh commented Aug 5, 2022

babsingh left a comment

babsingh commented Aug 5, 2022

AdamBrousseau commented Aug 5, 2022

Update jvmtiGetThreadGroupChildren for java 19 #15539

Update jvmtiGetThreadGroupChildren for java 19 #15539

Conversation

EricYangIBM commented Jul 13, 2022 • edited Loading

EricYangIBM commented Jul 19, 2022

babsingh left a comment • edited Loading

Choose a reason for hiding this comment

babsingh Jul 20, 2022

Choose a reason for hiding this comment

EricYangIBM Jul 20, 2022

Choose a reason for hiding this comment

babsingh Jul 20, 2022

Choose a reason for hiding this comment

babsingh Jul 20, 2022

Choose a reason for hiding this comment

babsingh Jul 20, 2022

Choose a reason for hiding this comment

babsingh commented Jul 20, 2022

tajila commented Aug 4, 2022

tajila commented Aug 4, 2022

babsingh commented Aug 4, 2022

EricYangIBM commented Aug 4, 2022

babsingh commented Aug 4, 2022

EricYangIBM commented Aug 4, 2022

babsingh commented Aug 4, 2022

babsingh commented Aug 4, 2022

babsingh commented Aug 5, 2022

babsingh commented Aug 5, 2022

babsingh commented Aug 5, 2022

babsingh left a comment

Choose a reason for hiding this comment

babsingh commented Aug 5, 2022

AdamBrousseau commented Aug 5, 2022

EricYangIBM commented Jul 13, 2022 •

edited

Loading

babsingh left a comment •

edited

Loading