Skip to content

Commit 95b689f

Browse files
mur47x111mur47x111
committed
Eliminate nested locks when materializing locks during CommitAllocationNode lowering
1 parent c176991 commit 95b689f

File tree

3 files changed

+185
-0
lines changed

3 files changed

+185
-0
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/MonitorPEATest.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.junit.Before;
3333
import org.junit.Test;
3434

35+
import jdk.graal.compiler.api.directives.GraalDirectives;
3536
import jdk.graal.compiler.graph.Node;
3637
import jdk.graal.compiler.graph.NodeFlood;
3738
import jdk.graal.compiler.hotspot.replacements.HotSpotReplacementsUtil;
@@ -274,4 +275,129 @@ public static void snippet8() {
274275
public void testSnippet8() {
275276
test("snippet8");
276277
}
278+
279+
public static void snippet9() {
280+
B b = new B();
281+
C c = new C(b);
282+
synchronized (c) {
283+
synchronized (b) {
284+
synchronized (b) {
285+
synchronized (c) {
286+
synchronized (B.class) {
287+
}
288+
}
289+
}
290+
}
291+
}
292+
}
293+
294+
@Test
295+
public void testSnippet9() {
296+
test("snippet9");
297+
}
298+
299+
public static void snippet15(boolean deoptimize) {
300+
synchronized (new B()) {
301+
synchronized (new StringBuilder()) {
302+
synchronized (B.class) {
303+
if (deoptimize) {
304+
GraalDirectives.deoptimize();
305+
}
306+
}
307+
}
308+
}
309+
}
310+
311+
@Test
312+
public void testSnippet15() {
313+
test("snippet15", true);
314+
}
315+
316+
public static void snippet16(boolean deoptimize) {
317+
B b = new B();
318+
C c = new C(b);
319+
synchronized (c) {
320+
synchronized (b) {
321+
synchronized (B.class) {
322+
if (deoptimize) {
323+
GraalDirectives.deoptimize();
324+
}
325+
}
326+
}
327+
}
328+
}
329+
330+
@Test
331+
public void testSnippet16() {
332+
test("snippet16", true);
333+
}
334+
335+
public static void snippet17(boolean deoptimize) {
336+
B b = new B();
337+
C c = new C(b);
338+
synchronized (c) {
339+
synchronized (c) {
340+
synchronized (b) {
341+
synchronized (b) {
342+
synchronized (B.class) {
343+
if (deoptimize) {
344+
GraalDirectives.deoptimize();
345+
}
346+
}
347+
}
348+
}
349+
}
350+
}
351+
}
352+
353+
@Test
354+
public void testSnippet17() {
355+
test("snippet17", true);
356+
}
357+
358+
public static void snippet18(boolean deoptimize) {
359+
B b = new B();
360+
C c = new C(b);
361+
synchronized (c) {
362+
synchronized (b) {
363+
synchronized (c) {
364+
synchronized (b) {
365+
synchronized (B.class) {
366+
if (deoptimize) {
367+
GraalDirectives.deoptimize();
368+
}
369+
}
370+
}
371+
}
372+
}
373+
}
374+
}
375+
376+
@Test
377+
public void testSnippet18() {
378+
test("snippet18", true);
379+
}
380+
381+
public static void snippet19(boolean deoptimize) {
382+
B b = new B();
383+
C c = new C(b);
384+
synchronized (c) {
385+
synchronized (b) {
386+
synchronized (b) {
387+
synchronized (c) {
388+
synchronized (B.class) {
389+
if (deoptimize) {
390+
GraalDirectives.deoptimize();
391+
}
392+
}
393+
}
394+
}
395+
}
396+
}
397+
}
398+
399+
@Test
400+
public void testSnippet19() {
401+
test("snippet19", true);
402+
}
277403
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/LockEliminationPhase.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.Optional;
2828

2929
import jdk.graal.compiler.debug.Assertions;
30+
import jdk.graal.compiler.debug.GraalError;
3031
import jdk.graal.compiler.graph.Node;
3132
import jdk.graal.compiler.nodes.FixedNode;
3233
import jdk.graal.compiler.nodes.GraphState;
@@ -45,6 +46,9 @@
4546
import jdk.graal.compiler.nodes.java.MonitorEnterNode;
4647
import jdk.graal.compiler.nodes.java.MonitorExitNode;
4748
import jdk.graal.compiler.nodes.java.MonitorIdNode;
49+
import jdk.graal.compiler.nodes.memory.MemoryAnchorNode;
50+
import jdk.graal.compiler.nodes.memory.MemoryKill;
51+
import jdk.graal.compiler.nodes.memory.SingleMemoryKill;
4852
import jdk.graal.compiler.nodes.spi.ValueProxy;
4953
import jdk.graal.compiler.nodes.util.GraphUtil;
5054
import jdk.graal.compiler.phases.Phase;
@@ -196,4 +200,34 @@ public static GuardingNode unproxifyHighestGuard(ValueNode value) {
196200
}
197201
}
198202

203+
public static void removeMonitorAccess(AccessMonitorNode access) {
204+
GraalError.guarantee(!(access instanceof OSRMonitorEnterNode), "Must not remove OSR monitor enters");
205+
if (access.usages().isNotEmpty()) {
206+
boolean replaced = false;
207+
for (FixedNode pred : GraphUtil.predecessorIterable((FixedNode) access.predecessor())) {
208+
if (MemoryKill.isSingleMemoryKill(pred)) {
209+
SingleMemoryKill single = (SingleMemoryKill) pred;
210+
if (single.getKilledLocationIdentity().isAny()) {
211+
// We do not need a memory anchor in this case.
212+
access.replaceAtUsages(pred);
213+
replaced = true;
214+
break;
215+
} else {
216+
break;
217+
}
218+
} else if (MemoryKill.isMultiMemoryKill(pred)) {
219+
break;
220+
}
221+
}
222+
if (!replaced) {
223+
MemoryAnchorNode anchor = access.graph().add(new MemoryAnchorNode());
224+
anchor.setNodeSourcePosition(access.getNodeSourcePosition());
225+
access.replaceAtUsages(anchor);
226+
access.graph().addBeforeFixed(access, anchor);
227+
}
228+
}
229+
GraalError.guarantee(access.hasNoUsages(), "Node must not have usages %s", access);
230+
GraphUtil.removeFixedWithUnusedInputs(access);
231+
}
232+
199233
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/DefaultJavaLoweringProvider.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static jdk.graal.compiler.nodes.calc.BinaryArithmeticNode.branchlessMax;
3232
import static jdk.graal.compiler.nodes.calc.BinaryArithmeticNode.branchlessMin;
3333
import static jdk.graal.compiler.nodes.java.ArrayLengthNode.readArrayLength;
34+
import static jdk.graal.compiler.phases.common.LockEliminationPhase.removeMonitorAccess;
3435
import static jdk.vm.ci.meta.DeoptimizationAction.InvalidateReprofile;
3536
import static jdk.vm.ci.meta.DeoptimizationReason.BoundsCheckException;
3637
import static jdk.vm.ci.meta.DeoptimizationReason.NullCheckException;
@@ -132,6 +133,7 @@
132133
import jdk.graal.compiler.nodes.java.LoweredAtomicReadAndAddNode;
133134
import jdk.graal.compiler.nodes.java.LoweredAtomicReadAndWriteNode;
134135
import jdk.graal.compiler.nodes.java.MonitorEnterNode;
136+
import jdk.graal.compiler.nodes.java.MonitorExitNode;
135137
import jdk.graal.compiler.nodes.java.MonitorIdNode;
136138
import jdk.graal.compiler.nodes.java.NewArrayNode;
137139
import jdk.graal.compiler.nodes.java.NewInstanceNode;
@@ -1115,6 +1117,15 @@ private static void computeAllocationEmissionOrder(CommitAllocationNode commit,
11151117
}
11161118
}
11171119

1120+
private static boolean isNestedLock(MonitorIdNode lock, CommitAllocationNode commit) {
1121+
for (MonitorIdNode otherLock : commit.getLocks()) {
1122+
if (otherLock.getLockDepth() < lock.getLockDepth() && commit.getObjectIndex(lock) == commit.getObjectIndex(otherLock)) {
1123+
return true;
1124+
}
1125+
}
1126+
return false;
1127+
}
1128+
11181129
public void finishAllocatedObjects(LoweringTool tool, FixedWithNextNode insertAfter, CommitAllocationNode commit, ValueNode[] allocations) {
11191130
FixedWithNextNode insertionPoint = insertAfter;
11201131
StructuredGraph graph = commit.graph();
@@ -1151,6 +1162,20 @@ public void finishAllocatedObjects(LoweringTool tool, FixedWithNextNode insertAf
11511162
// Ensure that the lock operations are performed in lock depth order
11521163
ArrayList<MonitorIdNode> newList = new ArrayList<>(locks);
11531164
newList.sort((a, b) -> Integer.compare(a.getLockDepth(), b.getLockDepth()));
1165+
// Eliminate nested locks
1166+
newList.removeIf(lock -> isNestedLock(lock, commit));
1167+
1168+
for (MonitorIdNode lock : locks) {
1169+
if (!newList.contains(lock)) {
1170+
// lock is nested and eliminated
1171+
for (Node usage : lock.usages().snapshot()) {
1172+
if (usage.isAlive() && usage instanceof MonitorExitNode exit) {
1173+
removeMonitorAccess(exit);
1174+
}
1175+
}
1176+
lock.setEliminated();
1177+
}
1178+
}
11541179
locks = newList;
11551180
}
11561181

0 commit comments

Comments
 (0)