RFR: 8352251: Implement Cooperative JFR Sampling [v12]

[Markus Grönlund]

On Thu, 24 Apr 2025 09:53:38 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 incrementally with one additional commit since the last revision:
> 
>   Attempt to build Windows-AARCH64

> Hi, I've taken a quick look and wonder what needs to be done for PPC64 which has some fundamental differences.
> 
> We never need to pass an FP because it can always be retrieved like this via Back Chain (required by "Power Architecture 64-Bit ELF V2 ABI Specification"): FP = *SP So, I guess we could use something like
> 
> ```diff
> diff --git a/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp b/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp
> index 8b539bc8101..7d07c302b36 100644
> --- a/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp
> +++ b/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp
> @@ -73,6 +73,10 @@
>  
>    address last_Java_pc(void)          { return _last_Java_pc; }
>  
> +  intptr_t* last_Java_fp() const      { return *(intptr_t**)_last_Java_sp; }
> +
> +  intptr_t* last_sender_Java_fp() const { return **(intptr_t***)_last_Java_sp; }
> +
>    void set_last_Java_sp(intptr_t* sp) { OrderAccess::release(); _last_Java_sp = sp; }
>  
>  #endif // CPU_PPC_JAVAFRAMEANCHOR_PPC_HPP
> ```
> 
> This frame constructor does not exist on PPC64:
> 
> https://github.com/openjdk/jdk/blob/771a397a12a3bcb25af1f508061fd928d8f13c2e/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp#L174
> 
> We have one with different argument order and could emulate it like this:
> 
> ```c++
> inline frame::frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* sender_sp, address pc, CodeBlob* cb) :
>   frame(sp, pc, unextended_sp, sender_sp, cb) {}
> ```
> 
> Strange if we really need another one which only turns the argument order.
> 
> The following constants do not exist:
> 
> ```c++
>     // Used by JFR. Unfortunately, we can't use offset_of because it's not constexpr.
>     ijava_state_slots                      = ijava_state_size >> LogBytesPerWord,
>     // Members of ijava_state:
>     interpreter_frame_method_offset        = -ijava_state_slots + 0,
>     interpreter_frame_bcp_offset           = -ijava_state_slots + 5,
>     interpreter_frame_sender_sp_offset     = -ijava_state_slots + 9,
>     // Stuff which shouldn't be used on this platform:
>     interpreter_frame_initial_sp_offset    = -ijava_state_slots,
>     link_offset                            = 0,
>     sender_sp_offset                       = 0,
>     // Member of common_abi: lr
>     return_addr_offset                     = 2,
> ```
> 
> I hate adding them. Could we move the usages into platform code?

Hi Martin,

On the other platforms, the walker code only uses _last_sender_Java_fp() if it is set. And it is only set by the sensitive Interpreter safe point polls just before frame pop. We would like something that is a value to be tested, which can be set using the mechanism you suggest, but only for those sensitive poll sites.

I am moving support routines that rely on platform-specific constants into platform code. Stay tuned.

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

PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2833538638