Re: RFR: 8352251: Implement Cooperative JFR Sampling [v14]
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: include guards ------------- Changes: - all: https://git.openjdk.org/jdk/pull/24296/files - new: https://git.openjdk.org/jdk/pull/24296/files/ed57ed9f..c65bc8b6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24296&range=13 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24296&range=12-13 Stats: 57 lines in 4 files changed: 10 ins; 47 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/24296.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24296/head:pull/24296 PR: https://git.openjdk.org/jdk/pull/24296
On Sun, 27 Apr 2025 21:58:32 GMT, Markus Grönlund <mgronlun@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:
include guards
Naming is hard here. The functions are determined by their inputs (fp or sp). But we can try to come up with more descriptive names. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2836702603
On Sun, 27 Apr 2025 21:58:32 GMT, Markus Grönlund <mgronlun@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:
include guards
Ok for now. I have a version which has passed jtreg:test/jdk/jdk/jfr tests. Not sure if this is what we want, but feel free to pick it up if you want to repair PPC64: [PPC64_JFR.patch](https://github.com/user-attachments/files/19949028/PPC64_JFR.patch) ------------- PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2836725707
On Mon, 28 Apr 2025 21:23:04 GMT, Martin Doerr <mdoerr@openjdk.org> wrote:
Ok for now. I have a version which has passed jtreg:test/jdk/jdk/jfr tests. Not sure if this is what we want, but feel free to pick it up if you want to repair PPC64: [PPC64_JFR.patch](https://github.com/user-attachments/files/19949028/PPC64_JFR.patch)
Thanks Martin. I think it needs some additional work: please take a look at InterpreterMacroAssembler::remove_activation() and InterpreterMacroAssembler::call_VM_with_sender_Java_fp() Can we let _last_sender_Java_fp be a state field that can be tested? You can see how it is tested in: jfrSampleRequest.cpp static bool build_from_ljf() ------------- PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2838056539
On Tue, 29 Apr 2025 09:17:40 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Can we let _last_sender_Java_fp be a state field that can be tested?
That is possible, but I don't see any effect. All JFR tests are already passing without that. Should `process_sender_Java_fp` be ever called on platforms which always have a valid Back Chain? I don't see anything which needs to be done for PPC64. We could return nullptr in `last_sender_Java_fp()`. The tests still pass with that. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2842060679
On Wed, 30 Apr 2025 13:50:53 GMT, Martin Doerr <mdoerr@openjdk.org> wrote:
Can we let _last_sender_Java_fp be a state field that can be tested?
I still couldn't hit any failures or errors with my simple version, but I understand that it might be problematic. I have an implementation: https://github.com/TheRealMDoerr/jdk/commit/b2f83fae262f129f864e109d7adce169... Please take a look. I hope we don't need more ;-) I'm planning to run more test when I find more time. If `_last_sender_Java_fp` is needed on all platforms, shouldn't it be better moved to shared javaFrameAnchor.hpp and javaThread.hpp? ------------- PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2851350228
On Sun, 27 Apr 2025 21:58:32 GMT, Markus Grönlund <mgronlun@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:
include guards
"frame_link" as a name looks better when inspecting the declaration, but in all usages it is redundant: frame::link() vs frame::frame_link() ------------- PR Comment: https://git.openjdk.org/jdk/pull/24296#issuecomment-2837900109
participants (2)
-
Markus Grönlund
-
Martin Doerr