JDK-8318016: Per-compilation memory ceiling #16220

tstuefe · 2023-10-17T13:29:20Z

This RFE introduces a new compile command to limit memory usage per compilation. It complements and is based upon the JIT memory statistic introduced with https://bugs.openjdk.org/browse/JDK-8317683.

This new memory limit is applied during compilation for either C1 or C2. It has one of two modes:

crash mode (mainly for compiler devs), which aborts with hs-err file when the ceiling is reached, right in the compiler thread at the allocation point that triggered the ceiling
stop mode, which stops the compilation similarly to how hitting the node limit in C2 would stop the compilation.

Usage

The compile command takes the form -XX:CompileCommand:MemLimit,<method>,<memory size>[~<option>]

The memory size that can be specified as usual. The command also takes an optional trailing option, separated from the memory size by a tilde (odd choice but needed because of https://bugs.openjdk.org/browse/JDK-8318214).

The <suboption> can be either crash or stop and distinguishes between the aforementioned modes. If omitted, the default is "stop".

Since MemLimit is based on the JIT statistic, it is implicitly enabled, so there is no need to explicitly enable it with MemStat.

Usage examples:

crash when reaching 100m for a single compilation:

-XX:CompileCommand='MemLimit,*.*,100m~crash'

limit all methods to a 2m footprint and print a trailing record at VM shutdown

-XX:CompileCommand='MemLimit,*.*,2m' -XX:CompileCommand='MemStat,*.*,print'

The JIT statistic table (printed either at VM shutdown or via jcmd <pid> Compiler.memory) now has a new column "result" that shows if the compilation was completed successfully or had an error or an oom:

                              vvvvv                                                                                                                                                                                                                                                
total     NA        RA        result  #nodes  time    type  #rc thread              method
3036016   687328    1757456   oom     2607    0,320   c2    1   0x00007fcf1c181550  java/lang/invoke/StringConcatFactory::<clinit>(()V)

Implementation notes

The crash variant works immediately.

The stop variant works delayed: If arena memory reaches the limit in the scope of a compilation, we signal the Compile object (Compile for c2, Compilation for c1) that an oom has occurred.

For C1, we set the bailout string right here. That causes the C1 compilation to fail at the next bailout check.

For C2, we cannot just set Compile::_failure_reason since I found that the C2 is not well prepared to bail out from wherever it is tested. So I incorporated the oom check in the nodelimit check C2 does. Note that even that needs some preparatory work (see #16205, a prerequisite for this RFE).

Due to this delay, the actual memory used per compilation may be somewhat higher than the limit. In practice the limit works reasonably well, though.

Progress

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

Issue

JDK-8318016: Per-compilation memory ceiling (Enhancement - P4)

Reviewers

Roland Westrelin (@rwestrel - Reviewer) ⚠️ Review applies to 8e0ab821
Tobias Hartmann (@TobiHartmann - Reviewer) ⚠️ Review applies to 55e12f68

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16220

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-10-17T13:31:19Z

👋 Welcome back stuefe! 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 · 2023-10-17T13:35:21Z

@tstuefe 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.

tstuefe · 2023-10-17T15:01:30Z

/label add hotspot-compiler
/label remove hotspot

openjdk · 2023-10-17T15:05:16Z

@tstuefe
The hotspot-compiler label was successfully added.

openjdk · 2023-10-17T15:05:49Z

@tstuefe
The hotspot label was successfully removed.

mlbridge · 2023-10-24T05:52:51Z

Webrevs

rwestrel

That looks good to me.
Have you done any stress testing? Maybe running compile the world with a low memory limit.

rwestrel · 2023-10-27T08:00:35Z

src/hotspot/share/compiler/compilationMemoryStatistic.cpp

@@ -368,6 +395,20 @@ void CompilationMemoryStatistic::on_end_compilation() {
    arena_stat->print_on(tty);
    tty->cr();
  }
+
+  // Store result
+  // For this to work, we must call on_end_compilatio()n at a point where


Typo: on_end_compilatio()n

openjdk · 2023-10-27T08:04:49Z

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

8318016: Per-compilation memory ceiling

Reviewed-by: roland, thartmann

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 68 new commits pushed to the master branch:

2a76ad9: 8318683: compiler/c2/irTests/TestPhiDuplicatedConversion.java "Failed IR Rules (2) of Methods (2)"
b3fec6b: 8306980: Generated docs should contain correct Legal Documents
1139482: 8316132: CDSProtectionDomain::get_shared_protection_domain should check for exception
2182c93: 8313643: Update HarfBuzz to 8.2.2
613a3cc: 8301846: Invalid TargetDataLine after screen lock when using JFileChooser or COM library
613d32c: 8169475: WheelModifier.java fails by timeout
f1e8787: 8317609: Classfile API fails to verify /jdk.jcmd/sun/tools/jstat/Alignment.class
47624f6: 8299058: AssertionError in sun.net.httpserver.ServerImpl when connection is idle
2d5829a: 8239508: JFR: @RemoveFields
0064cf9: 8311596: Add separate system properties for TLS server and client for maximum chain length
... and 58 more: https://git.openjdk.org/jdk/compare/d96f38b80c1606b54b9f3dbfe9717ab9653a0605...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.

tstuefe · 2023-10-27T13:02:30Z

That looks good to me. Have you done any stress testing? Maybe running compile the world with a low memory limit.

Many thanks @rwestrel .

I did stress testing by adding a random (low) limit and running GHAs with it. I also ran with -Xcomp -Xbatch. I found a bunch of missing bailout chains, which I took care of before posting this PR (https://bugs.openjdk.org/browse/JDK-8318445, https://bugs.openjdk.org/browse/JDK-8318183).

But there are more (e.g. #16375). We have been discussing a better way to care for these errors (I should have done this on the mailing list, sorry) and maybe introduce a semi-automatic way. Experiments of mine are here: #16289 - but I try to find a way that is less invasive.

Finally, there is #16247, which adds better diagnostic information to the hs-err file when we miss a bailout.

Bottom line, there are still outstanding issues with missing bailouts, and possibly a better solution at some point. In my view this does not prevent this RFE from going forward.

rwestrel · 2023-10-27T16:04:58Z

Bottom line, there are still outstanding issues with missing bailouts, and possibly a better solution at some point. In my view this does not prevent this RFE from going forward.

Thanks for the details. That sounds reasonable to me.

TobiHartmann

That's a very useful feature, the implementation looks good to me. I've submitted some testing and will report back once it passed.

test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java

TobiHartmann · 2023-11-01T06:18:57Z

All tests passed.

Co-authored-by: Tobias Hartmann <tobias.hartmann@oracle.com>

tstuefe · 2023-11-01T08:12:17Z

Many thanks @TobiHartmann!

/integrate

openjdk · 2023-11-01T08:12:41Z

Going to push as commit 0461d9a.
Since your change was applied there have been 68 commits pushed to the master branch:

2a76ad9: 8318683: compiler/c2/irTests/TestPhiDuplicatedConversion.java "Failed IR Rules (2) of Methods (2)"
b3fec6b: 8306980: Generated docs should contain correct Legal Documents
1139482: 8316132: CDSProtectionDomain::get_shared_protection_domain should check for exception
2182c93: 8313643: Update HarfBuzz to 8.2.2
613a3cc: 8301846: Invalid TargetDataLine after screen lock when using JFileChooser or COM library
613d32c: 8169475: WheelModifier.java fails by timeout
f1e8787: 8317609: Classfile API fails to verify /jdk.jcmd/sun/tools/jstat/Alignment.class
47624f6: 8299058: AssertionError in sun.net.httpserver.ServerImpl when connection is idle
2d5829a: 8239508: JFR: @RemoveFields
0064cf9: 8311596: Add separate system properties for TLS server and client for maximum chain length
... and 58 more: https://git.openjdk.org/jdk/compare/d96f38b80c1606b54b9f3dbfe9717ab9653a0605...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-11-01T08:12:48Z

@tstuefe Pushed as commit 0461d9a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

tstuefe force-pushed the JDK-8318016-compiler-memory-ceiling branch from e2ca348 to 9c2b498 Compare October 17, 2023 13:32

openjdk bot added the hotspot hotspot-dev@openjdk.org label Oct 17, 2023

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Oct 17, 2023

openjdk bot removed the hotspot hotspot-dev@openjdk.org label Oct 17, 2023

tstuefe force-pushed the JDK-8318016-compiler-memory-ceiling branch 4 times, most recently from 3312263 to 9cddbec Compare October 19, 2023 14:40

tstuefe mentioned this pull request Oct 20, 2023

JDK-8318445: More broken bailout chains in C2 #16248

Closed

3 tasks

tstuefe force-pushed the JDK-8318016-compiler-memory-ceiling branch from c39c0c7 to e807fec Compare October 23, 2023 11:41

memlimit

976d011

tstuefe force-pushed the JDK-8318016-compiler-memory-ceiling branch from e807fec to 976d011 Compare October 23, 2023 15:55

tstuefe marked this pull request as ready for review October 24, 2023 05:48

openjdk bot added the rfr Pull request is ready for review label Oct 24, 2023

Merge branch 'openjdk:master' into JDK-8318016-compiler-memory-ceiling

8e0ab82

rwestrel approved these changes Oct 27, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 27, 2023

fix typo

55e12f6

TobiHartmann approved these changes Oct 31, 2023

View reviewed changes

test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java Outdated Show resolved Hide resolved

test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java Outdated Show resolved Hide resolved

test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java Outdated Show resolved Hide resolved

Update test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java

b7adac2

Co-authored-by: Tobias Hartmann <tobias.hartmann@oracle.com>

tstuefe and others added 2 commits November 1, 2023 09:11

Update test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java

65b73fe

Co-authored-by: Tobias Hartmann <tobias.hartmann@oracle.com>

Update test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java

153f42e

Co-authored-by: Tobias Hartmann <tobias.hartmann@oracle.com>

openjdk bot added the integrated Pull request has been integrated label Nov 1, 2023

openjdk bot closed this Nov 1, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8318016: Per-compilation memory ceiling #16220

JDK-8318016: Per-compilation memory ceiling #16220

tstuefe commented Oct 17, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 17, 2023

openjdk bot commented Oct 17, 2023

tstuefe commented Oct 17, 2023

openjdk bot commented Oct 17, 2023

openjdk bot commented Oct 17, 2023

mlbridge bot commented Oct 24, 2023 •

edited

Loading

rwestrel left a comment

rwestrel Oct 27, 2023

openjdk bot commented Oct 27, 2023 •

edited

Loading

tstuefe commented Oct 27, 2023

rwestrel commented Oct 27, 2023

TobiHartmann left a comment

TobiHartmann commented Nov 1, 2023

tstuefe commented Nov 1, 2023

openjdk bot commented Nov 1, 2023

openjdk bot commented Nov 1, 2023

JDK-8318016: Per-compilation memory ceiling #16220

JDK-8318016: Per-compilation memory ceiling #16220

Conversation

tstuefe commented Oct 17, 2023 • edited by openjdk bot Loading

Usage

Implementation notes

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Oct 17, 2023

openjdk bot commented Oct 17, 2023

tstuefe commented Oct 17, 2023

openjdk bot commented Oct 17, 2023

openjdk bot commented Oct 17, 2023

mlbridge bot commented Oct 24, 2023 • edited Loading

Webrevs

rwestrel left a comment

Choose a reason for hiding this comment

rwestrel Oct 27, 2023

Choose a reason for hiding this comment

openjdk bot commented Oct 27, 2023 • edited Loading

tstuefe commented Oct 27, 2023

rwestrel commented Oct 27, 2023

TobiHartmann left a comment

Choose a reason for hiding this comment

TobiHartmann commented Nov 1, 2023

tstuefe commented Nov 1, 2023

openjdk bot commented Nov 1, 2023

openjdk bot commented Nov 1, 2023

tstuefe commented Oct 17, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 24, 2023 •

edited

Loading

openjdk bot commented Oct 27, 2023 •

edited

Loading