8349814: [leyden] Reduce uncommon traps in preload code #38

shipilev · 2025-02-11T10:47:37Z

Preload code is the C2 code we load very early, before it gets substituted by more advanced C2 code. We are seeing lots of uncommon traps from the preloaded code, though, which invalidates it too early. We can do precompile code without uncommon traps to alleviate this.

Additional testing:

Ad-hoc perf tests (see comment below)
Linux x86_64 server fastdebug, runtime/cds

Progress

Change must not contain extraneous whitespace
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8349814: [leyden] Reduce uncommon traps in preload code (Enhancement - P4)

Reviewers

Vladimir Kozlov (@vnkozlov - Committer) ⚠️ Review applies to 4051ce11
Vladimir Ivanov (@iwanowww - Committer) ⚠️ Review applies to de15c2cf

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 38

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/leyden/pull/38.diff

Using Webrev

Link to Webrev Comment

shipilev · 2025-02-11T10:48:32Z

Since this avoids excess C2 (re-)compilations, the major impact is on "user" time. This impact reflects to run-to-run times when we are short on available CPU cores:

Benchmark 1: build/linux-x86_64-server-release/images/jdk/bin/java -Xms64m -Xmx1g -XX:+UseParallelGC \
  -XX:CacheDataStore=JavacBenchApp.cds -cp JavacBenchApp.jar JavacBenchApp 50

# --- 32 cores

# Before: ~38 deopts from preload code
  Time (mean ± σ):     436.9 ms ±   9.0 ms    [User: 1162.8 ms, System: 148.6 ms]
  Range (min … max):   428.2 ms … 453.4 ms    10 runs

# After: ~0 deopts from preload code
  Time (mean ± σ):     438.2 ms ±   4.5 ms    [User: 828.9 ms, System: 131.0 ms]
  Range (min … max):   430.3 ms … 444.9 ms    10 runs

# --- 2 cores

# Before: ~30 deopts from preload code
  Time (mean ± σ):     531.0 ms ±  12.8 ms    [User: 846.1 ms, System: 117.8 ms]
  Range (min … max):   508.9 ms … 553.1 ms    10 runs

# After: ~0 deopts from preload code
  Time (mean ± σ):     510.4 ms ±  14.1 ms    [User: 803.7 ms, System: 115.0 ms]
  Range (min … max):   483.7 ms … 535.5 ms    10 runs

bridgekeeper · 2025-02-11T10:48:35Z

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

openjdk · 2025-02-11T10:49:17Z

@shipilev 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:

8349814: [leyden] Reduce uncommon traps in preload code

Reviewed-by: kvn, vlivanov

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 no new commits pushed to the premain branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@vnkozlov, @iwanowww) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mlbridge · 2025-02-11T10:52:59Z

Webrevs

src/hotspot/share/compiler/precompiler.cpp

vnkozlov

Good.

shipilev · 2025-02-11T19:59:51Z

@iwanowww, is this what you had in mind when we talked at our last meeting?

src/hotspot/share/compiler/precompiler.cpp

iwanowww · 2025-02-11T20:44:41Z

src/hotspot/share/runtime/deoptimization.cpp

@@ -2126,6 +2126,13 @@ JRT_ENTRY_PROF(void, Deoptimization, uncommon_trap_inner, Deoptimization::uncomm
      Events::log_deopt_message(current, "Uncommon trap: reason=%s action=%s pc=" INTPTR_FORMAT " method=%s @ %d %s",
                                reason_name, reason_action, pc,
                                tm->name_and_sig_as_C_string(), trap_bci, nm->compiler_name());
+
+      if (nm->preloaded() && (action != Action_none)) {


At some point, I added SCC-related info to TraceDeoptimization output [1], but it turns out I didn't upstream it. (I had a script which parsed the log and aggregated information about deopts in archived code.)

Maybe it's better to upstream it instead of introducing such adhoc logging.

[1] https://github.com/iwanowww/leyden/blame/premain-precompile/src/hotspot/share/runtime/deoptimization.cpp#L2151

But speaking of monitoring for uncommon traps in generated code: the ultimate goal may be to completely eliminate uncommon traps in preloaded code. In that respect, it's more interesting to monitor for all uncommon traps issued during compilation rather than those hit at runtime.

For C2 it's GraphKit::uncommon_trap(), so I suggest to move the code there. Also, additional information about compilation (id) and location (jvms info) is useful to pin down the root cause.

Yeah. I added logging at GraphKit::uncommon_trap. That logging is super-noisy. So I left the tracing code that warns when we trigger the actual deopt by stepping on such trap.

shipilev · 2025-02-13T13:15:52Z

In addition to Vladimir's comments above, I also found this greatly expands the size of generated code for preloading, which leads to expanded code buffers, which makes more methods ineligible for SCC. So the net effect can be negative. This PR is on hold until I figure out a way out of this.

openjdk · 2025-02-21T12:51:53Z

@shipilev Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

iwanowww

Looks good.

iwanowww · 2025-02-24T08:03:47Z

src/hotspot/share/runtime/deoptimization.cpp

+        // For performance reasons, preloaded nmethods should avoid deopts that lead to recompilations.
+        // Compiler logs all uncommon traps with -Xlog:scc. That log is noisy, and some traps may be
+        // legitimate. Here, we log the problems that really caused the deopts at runtime.
+        log_warning(deoptimization)("In preload code: reason=%s action=%s method=%s",


Does it warrant a warning though? It can become quite noisy and users don't have any way to address the problem. log_info looks a better fit here (and, probably, narrow the category to scc+deoptimization).

Doing these as warnings are my habit when doing high-perf prototypes. But yeah, we can do info instead, and I'll just enable the logging in my perf scripts.

openjdk bot added the rfr Pull request is ready for review label Feb 11, 2025

vnkozlov reviewed Feb 11, 2025

View reviewed changes

src/hotspot/share/compiler/precompiler.cpp Outdated Show resolved Hide resolved

vnkozlov approved these changes Feb 11, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 11, 2025

iwanowww reviewed Feb 11, 2025

View reviewed changes

shipilev mentioned this pull request Feb 21, 2025

8350488: [leyden] Experimental AOT-only mode #44

Closed

5 tasks

shipilev added 3 commits February 21, 2025 12:38

Fix

7abe8f9

Comment

fb6f4b2

Shuffle things around to capture more cases

de15c2c

shipilev force-pushed the JDK-8349814-preload-avoid-uncommon-traps branch from 4051ce1 to de15c2c Compare February 21, 2025 12:51

shipilev changed the title ~~8349814: [leyden] Avoid uncommon traps in preload code~~ 8349814: [leyden] Reduce uncommon traps in preload code Feb 21, 2025

iwanowww approved these changes Feb 24, 2025

View reviewed changes

shipilev added 2 commits February 24, 2025 10:29

Merge branch 'premain' into JDK-8349814-preload-avoid-uncommon-traps

2901b6c

Demote to info

bc4e825

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8349814: [leyden] Reduce uncommon traps in preload code #38

8349814: [leyden] Reduce uncommon traps in preload code #38

shipilev commented Feb 11, 2025 •

edited by openjdk bot

Loading

shipilev commented Feb 11, 2025 •

edited

Loading

bridgekeeper bot commented Feb 11, 2025

openjdk bot commented Feb 11, 2025 •

edited

Loading

mlbridge bot commented Feb 11, 2025 •

edited

Loading

vnkozlov left a comment

shipilev commented Feb 11, 2025

iwanowww Feb 11, 2025

iwanowww Feb 11, 2025

shipilev Feb 21, 2025

shipilev commented Feb 13, 2025

openjdk bot commented Feb 21, 2025

iwanowww left a comment

iwanowww Feb 24, 2025

shipilev Feb 24, 2025

8349814: [leyden] Reduce uncommon traps in preload code #38

Are you sure you want to change the base?

8349814: [leyden] Reduce uncommon traps in preload code #38

Conversation

shipilev commented Feb 11, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

shipilev commented Feb 11, 2025 • edited Loading

bridgekeeper bot commented Feb 11, 2025

openjdk bot commented Feb 11, 2025 • edited Loading

mlbridge bot commented Feb 11, 2025 • edited Loading

Webrevs

vnkozlov left a comment

Choose a reason for hiding this comment

shipilev commented Feb 11, 2025

iwanowww Feb 11, 2025

Choose a reason for hiding this comment

iwanowww Feb 11, 2025

Choose a reason for hiding this comment

shipilev Feb 21, 2025

Choose a reason for hiding this comment

shipilev commented Feb 13, 2025

openjdk bot commented Feb 21, 2025

iwanowww left a comment

Choose a reason for hiding this comment

iwanowww Feb 24, 2025

Choose a reason for hiding this comment

shipilev Feb 24, 2025

Choose a reason for hiding this comment

shipilev commented Feb 11, 2025 •

edited by openjdk bot

Loading

shipilev commented Feb 11, 2025 •

edited

Loading

openjdk bot commented Feb 11, 2025 •

edited

Loading

mlbridge bot commented Feb 11, 2025 •

edited

Loading