On Sat, 7 Sep 2024 00:07:27 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:
>> This PR is successor of #20789 . I got some comments in there, then I needed >> to fix in many pooints. And also I have to fix branch name to kick GHA >> automatically (#20789 is named with `pr/`, it meets the condition to skip >> GHA). Hence I've opened another PR for this JBS issue. >> >> This PR has been updated with about topics since #20789: >> * Use `JavaFrameAnchor` instead of raw frame pointer to unwind frame of >> `UpcallStub`. >> * The change happens x86 (includes AMD64), aarch64, PPC64, RISC-V 64 only - >> s390 is out of scope because SA does not have s390 implementation. >> * Only both AMD64 and aarch64 have tested on GHA. >> * Refactor testcase to meet expected condition certainly. > > Yasumasa Suenaga has updated the pull request incrementally with two > additional commits since the last revision: > > - Update test/hotspot/jtreg/serviceability/sa/libupcall.c > > Co-authored-by: Jorn Vernee <jornver...@users.noreply.github.com> > - Update > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ppc64/PPC64Frame.java > > Co-authored-by: Jorn Vernee <jornver...@users.noreply.github.com> Overall the SA code looks good, but since I don't understand how UpcallStub linkage works, it's hard for me to say for sure if it is correct for what you are trying to accomplish. However, the test coverage seems to sufficiently ensure correctness of the implementation. Thanks for adding the code to have the LingeredApp readiness wait until the upcall thread is ready. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/UpcallStub.java line 41: > 39: > 40: private static AddressField lastJavaSPField; > 41: I think it's better without the blank lines. ------------- Marked as reviewed by cjplummer (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20885#pullrequestreview-2287301997 PR Review Comment: https://git.openjdk.org/jdk/pull/20885#discussion_r1747848359
