JDK-8256368 : Avoid repeated upcalls into Java to re-resolve MH/VH linkers/invokers
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 16,17,18,19
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-11-14
  • Updated: 2022-04-04
  • Resolved: 2022-02-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17 JDK 18 JDK 19
17.0.4-oracleFixed 18.0.2Fixed 19 b10Fixed
Related Reports
Relates :  
JDK-8253734 - C2: Optimize Move nodes
Relates :  
JDK-8254162 - Implementation of Foreign-Memory Access API (Third Incubator)
Relates :  
JDK-8252049 - Native memory leak in ciMethodData ctor
Sub Tasks
JDK-8256567 :  
ProblemList java/util/stream/test/org/openjdk/tests/java/util/stream/SpliteratorTest.java for Xcomp testing - Resolved
Description
The following test timed out on two platforms in the JDK16 CI:

java/util/stream/test/org/openjdk/tests/java/util/stream/SpliteratorTest.java

Here's a snippet from the Linux-X64 log:

#section:testng
----------messages:(7/410)----------
command: testng org.openjdk.tests.java.util.stream.SpliteratorTest
reason: Assumed action based on file name: run testng org.openjdk.tests.java.util.stream.SpliteratorTest 
Mode: othervm [test needs --add-modules, test or library overrides a system module]
Additional options from @modules: --add-modules jdk.incubator.foreign
Timeout information:
--- Timeout information end.
elapsed time (seconds): 1437.672
----------configuration:(5/428)----------
Boot Layer
  add modules: jdk.incubator.foreign 
  add reads:   java.base ALL-UNNAMED

----------System.out:(207/42802)----------
[TestNG] Running:
  java/util/stream/test/org/openjdk/tests/java/util/stream/SpliteratorTest.java

test org.openjdk.tests.java.util.stream.SpliteratorTest.testDoubleSpliterator("Arrays.s(array):empty", java.util.stream.DoubleStreamTestDataProvider$$Lambda$56/0x00000001000fb398@35adceef): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testDoubleSpliterator("Arrays.s(array,o,l):empty", java.util.stream.DoubleStreamTestDataProvider$$Lambda$57/0x00000001000fb5d0@42aa08cd): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testDoubleSpliterator("SpinedBuffer.s():empty", java.util.stream.DoubleStreamTestDataProvider$$Lambda$58/0x00000001000fb808@6de8c650): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testDoubleSpliterator("Primitives.s(SpinedBuffer.iterator(), size):empty", java.util.stream.DoubleStreamTestDataProvider$$Lambda$59/0x00000001000fba40@195b660e): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testDoubleSpliterator("Primitives.s(SpinedBuffer.iterator()):empty", java.util.stream.DoubleStreamTestDataProvider$$Lambda$60/0x00000001000fbc78@4f7d7d0f): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testDoubleSpliterator("DoubleStream.iterate(0,x->x<l;x->x+1):empty", java.util.stream.DoubleStreamTestDataProvider$$Lambda$61/0x00000001000fbeb0@3a40c10c): success

<snip>

test org.openjdk.tests.java.util.stream.SpliteratorTest.testSegmentSpliterator("bytes", [1024:b8], org.openjdk.tests.java.util.stream.SegmentTestDataProvider$$Lambda$203/0x00000001000c3208@1997bb47): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testSegmentSpliterator("chars", [1024:b16], org.openjdk.tests.java.util.stream.SegmentTestDataProvider$$Lambda$204/0x00000001000c3438@2ca39503): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testSegmentSpliterator("shorts", [1024:b16], org.openjdk.tests.java.util.stream.SegmentTestDataProvider$$Lambda$205/0x00000001000c3668@6ab2a087): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testSegmentSpliterator("ints", [1024:b32], org.openjdk.tests.java.util.stream.SegmentTestDataProvider$$Lambda$206/0x00000001000c3898@68de1eb4): success
test org.openjdk.tests.java.util.stream.SpliteratorTest.testSegmentSpliterator("longs", [1024:b64], org.openjdk.tests.java.util.stream.SegmentTestDataProvider$$Lambda$207/0x00000001000c3ac8@6027718e): success
Timeout refired 1200 times
----------System.err:(1/56)----------
WARNING: Using incubator modules: jdk.incubator.foreign
----------rerun:(51/6812)*----------

This test was modified by the following changeset:

    JDK-8254162 Implementation of Foreign-Memory Access API (Third Incubator)

which is in the affected build-ID: jdk-16+25-1585-tier6
Comments
A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk18u/pull/82 Date: 2022-03-30 05:48:55 +0000
30-03-2022
Fix Request (JDK 18u) Fixes a timeout / performance issue. The fix is low risk and applies cleanly. Already tested and about to be backported to Oracle JDK 17u. Tier 1-3 testing is running for JDK 18u.
30-03-2022
A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk17u-dev/pull/318 Date: 2022-03-30 12:25:51 +0000
30-03-2022
Fix Request (17u): Should get backported for parity with 17.0.4-oracle. Applies cleanly. Test has passed.
30-03-2022
Changeset: 534e5578 Author: Vladimir Ivanov <vlivanov@openjdk.org> Date: 2022-02-14 11:57:46 +0000 URL: https://git.openjdk.java.net/jdk/commit/534e557874274255c55086b4f6128063cbd9cc58
14-02-2022
A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk/pull/7429 Date: 2022-02-10 16:57:30 +0000
10-02-2022
ILW = MMH = P3
30-11-2020
Here's a working theory for what we're seeing: 1. This problem has always been there (as JDK-8252049, which is NOT memory access related demonstrates) 2. The previous version of the memory access API "seemed to work" because SpliteratorTest was using simple VarHandle of the form L->T, for which we have guards (for all carriers T) 3. The new iteration switches to a new shape off memory access var handle which is more regular: segment + offset so, now the VH is LJ->T and, for T=float/double we don't have var handle guards 4. When VH guards are not taken, we end up generating a _new_ lambda form on each invocation if the VarHandle is not a compiler constant and, even if it is a compiler constant, Xcomp seems to be enough to cause a new lambda form to be generated (for reasons that are obscure to me)
17-11-2020
This patch: diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index d62c187ccb0..837142b5634 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -1269,15 +1269,16 @@ Node* LoadNode::convert_to_signed_load(PhaseGVN& gvn) { } bool LoadNode::has_reinterpret_variant(const Type* rt) { - BasicType bt = rt->basic_type(); - switch (Opcode()) { - case Op_LoadI: return (bt == T_FLOAT); - case Op_LoadL: return (bt == T_DOUBLE); - case Op_LoadF: return (bt == T_INT); - case Op_LoadD: return (bt == T_LONG); - - default: return false; - } + return false; +// BasicType bt = rt->basic_type(); +// switch (Opcode()) { +// case Op_LoadI: return (bt == T_FLOAT); +// case Op_LoadL: return (bt == T_DOUBLE); +// case Op_LoadF: return (bt == T_INT); +// case Op_LoadD: return (bt == T_LONG); +// +// default: return false; +// } } Node* LoadNode::convert_to_reinterpret_load(PhaseGVN& gvn, const Type* rt) { @@ -1294,15 +1295,16 @@ Node* LoadNode::convert_to_reinterpret_load(PhaseGVN& gvn, const Type* rt) { } bool StoreNode::has_reinterpret_variant(const Type* vt) { - BasicType bt = vt->basic_type(); - switch (Opcode()) { - case Op_StoreI: return (bt == T_FLOAT); - case Op_StoreL: return (bt == T_DOUBLE); - case Op_StoreF: return (bt == T_INT); - case Op_StoreD: return (bt == T_LONG); - - default: return false; - } + return false; +// BasicType bt = vt->basic_type(); +// switch (Opcode()) { +// case Op_StoreI: return (bt == T_FLOAT); +// case Op_StoreL: return (bt == T_DOUBLE); +// case Op_StoreF: return (bt == T_INT); +// case Op_StoreD: return (bt == T_LONG); +// +// default: return false; +// } } Node* StoreNode::convert_to_reinterpret_store(PhaseGVN& gvn, Node* val, const Type* vt) { Seems to fix the issue (it's a very crude attempt to revert JDK-8253734).
17-11-2020
I managed to reproduce with the following set of options: `-vmoptions:"-Xcomp -XX:+CreateCoredumpOnCrash -ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:+TieredCompilation"` With these options, I can run all non-memory segment related tests in 15 seconds. I can also run on top all the memory segment tests (in additional 3s), but, mind you, excluding the float/double tests (this can be done by altering the set of tests returned by the data provider in SegmentTestDataProvider - e.g. by commenting lines). With float/double enabled (even on their own) the test timeouts. I suspect this is related to a recent VM fix: JDK-8253734, which optimizes access for floats/double. This fix was pushed more or less at the same time with the new memory access changes. While we could disable the float/double tests, I think it would be better to investigate, from a VM perspective, what is going wrong here.
16-11-2020
This test is normally ran in tier1 - the failures here only refer to tier6 (and history on that test show that only tier6 is failing) - what VM configuration is used, exactly, run to run these tests? On a regular linux x64 build (either release or fastdebug) on my local machine, the test runs in ~3 seconds, which hardly indicates a potential timeout issue. I believe there could be something else at play here?
16-11-2020
[~mcimadamore] - Can you take a look at these timeout failures? Thanks!
14-11-2020