Remove thread local code mapping.
Fixes: 181636450
Change-Id: I1b4b7f036756722243e9e23a32cb6164abb5b481
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java b/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java
index 8209749..a45e7fb 100644
--- a/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java
+++ b/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java
@@ -63,13 +63,10 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
-import java.util.IdentityHashMap;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
import java.util.function.Predicate;
import java.util.stream.Collectors;
@@ -269,7 +266,6 @@
}
// Generate the dex file contents.
- List<Future<Boolean>> dexDataFutures = new ArrayList<>();
timing.begin("Distribute");
List<VirtualFile> virtualFiles = distribute(executorService);
timing.end();
@@ -380,11 +376,10 @@
virtualFile.computeMapping(appView, graphLens, namingLens, initClassLens, timing);
timing.end();
timing.begin("Rewrite jumbo strings");
- MethodToCodeObjectMapping codeMapping =
- rewriteCodeWithJumboStrings(objectMapping, virtualFile.classes(), appView.appInfo().app());
+ rewriteCodeWithJumboStrings(objectMapping, virtualFile.classes(), appView.appInfo().app());
timing.end();
timing.begin("Write bytes");
- ByteBufferResult result = writeDexFile(objectMapping, codeMapping, byteBufferProvider);
+ ByteBufferResult result = writeDexFile(objectMapping, byteBufferProvider);
ByteDataView data =
new ByteDataView(result.buffer.array(), result.buffer.arrayOffset(), result.length);
timing.end();
@@ -652,7 +647,7 @@
* <p>If run multiple times on a class, the lowest index that is required to be a JumboString will
* be used.
*/
- private MethodToCodeObjectMapping rewriteCodeWithJumboStrings(
+ private void rewriteCodeWithJumboStrings(
ObjectToOffsetMapping mapping,
Collection<DexProgramClass> classes,
DexApplication application) {
@@ -660,19 +655,14 @@
if (!options.testing.forceJumboStringProcessing) {
// If there are no strings with jumbo indices at all this is a no-op.
if (!mapping.hasJumboStrings()) {
- return MethodToCodeObjectMapping.fromMethodBacking();
+ return;
}
// If the globally highest sorting string is not a jumbo string this is also a no-op.
if (application.highestSortingString != null
&& application.highestSortingString.compareTo(mapping.getFirstJumboString()) < 0) {
- return MethodToCodeObjectMapping.fromMethodBacking();
+ return;
}
}
- // At least one method needs a jumbo string in which case we construct a thread local mapping
- // for all code objects and write the processed results into that map.
- // TODO(b/181636450): Reconsider the code mapping setup now that synthetics are never duplicated
- // in outputs.
- Map<DexEncodedMethod, DexWritableCode> codeMapping = new IdentityHashMap<>();
for (DexProgramClass clazz : classes) {
clazz.forEachProgramMethodMatching(
DexEncodedMethod::hasCode,
@@ -684,25 +674,18 @@
mapping,
application.dexItemFactory,
options.testing.forceJumboStringProcessing);
- codeMapping.put(method.getDefinition(), rewrittenCode);
- // The mapping now has ownership of the methods code object. This ensures freeing of
- // code resources once the map entry is cleared and also ensures that we don't end up
- // using the incorrect code pointer again later!
- method.getDefinition().unsetCode();
+ method.getDefinition().setCode(rewrittenCode.asCode(), appView);
});
}
- return MethodToCodeObjectMapping.fromMapBacking(codeMapping);
}
private ByteBufferResult writeDexFile(
ObjectToOffsetMapping objectMapping,
- MethodToCodeObjectMapping codeMapping,
ByteBufferProvider provider) {
FileWriter fileWriter =
new FileWriter(
provider,
objectMapping,
- codeMapping,
appView.appInfo(),
options,
namingLens,
diff --git a/src/main/java/com/android/tools/r8/dex/FileWriter.java b/src/main/java/com/android/tools/r8/dex/FileWriter.java
index c937839..84b245c 100644
--- a/src/main/java/com/android/tools/r8/dex/FileWriter.java
+++ b/src/main/java/com/android/tools/r8/dex/FileWriter.java
@@ -98,8 +98,6 @@
}
private final ObjectToOffsetMapping mapping;
- private final MethodToCodeObjectMapping codeMapping;
- private final AppInfo appInfo;
private final DexApplication application;
private final InternalOptions options;
private final GraphLens graphLens;
@@ -112,20 +110,17 @@
public FileWriter(
ByteBufferProvider provider,
ObjectToOffsetMapping mapping,
- MethodToCodeObjectMapping codeMapping,
AppInfo appInfo,
InternalOptions options,
NamingLens namingLens,
CodeToKeep desugaredLibraryCodeToKeep) {
this.mapping = mapping;
- this.codeMapping = codeMapping;
- this.appInfo = appInfo;
this.application = appInfo.app();
this.options = options;
this.graphLens = mapping.getGraphLens();
this.namingLens = namingLens;
this.dest = new DexOutputBuffer(provider);
- this.mixedSectionOffsets = new MixedSectionOffsets(options, codeMapping);
+ this.mixedSectionOffsets = new MixedSectionOffsets(options);
this.desugaredLibraryCodeToKeep = desugaredLibraryCodeToKeep;
}
@@ -179,9 +174,6 @@
Layout layout = Layout.from(mapping);
layout.setCodesOffset(layout.dataSectionOffset);
- // Check code objects in the code-mapping are consistent with the collected code objects.
- assert codeMapping.verifyCodeObjects(mixedSectionOffsets.getCodes());
-
// Sort the codes first, as their order might impact size due to alignment constraints.
List<ProgramDexCode> codes = sortDexCodesByClassName();
@@ -340,7 +332,7 @@
for (DexProgramClass clazz : mapping.getClasses()) {
clazz.forEachProgramMethod(
method -> {
- DexWritableCode code = codeMapping.getCode(method.getDefinition());
+ DexWritableCode code = method.getDefinition().getDexWritableCodeOrNull();
assert code != null || method.getDefinition().shouldNotHaveCode();
if (code != null) {
ProgramDexCode programCode = new ProgramDexCode(code, method);
@@ -661,7 +653,7 @@
dest.putUleb128(nextOffset - currentOffset);
currentOffset = nextOffset;
dest.putUleb128(method.accessFlags.getAsDexAccessFlags());
- DexWritableCode code = codeMapping.getCode(method);
+ DexWritableCode code = method.getDexWritableCodeOrNull();
desugaredLibraryCodeToKeep.recordMethod(method.getReference());
if (code == null) {
assert method.shouldNotHaveCode();
@@ -670,7 +662,7 @@
dest.putUleb128(mixedSectionOffsets.getOffsetFor(method, code));
// Writing the methods starts to take up memory so we are going to flush the
// code objects since they are no longer necessary after this.
- codeMapping.clearCode(method);
+ method.unsetCode();
}
}
}
@@ -1077,8 +1069,6 @@
private static final int NOT_SET = -1;
private static final int NOT_KNOWN = -2;
- private final MethodToCodeObjectMapping codeMapping;
-
private final Reference2IntMap<DexEncodedMethod> codes = createReference2IntMap();
private final Object2IntMap<DexDebugInfo> debugInfos = createObject2IntMap();
private final Object2IntMap<DexTypeList> typeLists = createObject2IntMap();
@@ -1108,9 +1098,8 @@
return result;
}
- private MixedSectionOffsets(InternalOptions options, MethodToCodeObjectMapping codeMapping) {
+ private MixedSectionOffsets(InternalOptions options) {
this.minApiLevel = options.getMinApiLevel();
- this.codeMapping = codeMapping;
}
private <T> boolean add(Object2IntMap<T> map, T item) {
@@ -1151,7 +1140,7 @@
@Override
public void visit(DexEncodedMethod method) {
- method.collectMixedSectionItemsWithCodeMapping(this, codeMapping);
+ method.collectMixedSectionItemsWithCodeMapping(this);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/dex/MethodToCodeObjectMapping.java b/src/main/java/com/android/tools/r8/dex/MethodToCodeObjectMapping.java
deleted file mode 100644
index dfdb07a..0000000
--- a/src/main/java/com/android/tools/r8/dex/MethodToCodeObjectMapping.java
+++ /dev/null
@@ -1,81 +0,0 @@
-// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-package com.android.tools.r8.dex;
-
-import com.android.tools.r8.graph.Code;
-import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexWritableCode;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
-
-public abstract class MethodToCodeObjectMapping {
-
- public abstract DexWritableCode getCode(DexEncodedMethod method);
-
- public abstract void clearCode(DexEncodedMethod method);
-
- public abstract boolean verifyCodeObjects(Collection<DexEncodedMethod> codes);
-
- public static MethodToCodeObjectMapping fromMethodBacking() {
- return MethodBacking.INSTANCE;
- }
-
- public static MethodToCodeObjectMapping fromMapBacking(
- Map<DexEncodedMethod, DexWritableCode> map) {
- return new MapBacking(map);
- }
-
- private static class MethodBacking extends MethodToCodeObjectMapping {
-
- private static final MethodBacking INSTANCE = new MethodBacking();
-
- @Override
- public DexWritableCode getCode(DexEncodedMethod method) {
- Code code = method.getCode();
- assert code == null || code.isDexWritableCode();
- return code == null ? null : code.asDexWritableCode();
- }
-
- @Override
- public void clearCode(DexEncodedMethod method) {
- method.unsetCode();
- }
-
- @Override
- public boolean verifyCodeObjects(Collection<DexEncodedMethod> codes) {
- return true;
- }
- }
-
- private static class MapBacking extends MethodToCodeObjectMapping {
-
- private final Map<DexEncodedMethod, DexWritableCode> codes;
-
- public MapBacking(Map<DexEncodedMethod, DexWritableCode> codes) {
- this.codes = codes;
- }
-
- @Override
- public DexWritableCode getCode(DexEncodedMethod method) {
- return codes.get(method);
- }
-
- @Override
- public void clearCode(DexEncodedMethod method) {
- // We can safely clear the thread local pointer to even shared methods.
- codes.put(method, null);
- }
-
- @Override
- public boolean verifyCodeObjects(Collection<DexEncodedMethod> methods) {
- // TODO(b/204056443): Convert to a Set<DexWritableCode> when DexCode#hashCode() works.
- List<DexWritableCode> codes =
- methods.stream().map(this::getCode).collect(Collectors.toList());
- assert this.codes.values().containsAll(codes);
- return true;
- }
- }
-}
diff --git a/src/main/java/com/android/tools/r8/dex/MixedSectionCollection.java b/src/main/java/com/android/tools/r8/dex/MixedSectionCollection.java
index 0ca695e..e082a31 100644
--- a/src/main/java/com/android/tools/r8/dex/MixedSectionCollection.java
+++ b/src/main/java/com/android/tools/r8/dex/MixedSectionCollection.java
@@ -59,8 +59,7 @@
* <p>Allows overriding the behavior when dex-file writing.
*/
public void visit(DexEncodedMethod dexEncodedMethod) {
- dexEncodedMethod.collectMixedSectionItemsWithCodeMapping(
- this, MethodToCodeObjectMapping.fromMethodBacking());
+ dexEncodedMethod.collectMixedSectionItemsWithCodeMapping(this);
}
/**
diff --git a/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java b/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
index f336d89..e25cb47 100644
--- a/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
@@ -123,6 +123,11 @@
}
@Override
+ public Code asCode() {
+ return this;
+ }
+
+ @Override
public void acceptHashing(HashingVisitor visitor) {
visitor.visitInt(getCfWritableCodeKind().hashCode());
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexCode.java b/src/main/java/com/android/tools/r8/graph/DexCode.java
index d49c43f..703977e 100644
--- a/src/main/java/com/android/tools/r8/graph/DexCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DexCode.java
@@ -351,6 +351,11 @@
}
@Override
+ public Code asCode() {
+ return this;
+ }
+
+ @Override
public IRCode buildIR(ProgramMethod method, AppView<?> appView, Origin origin) {
DexSourceCode source =
new DexSourceCode(
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index a68f038..e89b2ac 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -38,7 +38,6 @@
import com.android.tools.r8.code.Return;
import com.android.tools.r8.code.Throw;
import com.android.tools.r8.code.XorIntLit8;
-import com.android.tools.r8.dex.MethodToCodeObjectMapping;
import com.android.tools.r8.dex.MixedSectionCollection;
import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.Unreachable;
@@ -768,9 +767,8 @@
mixedItems.visit(this);
}
- public void collectMixedSectionItemsWithCodeMapping(
- MixedSectionCollection mixedItems, MethodToCodeObjectMapping mapping) {
- DexWritableCode code = mapping.getCode(this);
+ public void collectMixedSectionItemsWithCodeMapping(MixedSectionCollection mixedItems) {
+ DexWritableCode code = getDexWritableCodeOrNull();
if (code != null && mixedItems.add(this, code)) {
code.collectMixedSectionItems(mixedItems);
}
@@ -1315,6 +1313,12 @@
this.genericSignature = MethodTypeSignature.noSignature();
}
+ public DexWritableCode getDexWritableCodeOrNull() {
+ Code code = getCode();
+ assert code == null || code.isDexWritableCode();
+ return code == null ? null : code.asDexWritableCode();
+ }
+
public static Builder syntheticBuilder() {
return new Builder(true);
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexWritableCode.java b/src/main/java/com/android/tools/r8/graph/DexWritableCode.java
index 664b4e8..fffff66 100644
--- a/src/main/java/com/android/tools/r8/graph/DexWritableCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DexWritableCode.java
@@ -71,6 +71,8 @@
int getOutgoingRegisterSize();
+ Code asCode();
+
default boolean isDexCode() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java b/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java
index eb7728c..c573130 100644
--- a/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java
+++ b/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java
@@ -41,6 +41,11 @@
}
@Override
+ public Code asCode() {
+ return this;
+ }
+
+ @Override
public void acceptHashing(HashingVisitor visitor) {
visitor.visitInt(getCfWritableCodeKind().hashCode());
}
