RFR: 8301380: jdk/jfr/api/consumer/streaming/TestCrossProcessStreaming.java [v2]

[David Holmes]

On Thu, 2 Feb 2023 15:34:14 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> Greetings,
>> 
>> please help review this small adjustment to close the gap where iterated threads that attach via jni are left without a valid JFR thread id. For details, please see the JIRA issue.
>> 
>> Testing: jdk_jfr
>> 
>> Thanks
>> Markus
>
> Markus Grönlund has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - typos
>  - unconditional id assignment

Code changes look good. A couple of suggested comment changes to explain the reasoning more clearly. Thanks.

src/hotspot/share/prims/jni.cpp line 3468:

> 3466: static void post_thread_start_event(const JavaThread* jt) {
> 3467:   assert(jt != nullptr, "invariant");
> 3468:   // We hoist the read for the thread id to ensure the thread is assigned an id (lazy assignment).

Suggestion:

// Ensure the thread is assigned an id (lazy assignment) even if the event isn't committed.

src/hotspot/share/prims/jni.cpp line 3841:

> 3839:   }
> 3840: 
> 3841:   // Please keep this inside the _attaching_via_jni section.

Suggestion:

// Ensure the thread_id is set whilst still `_attaching_via_jni` so that it can be seen
// once the thread is included by the JFR thread iterator.

?

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12388