Skip to content

Commit f0ed8f9

Browse files
committed
Fix performance issue in InstanceOfPatternMatch
The `visitBinary()` in `UseInstanceOfPatternMatching` was visiting the left branch twice. This may not sound that bad, but a `J.Binary` will often be heavily skewed, like for example the tree for `a || b || c || d`: ``` || / \ || d / \ || c / \ a b ``` By visiting the left branch twice at every level, the `a` leaf will be visited a total of 8 times. This grows exponentially with the tree depth.
1 parent eee4327 commit f0ed8f9

File tree

1 file changed

+22
-22
lines changed

1 file changed

+22
-22
lines changed

src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -399,51 +399,51 @@ public UseInstanceOfPatternMatching(InstanceOfPatternReplacements replacements)
399399
}
400400

401401
@Override
402-
public J visitBinary(J.Binary original, Integer integer) {
403-
Expression newLeft = (Expression) super.visitNonNull(original.getLeft(), integer);
404-
if (newLeft != original.getLeft()) {
402+
public J visitBinary(J.Binary binary, Integer p) {
403+
J.Binary b = binary.withLeft((Expression) visitNonNull(binary.getLeft(), p));
404+
if (b.getLeft() != binary.getLeft()) {
405405
// The left side changed, so the right side should see any introduced variable names
406-
J.Binary replacement = original.withLeft(newLeft);
407-
Cursor widenedCursor = updateCursor(replacement);
406+
Cursor widenedCursor = updateCursor(b);
408407

409408
Expression newRight;
410-
if (original.getRight() instanceof J.InstanceOf) {
411-
newRight = replacements.processInstanceOf((J.InstanceOf) original.getRight(), widenedCursor);
412-
} else if (original.getRight() instanceof J.Parentheses &&
413-
((J.Parentheses<?>) original.getRight()).getTree() instanceof J.InstanceOf) {
409+
if (binary.getRight() instanceof J.InstanceOf) {
410+
newRight = replacements.processInstanceOf((J.InstanceOf) binary.getRight(), widenedCursor);
411+
} else if (binary.getRight() instanceof J.Parentheses &&
412+
((J.Parentheses<?>) binary.getRight()).getTree() instanceof J.InstanceOf) {
414413
@SuppressWarnings("unchecked")
415-
J.Parentheses<J.InstanceOf> originalRight = (J.Parentheses<J.InstanceOf>) original.getRight();
414+
J.Parentheses<J.InstanceOf> originalRight = (J.Parentheses<J.InstanceOf>) binary.getRight();
416415
newRight = originalRight.withTree(replacements.processInstanceOf(originalRight.getTree(), widenedCursor));
417416
} else {
418-
newRight = (Expression) super.visitNonNull(original.getRight(), integer, widenedCursor);
417+
newRight = (Expression) visitNonNull(binary.getRight(), p, widenedCursor);
419418
}
420-
return replacement.withRight(newRight);
419+
return b.withRight(newRight);
420+
} else {
421+
// The left side didn't change, so the right side doesn't need to see any introduced variable names
422+
return b.withRight((Expression) visitNonNull(binary.getRight(), p));
421423
}
422-
// The left side didn't change, so the right side doesn't need to see any introduced variable names
423-
return super.visitBinary(original, integer);
424424
}
425425

426426
@Override
427-
public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, Integer executionContext) {
428-
instanceOf = (J.InstanceOf) super.visitInstanceOf(instanceOf, executionContext);
427+
public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, Integer p) {
428+
instanceOf = (J.InstanceOf) super.visitInstanceOf(instanceOf, p);
429429
instanceOf = replacements.processInstanceOf(instanceOf, getCursor());
430430
return instanceOf;
431431
}
432432

433433
@Override
434-
public <T extends J> J visitParentheses(J.Parentheses<T> parens, Integer executionContext) {
434+
public <T extends J> J visitParentheses(J.Parentheses<T> parens, Integer p) {
435435
if (parens.getTree() instanceof J.TypeCast) {
436436
J replacement = replacements.processTypeCast((J.TypeCast) parens.getTree(), getCursor());
437437
if (replacement != null) {
438438
return replacement.withPrefix(parens.getPrefix());
439439
}
440440
}
441-
return super.visitParentheses(parens, executionContext);
441+
return super.visitParentheses(parens, p);
442442
}
443443

444444
@Override
445-
public J visitTypeCast(J.TypeCast typeCast, Integer executionContext) {
446-
typeCast = (J.TypeCast) super.visitTypeCast(typeCast, executionContext);
445+
public J visitTypeCast(J.TypeCast typeCast, Integer p) {
446+
typeCast = (J.TypeCast) super.visitTypeCast(typeCast, p);
447447
J replacement = replacements.processTypeCast(typeCast, getCursor());
448448
if (replacement != null) {
449449
return replacement;
@@ -453,8 +453,8 @@ public J visitTypeCast(J.TypeCast typeCast, Integer executionContext) {
453453

454454
@Override
455455
@SuppressWarnings("NullableProblems")
456-
public @Nullable J visitVariableDeclarations(J.VariableDeclarations multiVariable, Integer integer) {
457-
multiVariable = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, integer);
456+
public @Nullable J visitVariableDeclarations(J.VariableDeclarations multiVariable, Integer p) {
457+
multiVariable = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, p);
458458
return replacements.processVariableDeclarations(multiVariable);
459459
}
460460
}

0 commit comments

Comments
 (0)