From 0f0d07c9fe6e459eb17d0ad722dd8c598242782c Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Tue, 8 Jul 2014 12:11:29 +0400 Subject: [PATCH] Refactoring: extracted specific logic into RedundantBoxedValuesCollection class --- .../optimization/boxing/BoxedBasicValue.java | 28 ++------ .../RedundantBoxedValuesCollection.java | 65 +++++++++++++++++++ .../boxing/RedundantBoxingInterpreter.java | 19 +++--- .../RedundantBoxingMethodTransformer.java | 42 +++++------- 4 files changed, 97 insertions(+), 57 deletions(-) create mode 100644 compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxedValuesCollection.java diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/BoxedBasicValue.java b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/BoxedBasicValue.java index be077a25e28..5030fe1f089 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/BoxedBasicValue.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/BoxedBasicValue.java @@ -88,31 +88,17 @@ public class BoxedBasicValue extends BasicValue { return primitiveType; } - public void mergeWith(BoxedBasicValue value) { - if (this == value) { - return; - } - - if (!value.isSafeToRemove) { - propagateRemovingAsUnsafe(); - } - - if (!isSafeToRemove) { - value.propagateRemovingAsUnsafe(); - } - + public void addMergedWith(@NotNull BoxedBasicValue value) { mergedWith.add(value); - value.mergedWith.add(this); } - public void propagateRemovingAsUnsafe() { - isSafeToRemove = false; + @NotNull + public Iterable getMergedWith() { + return mergedWith; + } - for (BoxedBasicValue value : mergedWith) { - if (value.isSafeToRemove()) { - value.propagateRemovingAsUnsafe(); - } - } + public void markAsUnsafeToRemove() { + isSafeToRemove = false; } public boolean isSafeToRemove() { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxedValuesCollection.java b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxedValuesCollection.java new file mode 100644 index 00000000000..87d60195e09 --- /dev/null +++ b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxedValuesCollection.java @@ -0,0 +1,65 @@ +/* + * Copyright 2010-2014 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.jet.codegen.optimization.boxing; + +import org.jetbrains.annotations.NotNull; + +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; + +public class RedundantBoxedValuesCollection implements Iterable { + private final Set safeToDeleteValues = new HashSet(); + + public void add(@NotNull BoxedBasicValue value) { + safeToDeleteValues.add(value); + } + + public void remove(@NotNull BoxedBasicValue value) { + if (safeToDeleteValues.contains(value)) { + safeToDeleteValues.remove(value); + value.markAsUnsafeToRemove(); + + for (BoxedBasicValue mergedValue : value.getMergedWith()) { + remove(mergedValue); + } + } + } + + public void merge(@NotNull BoxedBasicValue v, @NotNull BoxedBasicValue w) { + v.addMergedWith(w); + w.addMergedWith(v); + + if (v.isSafeToRemove() && !w.isSafeToRemove()) { + remove(v); + } + + if (!v.isSafeToRemove() && w.isSafeToRemove()) { + remove(w); + } + } + + public boolean isEmpty() { + return safeToDeleteValues.isEmpty(); + } + + @NotNull + @Override + public Iterator iterator() { + return safeToDeleteValues.iterator(); + } +} diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingInterpreter.java b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingInterpreter.java index 293aad513b2..484d8919f1d 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingInterpreter.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingInterpreter.java @@ -28,9 +28,6 @@ import org.jetbrains.org.objectweb.asm.tree.VarInsnNode; import org.jetbrains.org.objectweb.asm.tree.analysis.AnalyzerException; import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue; -import java.util.HashSet; -import java.util.Set; - class RedundantBoxingInterpreter extends BoxingInterpreter { private static final ImmutableSet UNSAFE_OPERATIONS_OPCODES; @@ -44,14 +41,14 @@ class RedundantBoxingInterpreter extends BoxingInterpreter { UNSAFE_OPERATIONS_OPCODES = unsafeOperationsOpcodesBuilder.build(); } - private final Set candidatesBoxedValues = new HashSet(); + private final RedundantBoxedValuesCollection values = new RedundantBoxedValuesCollection(); public RedundantBoxingInterpreter(InsnList insnList) { super(insnList); } - private static void markValueAsDirty(@NotNull BoxedBasicValue value) { - value.propagateRemovingAsUnsafe(); + private void markValueAsDirty(@NotNull BoxedBasicValue value) { + values.remove(value); } private static void addAssociatedInsn(@NotNull BoxedBasicValue value, @NotNull AbstractInsnNode insn) { @@ -60,7 +57,7 @@ class RedundantBoxingInterpreter extends BoxingInterpreter { } } - private static void processOperationWithBoxedValue(@Nullable BasicValue value, @NotNull AbstractInsnNode insnNode) { + private void processOperationWithBoxedValue(@Nullable BasicValue value, @NotNull AbstractInsnNode insnNode) { if (value instanceof BoxedBasicValue) { if (UNSAFE_OPERATIONS_OPCODES.contains(insnNode.getOpcode())) { markValueAsDirty((BoxedBasicValue) value); @@ -141,7 +138,7 @@ class RedundantBoxingInterpreter extends BoxingInterpreter { @Override protected void onNewBoxedValue(@NotNull BoxedBasicValue value) { - candidatesBoxedValues.add(value); + values.add(value); } @Override @@ -170,11 +167,11 @@ class RedundantBoxingInterpreter extends BoxingInterpreter { protected void onMergeSuccess( @NotNull BoxedBasicValue v, @NotNull BoxedBasicValue w ) { - v.mergeWith(w); + values.merge(v, w); } @NotNull - public Set getCandidatesBoxedValues() { - return candidatesBoxedValues; + public RedundantBoxedValuesCollection getCandidatesBoxedValues() { + return values; } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingMethodTransformer.java b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingMethodTransformer.java index 5d95387abb6..bd70f44dcb7 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingMethodTransformer.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingMethodTransformer.java @@ -16,8 +16,6 @@ package org.jetbrains.jet.codegen.optimization.boxing; -import com.google.common.base.Predicate; -import com.google.common.collect.Collections2; import com.intellij.openapi.util.Pair; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.codegen.optimization.OptimizationUtils; @@ -44,12 +42,11 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { Analyzer analyzer = new Analyzer(interpreter); Frame[] frames = runAnalyzer(analyzer, internalClassName, node); - Set valuesToOptimize = filterSafeToRemoveValues(interpreter.getCandidatesBoxedValues()); + RedundantBoxedValuesCollection valuesToOptimize = interpreter.getCandidatesBoxedValues(); if (!valuesToOptimize.isEmpty()) { - findValuesClashingWithVariables(node, frames); - - valuesToOptimize = filterSafeToRemoveValues(valuesToOptimize); + // has side effect on valuesToOptimize and frames, containing BoxedBasicValues that are unsafe to remove + removeValuesClashingWithVariables(valuesToOptimize, node, frames); adaptLocalVariableTableForBoxedValues(node, frames); @@ -61,16 +58,6 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { super.transform(internalClassName, node); } - @NotNull - private static Set filterSafeToRemoveValues(@NotNull Set values) { - return new HashSet(Collections2.filter(values, new Predicate() { - @Override - public boolean apply(BoxedBasicValue input) { - return input.isSafeToRemove(); - } - })); - } - private static void adaptLocalVariableTableForBoxedValues(@NotNull MethodNode node, @NotNull Frame[] frames) { for (LocalVariableNode localVariableNode : node.localVariables) { if (Type.getType(localVariableNode.desc).getSort() != Type.OBJECT) { @@ -107,16 +94,20 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { } } - private static void findValuesClashingWithVariables( - @NotNull MethodNode node, @NotNull Frame[] frames + private static void removeValuesClashingWithVariables( + @NotNull RedundantBoxedValuesCollection values, + @NotNull MethodNode node, + @NotNull Frame[] frames ) { - while (findValuesClashingWithVariablesPass(node, frames)) { + while (removeValuesClashingWithVariablesPass(values, node, frames)) { // do nothing } } - private static boolean findValuesClashingWithVariablesPass( - @NotNull MethodNode node, @NotNull Frame[] frames + private static boolean removeValuesClashingWithVariablesPass( + @NotNull RedundantBoxedValuesCollection values, + @NotNull MethodNode node, + @NotNull Frame[] frames ) { InsnList insnList = node.instructions; boolean needToRepeat = false; @@ -131,7 +122,7 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { int to = insnList.indexOf(localVariableNode.end) - 1; if (isThereUnsafeStoreInstruction(insnList, frames, from, to, index)) { - needToRepeat |= markAllBoxedValuesStoredAsUnsafeToRemove(insnList, frames, from, to, index); + needToRepeat |= markAllBoxedValuesStoredAsUnsafeToRemove(insnList, frames, values, from, to, index); } } @@ -183,6 +174,7 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { private static boolean markAllBoxedValuesStoredAsUnsafeToRemove( @NotNull InsnList insnList, @NotNull Frame[] frames, + @NotNull RedundantBoxedValuesCollection values, int from, int to, int varIndex ) { boolean wasChanges = false; @@ -200,7 +192,7 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { } wasChanges |= true; - ((BoxedBasicValue) top).propagateRemovingAsUnsafe(); + values.remove((BoxedBasicValue) top); } } @@ -208,7 +200,7 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { } @NotNull - private static int[] buildVariablesRemapping(@NotNull Iterable values, @NotNull MethodNode node) { + private static int[] buildVariablesRemapping(@NotNull RedundantBoxedValuesCollection values, @NotNull MethodNode node) { Set doubleSizedVars = new HashSet(); for (BoxedBasicValue value : values) { if (value.getPrimitiveType().getSize() == 2) { @@ -247,7 +239,7 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer { } private static void adaptInstructionsForBoxedValues( - @NotNull MethodNode node, @NotNull Frame[] frames, @NotNull Set values + @NotNull MethodNode node, @NotNull Frame[] frames, @NotNull RedundantBoxedValuesCollection values ) { addPopInstructionsForBoxedValues(node, frames);