8389130: [lworld] Compiled frame walking is slow due to scalarized calling convention handling

[marc-chevalier]

This PR replaces the Valhalla draft PR openjdk/valhalla#2687, which was already reviewed by @TobiHartmann and awaiting review from runtime but was not integrated due to the code freeze immediately before the Valhalla mainline integration.

---

The problem is that Valhalla made frame::sender a tiny bit bigger, but it is enough for gcc to decide not to inline it anymore. And then, functions such as void java_lang_Throwable::fill_in_stack_trace(Handle throwable, const methodHandle& method, TRAPS) have a call, instead of having it inlined all the way to intptr_t* frame::repair_sender_sp(intptr_t* sender_sp, intptr_t** saved_fp_addr) const. While there are a lot of paths, and its job is a bit subtle frame frame::sender_for_compiled_frame(RegisterMap* map) const is mostly doing rather basic arithmetic: a call can easily create some slowdown, Especially in cases where walking the stack is on a hot path.

So, to address this, I propose a 2-part solution: trying to shrink the size of sender, and marking it as ALWAYS_INLINE. To shrink the size, I cache whether the method needs stack repairs in its flags rather than recomputing it, since it is a constant. It doesn't seem to make it as small as the mainline version, but quite closer. This change is not satisfactory by itself because, even if it were enough to reenable inlining (which it is not), we are so clone from the inlining threshold that the least change in the future could make it bad again. Since we know we are not far from inlining, force inlining the method cannot be too bad.

It is to note that because it is about codesize and inlining problem, the performance issue appear without --enable-preview. Trying to branch on that just makes the code bigger and the problem worse.

Valhalla:

Time (mean ± σ):      9.429 s ±  0.098 s    [User: 9.274 s, System: 0.171 s]
Range (min … max):    9.246 s …  9.548 s    10 runs

Mainline:

Time (mean ± σ):      7.957 s ±  0.082 s    [User: 7.817 s, System: 0.152 s]
Range (min … max):    7.778 s …  8.051 s    10 runs

This PR:

Time (mean ± σ):      8.107 s ±  0.077 s    [User: 7.943 s, System: 0.183 s]
Range (min … max):    7.967 s …  8.218 s    10 runs

We can see the changes for the performances of mainline and this PR overlap. There is still a gap, but of 0.15s, that is about 2% instead of 18.5%. It seems the rest of the slowdown is mostly happening during startup.

For comparison, here are the timing for the given test, with only the flag fix, without forcing inlining:

Time (mean ± σ):      9.326 s ±  0.075 s    [User: 9.164 s, System: 0.179 s]
Range (min … max):    9.188 s …  9.422 s    10 runs

That is only marginally (but consistently) better than Valhalla's current state, but clearly not good enough.

Thanks,
Marc

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8389130: [lworld] Compiled frame walking is slow due to scalarized calling convention handling (Bug - P3)

Reviewers

• Tobias Hartmann (@TobiHartmann - Reviewer)
• Frederic Parain (@fparain - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32148/head:pull/32148
$ git checkout pull/32148

Update a local copy of the PR:
$ git checkout pull/32148
$ git pull https://git.openjdk.org/jdk.git pull/32148/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 32148

View PR using the GUI difftool:
$ git pr show -t 32148

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32148.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mchevalier! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@marc-chevalier This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8389130: [lworld] Compiled frame walking is slow due to scalarized calling convention handling

Reviewed-by: thartmann, fparain

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 41 new commits pushed to the master branch:

• 83fa552: 8389354: [lworld] PhaseIdealLoop::move_flat_array_check_out_of_loop creates anti-dependence cycle
• cc72529: 8388442: C2: LoadNKlassNode::Identity() may return a new node
• bc6a625: 8389089: [lworld] C1 should not initialize value class when accessing flat field or array
• ... and 38 more: https://git.openjdk.org/jdk/compare/cc278dbb8a1ca0754d5842708b9029441055d361...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@marc-chevalier The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: hotspot. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 00: Full (ad1f4c18)

[TobiHartmann]

Still good.

[fparain]

Changes look semantically correct to me.