8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0

[hseigel]

Please review this small fix for JDK-8284642. The fix was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux x64. Additionally, the modified test and the test in the bug report were run locally.

Thanks, Harold

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0

Reviewers

• Thomas Stuefe (@tstuefe - Reviewer) ⚠️ Review applies to c178c60
• David Holmes (@dholmes-ora - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8222/head:pull/8222
$ git checkout pull/8222

Update a local copy of the PR:
$ git checkout pull/8222
$ git pull https://git.openjdk.java.net/jdk pull/8222/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8222

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8222.diff

[bridgekeeper]

👋 Welcome back hseigel! 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]

@hseigel The following labels will be automatically applied to this pull request:

• core-libs
• nio

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

[hseigel]

/label add hotspot_runtime

[openjdk]

@hseigel
The label hotspot_runtime is not a valid label.
These labels are valid:

• serviceability
• hotspot
• hotspot-compiler
• ide-support
• kulla
• i18n
• shenandoah
• jdk
• javadoc
• security
• hotspot-runtime
• jmx
• build
• nio
• client
• core-libs
• compiler
• net
• hotspot-gc
• hotspot-jfr

[mlbridge]

Webrevs

• 02: Full - Incremental (22185a6d)
• 01: Full - Incremental (5fdb220a)
• 00: Full (c178c602)

[hseigel]

/label add hotspot-runtime

[openjdk]

@hseigel
The hotspot-runtime label was successfully added.

[tstuefe]

How long exactly has this been broken? I'd be worried about use cases where people use MaxDirectMemorySize=0 to prevent usage of direct memory altogether.

[AlanBateman]

If nothing else, I think the man page could be improved to say that JVM chooses the size for NIO direct-buffer allocations automatically when MaxDirectMemorySize is not set.

[tstuefe]

I looked around a bit but found no examples on GH or Google that relied on MaxDirectMemorySize=0 to disable direct memory. I found some examples of the opposite, e.g. cloudfoundry/java-buildpack#591. So I think this is fine.

[openjdk]

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

8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0

Reviewed-by: stuefe, dholmes

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

• 3416bfa: 8283022: com/sun/crypto/provider/Cipher/AEAD/GCMBufferTest.java failing with -Xcomp after 8273297
• 80a7f7b: 8267690: Revisit (Doc)Tree search implemented by throwing an exception
• 9b82708: 8284319: Test runtime/cds/appcds/TestParallelGCWithCDS.java fails in repo-loom
• fb60594: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double
• 1e79ded: 8284889: runtime/cds/appcds/loaderConstraints/DynamicLoaderConstraintsTest.java#custom-cl-zgc timed out
• 414918d: 8285389: EdDSA trimming zeros
• 293bc5e: 8129778: Few awt test fail for Solaris 11 with RuntimeException
• 36f2e52: 8225777: java/awt/Mixing/MixingOnDialog.java fails on Ubuntu
• 32593df: 8279888: Local variable independently used by multiple loops can interfere with loop optimizations
• 4c22a9b: 8282823: javac should constrain more uses of preview APIs
• ... and 135 more: https://git.openjdk.java.net/jdk/compare/8ee2944cc404d4d53d0f94b56dd52111fd31cc39...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.

[dholmes-ora]

I don't think this is the right "fix". There is a bit of a disconnect between the VM option and the system property. Although the default value of the VM flag is zero, that is ignored and the JDK side, not being directed otherwise, determines the max size. If you set the flag explicitly to zero that is passed through to the JDK and the max memory is actually set to zero.

I think this is more a documentation issue - MaxDirectMemory has no affect unless explicitly set on the command-line. The default value of the flag is actually irrelevant.

[dholmes-ora]

See main comment

[hseigel]

Should no changes be made to the code, instead just remove "the size is set to 0, meaning that" from the description of MaxDirectMemorySize?

-XX:MaxDirectMemorySize=size
Sets the maximum total size (in bytes) of the java.nio package, direct-buffer allocations. Append the letter k or K to indicate kilobytes, m or M to indicate megabytes, or g or G to indicate gigabytes. By default, the size is set to 0, meaning that the JVM chooses the size for NIO direct-buffer allocations automatically.

[dholmes-ora]

I would delete the last sentence:

By default, the size is set to 0, meaning that the JVM chooses the size for NIO direct-buffer allocations automatically.

and replace with:

If not set, the flag is ignored and the JVM chooses the size for NIO direct-buffer allocations automatically.

[hseigel]

Please review the updated man page change.
Thanks, Harold

[dholmes-ora]

I didn't realize that text was from the manpage. It is fine but I think we also need something in globals.hpp:

 product(uint64_t, MaxDirectMemorySize, 0,                                 \
          "Maximum total size of NIO direct-buffer allocations. "        \
          "Ignored if not explicitly set.")
          range(0, max_jlong)                                               \

[hseigel]

Hi David, Thanks for looking at this and for the suggested text. Please review this latest commit to fix the text in globals.hpp.
Thanks, Harold

[dholmes-ora]

That all looks good to me.

Thanks,
David

[hseigel]

Thanks for the reviews!

/integrate

[openjdk]

Going to push as commit 975a060.
Since your change was applied there have been 158 commits pushed to the master branch:

• 03bcf7b: 8283620: System.out does not use the encoding/charset specified in the Javadoc
• 20a132d: 8284994: -Xdoclint:all returns warning for records, even when documented properly
• a3b7881: 8284930: Re-examine FilterInputStream mark/reset
• 97a0a29: 8283643: [AIX, testbug] MachCodeFramesInErrorFile test fails to find 'Native frames' text
• 67755ed: 8284890: Support for Do not fragment IP socket options
• a7b5157: 8282541: AArch64: Auto-vectorize Math.round API
• 8de3c65: 8284951: Compile::flatten_alias_type asserts with "indeterminate pointers come only from unsafe ops"
• 552e1b0: 8284779: Test java/util/logging/Logger/logrb/TestLogrbResourceBundle.java fails intermittently with vthreads wrapper
• e333cd3: 8285611: Retrofit (Doc)Pretty with java.io.UncheckedIOException
• 9478696: 8283441: C2: segmentation fault in ciMethodBlocks::make_block_at(int)
• ... and 148 more: https://git.openjdk.java.net/jdk/compare/8ee2944cc404d4d53d0f94b56dd52111fd31cc39...master

Your commit was automatically rebased without conflicts.

[openjdk]

@hseigel Pushed as commit 975a060.

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