From b0b83cef4c63fdca4244226f96086f3640a4e2e7 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 14 Dec 2017 16:08:12 +0700 Subject: [PATCH] GuessManagerImpl DFA: CoW myStates; onlyForPlace For normal completion we don't need to track expression types for expressions other than myForPlace Also changes in myState are much more rare than memory state copying, thus using copy-on-write strategy on myState seems rewarding Fixes IDEA-183497 Slow completion in somehow long method with workflow --- .../guess/impl/ExpressionTypeMemoryState.java | 25 +++++++++-- .../guess/impl/GuessManagerImpl.java | 41 +++++++++++-------- .../dataFlow/ControlFlowAnalyzer.java | 4 +- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/ExpressionTypeMemoryState.java b/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/ExpressionTypeMemoryState.java index 53b83a616775..a187dd75533c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/ExpressionTypeMemoryState.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/ExpressionTypeMemoryState.java @@ -53,7 +53,8 @@ public class ExpressionTypeMemoryState extends DfaMemoryStateImpl { return false; } }; - private final MultiMap myStates = MultiMap.createSet(EXPRESSION_HASHING_STRATEGY); + // may be shared between memory state instances + private MultiMap myStates = MultiMap.createSet(EXPRESSION_HASHING_STRATEGY); public ExpressionTypeMemoryState(final DfaValueFactory factory) { super(factory); @@ -67,7 +68,7 @@ public class ExpressionTypeMemoryState extends DfaMemoryStateImpl { @Override public DfaMemoryStateImpl createCopy() { final ExpressionTypeMemoryState copy = new ExpressionTypeMemoryState(this); - copy.myStates.putAllValues(myStates); + copy.myStates = myStates; return copy; } @@ -77,6 +78,7 @@ public class ExpressionTypeMemoryState extends DfaMemoryStateImpl { return false; } MultiMap thatStates = ((ExpressionTypeMemoryState)that).myStates; + if (thatStates == myStates) return true; for (Map.Entry> entry : myStates.entrySet()) { Collection thisTypes = entry.getValue(); Collection thatTypes = thatStates.get(entry.getKey()); @@ -126,7 +128,24 @@ public class ExpressionTypeMemoryState extends DfaMemoryStateImpl { return super.toString() + " states=[" + myStates + "]"; } + void removeExpressionType(@NotNull PsiExpression expression) { + if (myStates.containsKey(expression)) { + MultiMap oldStates = myStates; + myStates = MultiMap.createSet(EXPRESSION_HASHING_STRATEGY); + for (Map.Entry> entry : oldStates.entrySet()) { + if(!EXPRESSION_HASHING_STRATEGY.equals(entry.getKey(), expression)) { + myStates.putValues(entry.getKey(), entry.getValue()); + } + } + } + } + void setExpressionType(@NotNull PsiExpression expression, @NotNull PsiType type) { - myStates.putValue(expression, type); + if (!myStates.get(expression).contains(type)) { + MultiMap oldStates = myStates; + myStates = MultiMap.createSet(EXPRESSION_HASHING_STRATEGY); + myStates.putAllValues(oldStates); + myStates.putValue(expression, type); + } } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java index 62029fe07270..1350792530ce 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java @@ -36,6 +36,7 @@ import com.intellij.util.BitUtil; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; import com.siyeh.ig.psiutils.ExpressionUtils; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -130,12 +131,12 @@ public class GuessManagerImpl extends GuessManager { @NotNull @Override public MultiMap getControlFlowExpressionTypes(@NotNull final PsiExpression forPlace) { - MultiMap typeMap = buildDataflowTypeMap(forPlace); + MultiMap typeMap = buildDataflowTypeMap(forPlace, false); return typeMap != null ? typeMap : MultiMap.empty(); } @Nullable - private static MultiMap buildDataflowTypeMap(PsiExpression forPlace) { + private static MultiMap buildDataflowTypeMap(PsiExpression forPlace, boolean place) { PsiElement scope = DfaPsiUtil.getTopmostBlockInSameClass(forPlace); if (scope == null) { PsiFile file = forPlace.getContainingFile(); @@ -154,7 +155,7 @@ public class GuessManagerImpl extends GuessManager { } }; - final ExpressionTypeInstructionVisitor visitor = new ExpressionTypeInstructionVisitor(forPlace); + final ExpressionTypeInstructionVisitor visitor = new ExpressionTypeInstructionVisitor(forPlace, place); if (runner.analyzeMethodWithInlining(scope, visitor) == RunnerResult.OK) { return visitor.getResult(); } @@ -406,7 +407,7 @@ public class GuessManagerImpl extends GuessManager { return Collections.emptyList(); //optimization } - MultiMap fromDfa = buildDataflowTypeMap(expr); + MultiMap fromDfa = buildDataflowTypeMap(expr, true); if (fromDfa != null) { Collection conjuncts = fromDfa.get(expr); if (!conjuncts.isEmpty()) { @@ -437,14 +438,11 @@ public class GuessManagerImpl extends GuessManager { private MultiMap myResult; private final PsiElement myForPlace; private TypeConstraint myConstraint = null; + private final boolean myOnlyForPlace; - private ExpressionTypeInstructionVisitor(@NotNull PsiElement forPlace) { - PsiElement parent = PsiUtil.skipParenthesizedExprUp(forPlace.getParent()); - if (forPlace instanceof PsiThisExpression && parent instanceof PsiReferenceExpression) { - myForPlace = parent.getParent() instanceof PsiMethodCallExpression ? parent.getParent() : parent; - } else { - myForPlace = forPlace; - } + private ExpressionTypeInstructionVisitor(@NotNull PsiElement forPlace, boolean onlyForPlace) { + myOnlyForPlace = onlyForPlace; + myForPlace = PsiUtil.skipParenthesizedExprUp(forPlace); } MultiMap getResult() { @@ -460,21 +458,33 @@ public class GuessManagerImpl extends GuessManager { return myResult; } + @Contract("null -> false") + private boolean isInteresting(PsiExpression expression) { + if (expression == null) return false; + return !myOnlyForPlace || + (myForPlace instanceof PsiExpression && + ExpressionTypeMemoryState.EXPRESSION_HASHING_STRATEGY.equals((PsiExpression)myForPlace, expression)); + } + @Override public DfaInstructionState[] visitInstanceof(InstanceofInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - if (instruction.getLeft() == null) { + PsiExpression psiOperand = instruction.getLeft(); + if (!isInteresting(psiOperand)) { return super.visitInstanceof(instruction, runner, memState); } DfaValue type = memState.pop(); DfaValue operand = memState.pop(); DfaValue relation = runner.getFactory().createCondition(operand, DfaRelationValue.RelationType.IS, type); - memState.push(new DfaInstanceofValue(runner.getFactory(), instruction.getLeft(), instruction.getCastType(), relation, false)); + memState.push(new DfaInstanceofValue(runner.getFactory(), psiOperand, instruction.getCastType(), relation, false)); return new DfaInstructionState[]{new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState)}; } @Override public DfaInstructionState[] visitTypeCast(TypeCastInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - ((ExpressionTypeMemoryState) memState).setExpressionType(instruction.getCasted(), instruction.getCastTo()); + PsiExpression psiOperand = instruction.getCasted(); + if (isInteresting(psiOperand)) { + ((ExpressionTypeMemoryState)memState).setExpressionType(psiOperand, instruction.getCastTo()); + } return super.visitTypeCast(instruction, runner, memState); } @@ -483,8 +493,7 @@ public class GuessManagerImpl extends GuessManager { PsiExpression left = instruction.getLExpression(); PsiExpression right = instruction.getRExpression(); if (left != null && right != null) { - MultiMap states = ((ExpressionTypeMemoryState)memState).getStates(); - states.remove(left); + ((ExpressionTypeMemoryState)memState).removeExpressionType(left); } return super.visitAssign(instruction, runner, memState); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 003f660c4907..57c70915cd86 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -1759,7 +1759,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitSuperExpression(PsiSuperExpression expression) { startElement(expression); - addInstruction(new PushInstruction(myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), null)); + addInstruction(new PushInstruction(myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), expression)); finishElement(expression); } @@ -1769,7 +1769,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (myThisReadOnly) { value = myFactory.withFact(value, DfaFactType.MUTABLE, false); } - addInstruction(new PushInstruction(value, null)); + addInstruction(new PushInstruction(value, expression)); finishElement(expression); }