RFR: 8352738: Implement JEP 520: JFR Method Timing and Tracing [v8]

[Aleksey Shipilev]

On Wed, 28 May 2025 15:11:14 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:

>> Could I have a review of this enhancement that will add tracing capabilities to JFR? There are opportunities for performance improvements in the implementation, but I would rather add them later and separately.
>> 
>> Testing: tier 1-3, test/jdk/jdk/jfr
>> 
>> Thanks
>> Erik
>
> Erik Gahlin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge
>  - Fix comment
>  - Improve comment
>  - Remove METHOD_FLAG_USED_PREVIOUS_EPOCH_BIT(method)
>  - Merge
>  - Atomic::load_acquire(&_method_tracer_state);
>  - Reviewer feedback
>  - Initial

Looks like some of my comments were not fully addressed, these ones that I caught on second read:

src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp line 215:

> 213:   assert(JdkJfrEvent::is_a(ik), "invariant");
> 214:   if (has_annotation(ik, annotation_type, default_value, value)) {
> 215:      return true;

Unnecessary change?

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.cpp line 72:

> 70: void JfrTraceIdEpoch::set_method_tracer_tag_state() {
> 71:   assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
> 72:   Atomic::store(&_method_tracer_state, true);

Well, now that `has_method_tracer_changed_tag_state` does `Atomic::load_acquire`, the stores should be `Atomic::release_store` at very least? The fact these are done under the CLDG lock is not really enough.

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

PR Review: https://git.openjdk.org/jdk/pull/25306#pullrequestreview-2875844744
PR Review Comment: https://git.openjdk.org/jdk/pull/25306#discussion_r2112382006
PR Review Comment: https://git.openjdk.org/jdk/pull/25306#discussion_r2112387605