Revert "Log minification state for instabug methods"
This reverts commit ea1cb335b58563ccfa170d58853d34add657b1a6.
Reason for revert: Landed temporarily for debugging b/123730537.
Change-Id: I597a87cbf379bed2a010cd0fedcdb7ac903a6bd2
diff --git a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
index dfdd515..718ce78 100644
--- a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
@@ -19,7 +19,6 @@
import com.android.tools.r8.utils.Timing;
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collections;
@@ -30,7 +29,6 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
-import java.util.stream.Collectors;
class InterfaceMethodNameMinifier {
@@ -77,6 +75,7 @@
private void reserveNamesInInterfaces() {
for (DexType type : DexType.allInterfaces(appInfo.dexItemFactory)) {
assert type.isInterface();
+ frontierState.put(type, type);
frontierState.allocateNamingStateAndReserve(type, type, null);
}
}
@@ -89,16 +88,14 @@
// frontier states of classes that implement them. We add the frontier states so that we can
// reserve the names for later method naming.
timing.begin("Compute map");
- Map<DexType, Set<NamingState<DexProto, ?>>> reachableStatesCache = new IdentityHashMap<>();
for (DexType type : DexType.allInterfaces(appInfo.dexItemFactory)) {
assert type.isInterface();
DexClass clazz = appInfo.definitionFor(type);
if (clazz != null) {
assert clazz.isInterface();
- Set<NamingState<DexProto, ?>> collectedStates =
- getReachableStates(type, reachableStatesCache);
+ Set<NamingState<DexProto, ?>> collectedStates = getReachableStates(type);
for (DexEncodedMethod method : shuffleMethods(clazz.methods(), options)) {
- addStatesToGlobalMapForMethod(method.method, collectedStates, type);
+ addStatesToGlobalMapForMethod(method, collectedStates, type);
}
}
}
@@ -132,9 +129,8 @@
for (DexEncodedMethod method : implementedMethods) {
DexType iface = method.method.holder;
assert iface.isInterface();
- Set<NamingState<DexProto, ?>> collectedStates =
- getReachableStates(iface, reachableStatesCache);
- addStatesToGlobalMapForMethod(method.method, collectedStates, iface);
+ Set<NamingState<DexProto, ?>> collectedStates = getReachableStates(iface);
+ addStatesToGlobalMapForMethod(method, collectedStates, iface);
callSiteMethods.add(equivalence.wrap(method.method));
}
if (callSiteMethods.size() > 1) {
@@ -160,19 +156,31 @@
timing.end();
// Go over every method and assign a name.
timing.begin("Allocate names");
-
// Sort the methods by the number of dependent states, so that we use short names for methods
// that are referenced in many places.
- List<Wrapper<DexMethod>> interfaceMethods =
- globalStateMap.keySet().stream()
- .filter(wrapper -> unificationParent.getOrDefault(wrapper, wrapper).equals(wrapper))
- // TODO(christofferqa): Does this actually matter?
- .sorted((a, b) -> globalStateMap.get(b).size() - globalStateMap.get(a).size())
- .collect(Collectors.toList());
- for (Wrapper<DexMethod> key : interfaceMethods) {
- assignNameToInterfaceMethod(key, unification);
+ List<Wrapper<DexMethod>> methods = new ArrayList<>(globalStateMap.keySet());
+ methods.sort((a, b) -> globalStateMap.get(b).size() - globalStateMap.get(a).size());
+ for (Wrapper<DexMethod> key : methods) {
+ if (!unificationParent.getOrDefault(key, key).equals(key)) {
+ continue;
+ }
+ List<MethodNamingState> collectedStates = new ArrayList<>();
+ Set<DexMethod> sourceMethods = Sets.newIdentityHashSet();
+ for (Wrapper<DexMethod> k : unification.getOrDefault(key, Collections.singleton(key))) {
+ DexMethod unifiedMethod = k.get();
+ assert unifiedMethod != null;
+ sourceMethods.addAll(sourceMethodsMap.get(k));
+ for (NamingState<DexProto, ?> namingState : globalStateMap.get(k)) {
+ collectedStates.add(
+ new MethodNamingState(namingState, unifiedMethod.name, unifiedMethod.proto));
+ }
+ }
+ DexMethod method = key.get();
+ assert method != null;
+ MethodNamingState originState =
+ new MethodNamingState(originStates.get(key), method.name, method.proto);
+ assignNameForInterfaceMethodInAllStates(collectedStates, sourceMethods, originState);
}
-
for (Entry<DexCallSite, DexMethod> entry : callSites.entrySet()) {
DexMethod method = entry.getValue();
DexString renamed = minifierState.getRenaming(method);
@@ -187,49 +195,6 @@
timing.end();
}
- private void assignNameToInterfaceMethod(
- Wrapper<DexMethod> key, Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification) {
- List<MethodNamingState> collectedStates = new ArrayList<>();
- Set<DexMethod> sourceMethods = Sets.newIdentityHashSet();
- for (Wrapper<DexMethod> k : unification.getOrDefault(key, Collections.singleton(key))) {
- DexMethod unifiedMethod = k.get();
- assert unifiedMethod != null;
- sourceMethods.addAll(sourceMethodsMap.get(k));
- for (NamingState<DexProto, ?> namingState : globalStateMap.get(k)) {
- collectedStates.add(
- new MethodNamingState(namingState, unifiedMethod.name, unifiedMethod.proto));
- }
- }
-
- DexMethod method = key.get();
- assert method != null;
-
- boolean isMethodOfInterest =
- sourceMethods.stream()
- .anyMatch(
- sourceMethod ->
- sourceMethod.toSourceString().equals("void com.instabug.bug.view.b$b.b()")
- || sourceMethod
- .toSourceString()
- .equals("void com.instabug.bug.view.b$b.c()"));
- if (isMethodOfInterest) {
- System.out.println("-----------------------------------------------------------------------");
- System.out.println("assignNameToInterfaceMethod(`" + method.toSourceString() + "`)");
- System.out.println("-----------------------------------------------------------------------");
- System.out.println("Source methods:");
- for (DexMethod sourceMethod : sourceMethods) {
- System.out.println(" " + sourceMethod.toSourceString());
- }
- System.out.println("States:");
- collectedStates.forEach(state -> state.print(" ", minifierState::getStateKey, System.out));
- System.out.println();
- }
-
- MethodNamingState originState =
- new MethodNamingState(originStates.get(key), method.name, method.proto);
- assignNameForInterfaceMethodInAllStates(collectedStates, sourceMethods, originState);
- }
-
private void assignNameForInterfaceMethodInAllStates(
List<MethodNamingState> collectedStates,
Set<DexMethod> sourceMethods,
@@ -248,13 +213,11 @@
DexString candidate;
do {
candidate = originState.assignNewName();
-
- // If the state returns the same candidate for two consecutive trials, it should be an
- // interface method with the same signature (name, param) but different return type has
- // been already renamed; and 2) -useuniqueclassmembernames is set.
- //
- // The option forces the naming state to return the same renamed name for the same
- // signature. We therefore break the loop in an ad-hoc manner here.
+ // If the state returns the same candidate for two consecutive trials, it should be this case:
+ // 1) an interface method with the same signature (name, param) but different return type
+ // has been already renamed; and 2) -useuniqueclassmembernames is set.
+ // The option forces the naming state to return the same renamed name for the same signature.
+ // So, here we break the loop in an ad-hoc manner.
if (candidate != null && candidate == previousCandidate) {
assert minifierState.useUniqueMemberNames();
break;
@@ -277,10 +240,14 @@
}
private void addStatesToGlobalMapForMethod(
- DexMethod method, Set<NamingState<DexProto, ?>> collectedStates, DexType originInterface) {
- Wrapper<DexMethod> key = equivalence.wrap(method);
- globalStateMap.computeIfAbsent(key, k -> new HashSet<>()).addAll(collectedStates);
- sourceMethodsMap.computeIfAbsent(key, k -> new HashSet<>()).add(method);
+ DexEncodedMethod method,
+ Set<NamingState<DexProto, ?>> collectedStates,
+ DexType originInterface) {
+ Wrapper<DexMethod> key = equivalence.wrap(method.method);
+ Set<NamingState<DexProto, ?>> stateSet =
+ globalStateMap.computeIfAbsent(key, k -> new HashSet<>());
+ stateSet.addAll(collectedStates);
+ sourceMethodsMap.computeIfAbsent(key, k -> new HashSet<>()).add(method.method);
originStates.putIfAbsent(key, minifierState.getState(originInterface));
}
@@ -300,35 +267,21 @@
return false;
}
- private Set<NamingState<DexProto, ?>> getReachableStates(
- DexType type, Map<DexType, Set<NamingState<DexProto, ?>>> reachableStatesCache) {
- if (minifierState.useUniqueMemberNames()) {
- return ImmutableSet.of(minifierState.globalState());
- }
-
- if (reachableStatesCache.containsKey(type)) {
- return reachableStatesCache.get(type);
- }
-
- Set<DexType> reachableInterfaces = Sets.newIdentityHashSet();
- reachableInterfaces.add(type);
- collectSuperInterfaces(type, reachableInterfaces);
- collectSubInterfaces(type, reachableInterfaces);
-
+ private Set<NamingState<DexProto, ?>> getReachableStates(DexType type) {
+ Set<DexType> interfaces = Sets.newIdentityHashSet();
+ interfaces.add(type);
+ collectSuperInterfaces(type, interfaces);
+ collectSubInterfaces(type, interfaces);
Set<NamingState<DexProto, ?>> reachableStates = new HashSet<>();
- for (DexType reachableInterface : reachableInterfaces) {
- // Add the interface itself.
- reachableStates.add(minifierState.getState(reachableInterface));
-
+ for (DexType iface : interfaces) {
+ // Add the interface itself
+ reachableStates.add(minifierState.getState(iface));
// And the frontiers that correspond to the classes that implement the interface.
- for (DexType frontier : reachableInterface.allImplementsSubtypes()) {
- NamingState<DexProto, ?> state = minifierState.getState(frontierState.get(frontier));
+ for (DexType t : iface.allImplementsSubtypes()) {
+ NamingState<DexProto, ?> state = minifierState.getState(frontierState.get(t));
assert state != null;
reachableStates.add(state);
}
-
- // Cache the result.
- reachableStatesCache.put(reachableInterface, reachableStates);
}
return reachableStates;
}
diff --git a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
index 419bfbb..8117dd5 100644
--- a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
@@ -69,14 +69,6 @@
return useUniqueMemberNames ? globalState : states.get(type);
}
- DexType getStateKey(NamingState<StateType, ?> state) {
- return states.inverse().get(state);
- }
-
- NamingState<StateType, ?> globalState() {
- return globalState;
- }
-
boolean isReservedInGlobalState(DexString name, StateType state) {
return globalState.isReserved(name, state);
}
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
index 8864271..4b67208 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -19,7 +19,6 @@
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableMap;
-import java.io.PrintStream;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Map;
@@ -133,7 +132,8 @@
// Phase 1: Reserve all the names that need to be kept and allocate linked state in the
// library part.
timing.begin("Phase 1");
- reserveNamesInClasses();
+ reserveNamesInClasses(
+ appInfo.dexItemFactory.objectType, appInfo.dexItemFactory.objectType, null);
timing.end();
// Phase 2: Reserve all the names that are required for interfaces, and then assign names to
// interface methods. These are assigned by finding a name that is free in all naming
@@ -195,24 +195,30 @@
}
}
- private void reserveNamesInClasses() {
- reserveNamesInClasses(
- appInfo.dexItemFactory.objectType, appInfo.dexItemFactory.objectType, null);
- }
-
private void reserveNamesInClasses(
DexType type, DexType libraryFrontier, NamingState<DexProto, ?> parent) {
assert !type.isInterface();
NamingState<DexProto, ?> state =
frontierState.allocateNamingStateAndReserve(type, libraryFrontier, parent);
-
// If this is a library class (or effectively a library class as it is missing) move the
// frontier forward.
DexClass holder = appInfo.definitionFor(type);
- for (DexType subtype : type.allExtendsSubtypes()) {
- assert !subtype.isInterface();
- reserveNamesInClasses(
- subtype, holder == null || holder.isLibraryClass() ? subtype : libraryFrontier, state);
+ type.forAllExtendsSubtypes(
+ subtype -> {
+ assert !subtype.isInterface();
+ reserveNamesInClasses(
+ subtype,
+ holder == null || holder.isLibraryClass() ? subtype : libraryFrontier,
+ state);
+ });
+ }
+
+ private void reserveNamesForMethod(
+ DexEncodedMethod encodedMethod, boolean keepAll, NamingState<DexProto, ?> state) {
+ DexMethod method = encodedMethod.method;
+ if (keepAll || rootSet.noObfuscation.contains(method)) {
+ state.reserveName(method.name, method.proto);
+ globalState.reserveName(method.name, method.proto);
}
}
@@ -221,14 +227,12 @@
private final Map<DexType, DexType> frontiers = new IdentityHashMap<>();
NamingState<DexProto, ?> allocateNamingStateAndReserve(
- DexType type, DexType frontier, NamingState<DexProto, ?> parent) {
- if (frontier != type) {
- frontiers.put(type, frontier);
- }
+ DexType type, DexType libraryFrontier, NamingState<DexProto, ?> parent) {
+ frontiers.put(type, libraryFrontier);
NamingState<DexProto, ?> state =
computeStateIfAbsent(
- frontier,
+ libraryFrontier,
ignore ->
parent == null
? NamingState.createRoot(
@@ -242,28 +246,18 @@
if (holder != null) {
boolean keepAll = holder.isLibraryClass() || holder.accessFlags.isAnnotation();
for (DexEncodedMethod method : shuffleMethods(holder.methods(), options)) {
- // TODO(christofferqa): Wouldn't it be sufficient only to reserve names for non-private
- // methods?
- if (keepAll || rootSet.noObfuscation.contains(method.method)) {
- reserveNamesForMethod(method.method, state);
- }
+ reserveNamesForMethod(method, keepAll, state);
}
}
return state;
}
- private void reserveNamesForMethod(DexMethod method, NamingState<DexProto, ?> state) {
- state.reserveName(method.name, method.proto);
- globalState.reserveName(method.name, method.proto);
- }
-
public DexType get(DexType type) {
- return frontiers.getOrDefault(type, type);
+ return frontiers.get(type);
}
public DexType put(DexType type, DexType frontier) {
- assert frontier != type;
return frontiers.put(type, frontier);
}
}
@@ -313,19 +307,6 @@
DexProto getProto() {
return proto;
}
-
- void print(
- String indentation,
- Function<NamingState<DexProto, ?>, DexType> stateKeyGetter,
- PrintStream out) {
- DexType stateKey = stateKeyGetter.apply(parent);
- out.print(indentation);
- out.print(stateKey != null ? stateKey.toSourceString() : "<?>");
- out.print(".");
- out.print(name.toSourceString());
- out.println(proto.toSmaliString());
- parent.printState(proto, indentation + " ", out);
- }
}
// Shuffles the given methods if assertions are enabled and deterministic debugging is disabled.
diff --git a/src/main/java/com/android/tools/r8/naming/NamingState.java b/src/main/java/com/android/tools/r8/naming/NamingState.java
index 6f6df61..f0f1720 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingState.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingState.java
@@ -11,12 +11,10 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.Table;
-import java.io.PrintStream;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Function;
@@ -131,18 +129,6 @@
state.addRenaming(original, key, newName);
}
- public void printState(ProtoType proto, String indentation, PrintStream out) {
- KeyType key = keyTransform.apply(proto);
- InternalState state = findInternalStateFor(key);
- if (state != null) {
- state.printReservedNames(indentation, out);
- state.printRenamings(indentation, out);
- } else {
- out.print(indentation);
- out.println("<NO STATE>");
- }
- }
-
class InternalState {
private static final int INITIAL_NAME_COUNT = 1;
@@ -243,49 +229,5 @@
return StringUtils.numberToIdentifier(EMPTY_CHAR_ARRAY, nameCount++, false);
}
}
-
- void printReservedNames(String indentation, PrintStream out) {
- out.print(indentation);
- out.print("Reserved names:");
- if (reservedNames == null || reservedNames.isEmpty()) {
- out.print(" <NO RESERVED NAMES>");
- } else {
- for (DexString reservedName : reservedNames) {
- out.print(System.lineSeparator());
- out.print(indentation);
- out.print(" ");
- out.print(reservedName.toSourceString());
- }
- }
- out.println();
- if (parentInternalState != null) {
- parentInternalState.printReservedNames(indentation + " ", out);
- }
- }
-
- void printRenamings(String indentation, PrintStream out) {
- out.print(indentation);
- out.print("Renamings:");
- if (renamings == null || renamings.isEmpty()) {
- out.print(" <NO RENAMINGS>");
- } else {
- for (DexString original : renamings.rowKeySet()) {
- Map<KeyType, DexString> row = renamings.row(original);
- for (Entry<KeyType, DexString> entry : row.entrySet()) {
- out.print(System.lineSeparator());
- out.print(indentation);
- out.print(" ");
- out.print(original.toSourceString());
- out.print(entry.getKey().toString());
- out.print(" -> ");
- out.print(entry.getValue().toSourceString());
- }
- }
- }
- out.println();
- if (parentInternalState != null) {
- parentInternalState.printRenamings(indentation + " ", out);
- }
- }
}
}
