commit f58b55990d3fca2b3a4acf67ecba721cdc9c0262
author Ian Zerny <zerny@google.com> Thu Mar 19 09:10:13 2020 +0100
committer Ian Zerny <zerny@google.com> Thu Mar 19 08:53:37 2020 +0000
tree 0ad94464ffd75efaaa90e4e2a6cde36ee8f86033
parent 3d14bf21552d5cf616ed226f2c1b04aacc55efa4

Revert "Remove methods from definitionsFor cache."

This reverts commit 8ec568330915a0e5f522a0954c55fb87ee5d53f7.

Bug: 151878901
Change-Id: Id6c902f5e527c2f8b1eea029611b281d7f67102e

src/main/java/com/android/tools/r8/graph/AppInfo.java

diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java
index a41e119..2ddba3f 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -31,15 +31,11 @@
 
   private final DexApplication app;
   private final DexItemFactory dexItemFactory;
-
-  // TODO(b/151804585): Remove this cache.
-  private final ConcurrentHashMap<DexType, Map<DexField, DexEncodedField>> fieldDefinitionsCache =
-      new ConcurrentHashMap<>();
-
+  private final ConcurrentHashMap<DexType, Map<DexMember<?, ?>, DexEncodedMember<?, ?>>>
+      definitions = new ConcurrentHashMap<>();
   // For some optimizations, e.g. optimizing synthetic classes, we may need to resolve the current
   // class being optimized.
-  private final ConcurrentHashMap<DexType, DexProgramClass> synthesizedClasses =
-      new ConcurrentHashMap<>();
+  final ConcurrentHashMap<DexType, DexProgramClass> synthesizedClasses = new ConcurrentHashMap<>();
 
   // Set when a new AppInfo replaces a previous one. All public methods should verify that the
   // current instance is not obsolete, to ensure that we almost use the most recent AppInfo.
@@ -54,7 +50,7 @@
     assert !previous.isObsolete();
     this.app = previous.app;
     this.dexItemFactory = app.dexItemFactory;
-    this.fieldDefinitionsCache.putAll(previous.fieldDefinitionsCache);
+    this.definitions.putAll(previous.definitions);
     copyMetadataFromPrevious(previous);
   }
 
@@ -98,7 +94,7 @@
     assert checkIfObsolete();
     assert clazz.type.isD8R8SynthesizedClassType();
     DexProgramClass previous = synthesizedClasses.put(clazz.type, clazz);
-    invalidateFieldCacheFor(clazz.type);
+    invalidateTypeCacheFor(clazz.type);
     assert previous == null || previous == clazz;
   }
 
@@ -107,11 +103,12 @@
     return Collections.unmodifiableCollection(synthesizedClasses.values());
   }
 
-  private Map<DexField, DexEncodedField> computeFieldDefinitions(DexType type) {
-    Builder<DexField, DexEncodedField> builder = ImmutableMap.builder();
+  private Map<DexMember<?, ?>, DexEncodedMember<?, ?>> computeDefinitions(DexType type) {
+    Builder<DexMember<?, ?>, DexEncodedMember<?, ?>> builder = ImmutableMap.builder();
     DexClass clazz = definitionFor(type);
     if (clazz != null) {
-      clazz.forEachField(field -> builder.put(field.field, field));
+      clazz.forEachMethod(method -> builder.put(method.toReference(), method));
+      clazz.forEachField(field -> builder.put(field.toReference(), field));
     }
     return builder.build();
   }
@@ -176,29 +173,35 @@
   @Override
   public DexEncodedMethod definitionFor(DexMethod method) {
     assert checkIfObsolete();
-    assert method.holder.isClassType();
-    if (!method.holder.isClassType()) {
-      return null;
+    DexType holderType = method.holder;
+    DexEncodedMethod cached = (DexEncodedMethod) getDefinitions(holderType).get(method);
+    if (cached != null && cached.isObsolete()) {
+      definitions.remove(holderType);
+      cached = (DexEncodedMethod) getDefinitions(holderType).get(method);
     }
-    DexClass clazz = definitionFor(method.holder);
-    if (clazz == null) {
-      return null;
-    }
-    return clazz.getMethodCollection().getMethod(method);
+    return cached;
   }
 
   @Override
   public DexEncodedField definitionFor(DexField field) {
     assert checkIfObsolete();
-    return getFieldDefinitions(field.holder).get(field);
+    return (DexEncodedField) getDefinitions(field.holder).get(field);
   }
 
-  private Map<DexField, DexEncodedField> getFieldDefinitions(DexType type) {
-    return fieldDefinitionsCache.computeIfAbsent(type, this::computeFieldDefinitions);
+  private Map<DexMember<?, ?>, DexEncodedMember<?, ?>> getDefinitions(DexType type) {
+    Map<DexMember<?, ?>, DexEncodedMember<?, ?>> typeDefinitions = definitions.get(type);
+    if (typeDefinitions != null) {
+      return typeDefinitions;
+    }
+
+    typeDefinitions = computeDefinitions(type);
+    Map<DexMember<?, ?>, DexEncodedMember<?, ?>> existing =
+        definitions.putIfAbsent(type, typeDefinitions);
+    return existing != null ? existing : typeDefinitions;
   }
 
-  public void invalidateFieldCacheFor(DexType type) {
-    fieldDefinitionsCache.remove(type);
+  public void invalidateTypeCacheFor(DexType type) {
+    definitions.remove(type);
   }
 
   // TODO(b/147578480): Temporary API since most of the code base use a type instead

src/main/java/com/android/tools/r8/shaking/Enqueuer.java

diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index c43b1f9..d4545b3 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2527,6 +2527,7 @@
     for (ProgramMethod bridge : syntheticInterfaceMethodBridges.values()) {
       DexProgramClass holder = bridge.getHolder();
       DexEncodedMethod method = bridge.getMethod();
+      appView.appInfo().invalidateTypeCacheFor(holder.type);
       holder.addVirtualMethod(method);
       targetedMethods.add(method, graphReporter.fakeReportShouldNotBeUsed());
       liveMethods.add(holder, method, graphReporter.fakeReportShouldNotBeUsed());