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