RFR: 8352251: Implement JEP 518: JFR Cooperative Sampling [v23]

Markus Grönlund mgronlun at openjdk.org
Thu May 15 20:35:57 UTC 2025


On Thu, 15 May 2025 16:55:30 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   growable array constructur without default initialization
>
> src/hotspot/cpu/x86/interp_masm_x86.cpp line 782:
> 
>> 780: void InterpreterMacroAssembler::call_VM_with_sender_Java_fp(Register fp_reg, Register tmp, address entry_point, bool store_bcp) {
>> 781:   if (store_bcp) {
>> 782:     save_bcp(fp_reg); // We must save the bcp, but need not restore it. This is because we have already popped the fp.
> 
> Why do we need to save the bcp? We already removed the frame from the sampler perspective.

It's needed for the stack walking code as part of InterpreterRuntime::at_unwind(), the last java frame (ljf) still points to the unpopped frame.

The ljf is used by both JFR_ONLY(Jfr::check_and_process_sample_request(current);) and later   StackWatermarkSet::before_unwind(current);

> src/hotspot/share/jfr/periodic/sampling/jfrSampleRequest.cpp line 187:
> 
>> 185:     request._sample_sp = request._sample_bcp;
>> 186:     // Get real bcp.
>> 187:     void* const bcp = interpreter_frame_bcp(request);
> 
> Shouldn’t we get the bcp from the context instead? The one saved in the frame is not updated on each bytecode.

That would, of course, be ideal, but I don't think there exists any abstraction to read the current bcp from the ucontext_t, abstracting it for all platforms.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2091914816
PR Review Comment: https://git.openjdk.org/jdk/pull/24296#discussion_r2091918747


More information about the hotspot-jfr-dev mailing list