RFR: 8352251: Implement Cooperative JFR Sampling [v4]

[Jaroslav Bachorik]

On Mon, 7 Apr 2025 10:53:32 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> Greetings,
>> 
>> This is the implementation of JEP [JDK-8350338 Cooperative JFR Sampling](https://bugs.openjdk.org/browse/JDK-8350338).
>> 
>> Implementations in this change set are provided and have been tested on the following platforms:
>> 
>> - windows-x64
>> - windows-x64-debug
>> - linux-x64
>> - linux-x64-debug
>> - macosx-x64
>> - macosx-x64-debug
>> - linux-aarch64
>> - linux-aarch64-debug
>> - macosx-aarch64
>> - macosx-aarch64-debug
>> 
>> Testing: tier1-6, jdk_jfr, stress testing.
>> 
>> Platform porters note:
>> Some platform-specific code needs to be provided, mainly in the interpreter. Take a look at the following files for changes:
>> 
>> - src/hotspot/cpu/x86/frame_x86.cpp
>> - src/hotspot/cpu/x86/interp_masm_x86.cpp
>> - src/hotspot/cpu/x86/interp_masm_x86.hpp
>> - src/hotspot/cpu/x86/javaFrameAnchor_x86.hpp
>> - src/hotspot/cpu/x86/macroAssembler_x86.cpp
>> - src/hotspot/cpu/x86/macroAssembler_x86.hpp
>> - src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp
>> - src/hotspot/cpu/x86/templateTable_x86.cpp
>> - src/hotspot/os_cpu/linux_x86/javaThread_linux_x86.hpp
>> 
>> Thanks
>> Markus
>
> Markus Grönlund 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 8352251
>  - align params
>  - adjustments
>  - refactoring
>  - 8352251

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp line 73:

> 71: }
> 72: 
> 73: // A sampled interpreter frame is handled differenly compared to a sampled compiler frame.

Suggestion:

// A sampled interpreter frame is handled differently compared to a sampled compiler frame.

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp line 82:

> 80:   assert(jt->has_last_Java_frame(), "invariant");
> 81: 
> 82:   // For a request representing an interpreter frame, request._sample_sp is actually the frame pointer, fp.

:)

src/hotspot/share/jfr/recorder/service/jfrEventThrottler.cpp line 70:

> 68: // There is currently only two throttler instance, one for the jdk.ObjectAllocationSample event
> 69: // and another for the SamplingLatency event.
> 70: // When introducing many more throttlers, consider addomg a lookup map keyed by event id.

Suggestion:

// When introducing many more throttlers, consider adding a lookup map keyed by event id.

src/hotspot/share/jfr/recorder/stacktrace/jfrStackTrace.cpp line 114:

> 112:   _written = true;
> 113: }
> 114: 

`JfrStackFrame::equals()` is already defined in jfrStackFrame.cpp

src/hotspot/share/jfr/recorder/stacktrace/jfrVframeStream.hpp line 2:

> 1: /*
> 2:  * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.

Why are the copyright years different here?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2050656607
PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2050657042
PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2050765959
PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2050770381
PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2050775884