8299962: Speed up compiler/intrinsics/unsafe/DirectByteBufferTest.java and HeapByteBufferTest.java

[eme64]

The test used a blanket 100_000 iterations to ensure C2 triggers for the relevant intrinsics of jdk.internal.misc.Unsafe.

I experimented lowering the number of iterations, and got the following results:
100_000 (overkill, 8.5 sec) 5000(very safe, 3 sec), 2000(decent, 2.6 sec) and even 200 (ok, 2.5 sec).
I measured the time per @run statement (there are 6 over the two test files).

For 5000 I got the same count of intrinsifications per intrinsic.
For 2000 it dropped slightly, rarely an intrinsic was not intrinsified.
For 200 it dropped a bit more, and sometimes multiple intrinsics are not intrinsified.

If one uses -Xbatch -X:-TieredCompilation, then the cound does not change at all, even for 200.

Since the marginal speedup from 5000 to 200 is very small, I decided to be on the safe side.
This is still a speedup of 2.7x.

---

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-8299962: Speed up compiler/intrinsics/unsafe/DirectByteBufferTest.java and HeapByteBufferTest.java

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer)
• Tobias Hartmann (@TobiHartmann - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11944

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

Using diff file

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

[bridgekeeper]

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

@eme64 The following label will be automatically applied to this pull request:

• hotspot-compiler

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.

[mlbridge]

Webrevs

• 00: Full (2a8c0f58)

[vnkozlov]

Default C2 compilation threshold is around 10000. So 5000 is not safe.
Instead I suggest to use -XX:CompileThresholdScaling= together with small iterations numbers which guarantees C2 compilation.

[eme64]

@vnkozlov Something I failed to mention in the description:
In ByteBufferTest.java (which is the implementation of both tests) we have lots of inner loops, which all have at least 100 iterations. The buffer has a length of 1024, the element size is maximally 8 bytes. In those inner loops, we do all the buffer-operations, which lead to the Unsafe intrinsics.
So then if we take even the OSR C2 threshold of Tier4BackEdgeThreshold=40000, I would assume that an outer iteration count of 40_000 / 100 = 400 would have to suffice.
I analyzed the PrintInlining statements, and it seems to really be ok for 5000, also for 2000 in most cases.

[vnkozlov]

Thank you for explanation. Looks good.

ByteBufferTest::stepUsingAccessors() code is not C2 friendly because it will get profiling data only for first loop before triggering OSR and full compilation. Which may cause deoptimization and recompilation when it hit following loops. That is may be why iterations value was so high to make sure profiling data is collected for all loops.

The test should have separate methods for each type of intrinsics.

[openjdk]

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

8299962: Speed up compiler/intrinsics/unsafe/DirectByteBufferTest.java and HeapByteBufferTest.java

Reviewed-by: kvn, 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 38 new commits pushed to the master branch:

• be8e6d0: 8299957: Enhance error logging in instrument coding with additional jplis_assert_msg
• 640eff6: 8300040: TypeOopPtr::make_from_klass_common calls itself with args in wrong order
• 19628e3: 8300068: UBSan CFLAGS/LDFLAGS not passed when building ADLC
• 9887047: Merge
• 4b92fb0: 8299918: Update Xcode11.3.1-MacOSX10.15 devkit at Oracle
• 6a4a874: 8299034: Runtime::exec clarification of inherited environment
• 752a370: 8299439: java/text/Format/NumberFormat/CurrencyFormat.java fails for hr_HR
• 3918f9f: 8299090: Specify coordinate order for additional CaptureCallState parameters on class as well
• 33412c1: 8299677: Formatter.format might take a long time to format an integer or floating-point
• 7a85d95: 8298448: UndefinedBehaviorSanitizer
• ... and 28 more: https://git.openjdk.org/jdk/compare/5a9490a40e0fe281fc1b33d2c39a9a970d8a7b4f...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.

[TobiHartmann]

Looks good to me.

[eme64]

@vnkozlov I filed a follow up RFE with your suggestion for improvement: JDK-8300187

Thanks for the help and reviews @vnkozlov @TobiHartmann

/integrate

[openjdk]

Going to push as commit 7c1ebcc.
Since your change was applied there have been 62 commits pushed to the master branch:

• abfd7e8: 8300113: C2: Single-bit fields with signed type in TypePtr after JDK-8297933
• 83f2c9a: 8293410: Remove GenerateRangeChecks flag
• 50e7df9: 8300024: Replace use of JNI_COMMIT mode with mode 0
• fe7fca0: 8300032: DwarfParser resource leak
• 12edd6f: 8300052: PdhDll::PdhCollectQueryData and PdhLookupPerfNameByIndex will never be NULL
• 7bcd5f4: 8297914: Remove java_lang_Class::process_archived_mirror()
• 4be94e4: Merge
• 87865e0: 8299502: Usage of constructors of primitive wrapper classes should be avoided in javax.xml API docs
• dc5cc61: 8299499: Usage of constructors of primitive wrapper classes should be avoided in java.net API docs
• 98eae6d: 8299500: Usage of constructors of primitive wrapper classes should be avoided in java.text API docs
• ... and 52 more: https://git.openjdk.org/jdk/compare/5a9490a40e0fe281fc1b33d2c39a9a970d8a7b4f...master

Your commit was automatically rebased without conflicts.

[openjdk]

@eme64 Pushed as commit 7c1ebcc.

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