commit 4ce9ffe26a5014143727b55485580898ffe2aafb
author Ian Zerny <zerny@google.com> Thu Mar 19 10:52:23 2020 +0000
committer Ian Zerny <zerny@google.com> Thu Mar 19 10:52:37 2020 +0000
tree 51a2631c090e962c339b7fc0f6335d3e959ba87e
parent 6f3141761a84870dbcbdd09d9966a639e1e2cb24

Reland "Remove methods from definitionsFor cache."

This reverts commit f58b55990d3fca2b3a4acf67ecba721cdc9c0262.

Bug: 151878901
Change-Id: I3e94ffdd1c3a7ddb9826adcc2af405dff57e8001

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 2ddba3f..a41e119 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -31,11 +31,15 @@
 
   private final DexApplication app;
   private final DexItemFactory dexItemFactory;
-  private final ConcurrentHashMap<DexType, Map<DexMember<?, ?>, DexEncodedMember<?, ?>>>
-      definitions = new ConcurrentHashMap<>();
+
+  // TODO(b/151804585): Remove this cache.
+  private final ConcurrentHashMap<DexType, Map<DexField, DexEncodedField>> fieldDefinitionsCache =
+      new ConcurrentHashMap<>();
+
   // For some optimizations, e.g. optimizing synthetic classes, we may need to resolve the current
   // class being optimized.
-  final ConcurrentHashMap<DexType, DexProgramClass> synthesizedClasses = new ConcurrentHashMap<>();
+  private 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.
@@ -50,7 +54,7 @@
     assert !previous.isObsolete();
     this.app = previous.app;
     this.dexItemFactory = app.dexItemFactory;
-    this.definitions.putAll(previous.definitions);
+    this.fieldDefinitionsCache.putAll(previous.fieldDefinitionsCache);
     copyMetadataFromPrevious(previous);
   }
 
@@ -94,7 +98,7 @@
     assert checkIfObsolete();
     assert clazz.type.isD8R8SynthesizedClassType();
     DexProgramClass previous = synthesizedClasses.put(clazz.type, clazz);
-    invalidateTypeCacheFor(clazz.type);
+    invalidateFieldCacheFor(clazz.type);
     assert previous == null || previous == clazz;
   }
 
@@ -103,12 +107,11 @@
     return Collections.unmodifiableCollection(synthesizedClasses.values());
   }
 
-  private Map<DexMember<?, ?>, DexEncodedMember<?, ?>> computeDefinitions(DexType type) {
-    Builder<DexMember<?, ?>, DexEncodedMember<?, ?>> builder = ImmutableMap.builder();
+  private Map<DexField, DexEncodedField> computeFieldDefinitions(DexType type) {
+    Builder<DexField, DexEncodedField> builder = ImmutableMap.builder();
     DexClass clazz = definitionFor(type);
     if (clazz != null) {
-      clazz.forEachMethod(method -> builder.put(method.toReference(), method));
-      clazz.forEachField(field -> builder.put(field.toReference(), field));
+      clazz.forEachField(field -> builder.put(field.field, field));
     }
     return builder.build();
   }
@@ -173,35 +176,29 @@
   @Override
   public DexEncodedMethod definitionFor(DexMethod method) {
     assert checkIfObsolete();
-    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);
+    assert method.holder.isClassType();
+    if (!method.holder.isClassType()) {
+      return null;
     }
-    return cached;
+    DexClass clazz = definitionFor(method.holder);
+    if (clazz == null) {
+      return null;
+    }
+    return clazz.getMethodCollection().getMethod(method);
   }
 
   @Override
   public DexEncodedField definitionFor(DexField field) {
     assert checkIfObsolete();
-    return (DexEncodedField) getDefinitions(field.holder).get(field);
+    return getFieldDefinitions(field.holder).get(field);
   }
 
-  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;
+  private Map<DexField, DexEncodedField> getFieldDefinitions(DexType type) {
+    return fieldDefinitionsCache.computeIfAbsent(type, this::computeFieldDefinitions);
   }
 
-  public void invalidateTypeCacheFor(DexType type) {
-    definitions.remove(type);
+  public void invalidateFieldCacheFor(DexType type) {
+    fieldDefinitionsCache.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 d4545b3..c43b1f9 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2527,7 +2527,6 @@
     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());