Desugared library: Support all callbacks from library
- add minimalCallbacks option in DesugaredLibraryConfiguration
Bug: 163880202
Change-Id: I0cd64cf8c2b966d109cf4d8e14b8e364b118092c
diff --git a/src/library_desugar/desugar_jdk_libs.json b/src/library_desugar/desugar_jdk_libs.json
index 945ad60..98ceb80 100644
--- a/src/library_desugar/desugar_jdk_libs.json
+++ b/src/library_desugar/desugar_jdk_libs.json
@@ -2,9 +2,10 @@
"configuration_format_version": 3,
"group_id" : "com.tools.android",
"artifact_id" : "desugar_jdk_libs",
- "version": "1.0.12",
+ "version": "1.1.0",
"required_compilation_api_level": 26,
"synthesized_library_classes_package_prefix": "j$.",
+ "support_all_callbacks_from_library": true,
"common_flags": [
{
"api_level_below_or_equal": 25,
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java
index c337afe..9356caa 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java
@@ -186,6 +186,10 @@
.containsKey(method.getHolderType())) {
return false;
}
+ if (!appView.options().desugaredLibraryConfiguration.supportAllCallbacksFromLibrary
+ && appView.options().isDesugaredLibraryCompilation()) {
+ return false;
+ }
return overridesLibraryMethod(method);
}
@@ -212,6 +216,9 @@
if (!dexClass.isLibraryClass() && !appView.options().isDesugaredLibraryCompilation()) {
continue;
}
+ if (!shouldGenerateCallbacksForEmulateInterfaceAPIs(dexClass)) {
+ continue;
+ }
DexEncodedMethod dexEncodedMethod = dexClass.lookupVirtualMethod(method.getReference());
if (dexEncodedMethod != null) {
// In this case, the object will be wrapped.
@@ -224,6 +231,16 @@
return foundOverrideToRewrite;
}
+ private boolean shouldGenerateCallbacksForEmulateInterfaceAPIs(DexClass dexClass) {
+ if (appView.options().desugaredLibraryConfiguration.supportAllCallbacksFromLibrary) {
+ return true;
+ }
+ Map<DexType, DexType> emulateLibraryInterfaces =
+ appView.options().desugaredLibraryConfiguration.getEmulateLibraryInterface();
+ return !(emulateLibraryInterfaces.containsKey(dexClass.type)
+ || emulateLibraryInterfaces.containsValue(dexClass.type));
+ }
+
private synchronized void registerCallback(ProgramMethod method) {
// In R8 we should be in the enqueuer, therefore we can duplicate a default method and both
// methods will be desugared.
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
index 542c5d7..9896987 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
@@ -36,12 +36,15 @@
public class DesugaredLibraryConfiguration {
public static final String FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX = "j$/";
+ public static final boolean FALL_BACK_SUPPORT_ALL_CALLBACKS_FROM_LIBRARY = true;
+
public static final DesugaredLibraryConfiguration EMPTY_DESUGARED_LIBRARY_CONFIGURATION =
new DesugaredLibraryConfiguration(
AndroidApiLevel.B,
false,
FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX,
null,
+ FALL_BACK_SUPPORT_ALL_CALLBACKS_FROM_LIBRARY,
ImmutableMap.of(),
ImmutableMap.of(),
ImmutableMap.of(),
@@ -51,11 +54,18 @@
ImmutableList.of(),
ImmutableList.of());
- // TODO(b/158632510): should use DexString, DexType, DexMethod or so on when possible.
private final AndroidApiLevel requiredCompilationAPILevel;
private final boolean libraryCompilation;
private final String synthesizedLibraryClassesPackagePrefix;
private final String identifier;
+ // Setting supportAllCallbacksFromLibrary reduces the number of generated call-backs,
+ // more specifically:
+ // - no call-back is generated for emulated interface method overrides (forEach, etc.)
+ // - no call-back is generated inside the desugared library itself.
+ // Such setting decreases significantly the desugared library dex file, but virtual calls from
+ // within the library to desugared library classes instances as receiver may be incorrect, for
+ // example the method forEach in Iterable may be executed over a concrete implementation.
+ public final boolean supportAllCallbacksFromLibrary;
private final Map<String, String> rewritePrefix;
private final Map<DexType, DexType> emulateLibraryInterface;
private final Map<DexString, Map<DexType, DexType>> retargetCoreLibMember;
@@ -76,6 +86,7 @@
true,
FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX,
"testingOnlyVersion",
+ FALL_BACK_SUPPORT_ALL_CALLBACKS_FROM_LIBRARY,
prefix,
ImmutableMap.of(),
ImmutableMap.of(),
@@ -95,6 +106,7 @@
boolean libraryCompilation,
String packagePrefix,
String identifier,
+ boolean supportAllCallbacksFromLibrary,
Map<String, String> rewritePrefix,
Map<DexType, DexType> emulateLibraryInterface,
Map<DexString, Map<DexType, DexType>> retargetCoreLibMember,
@@ -107,6 +119,7 @@
this.libraryCompilation = libraryCompilation;
this.synthesizedLibraryClassesPackagePrefix = packagePrefix;
this.identifier = identifier;
+ this.supportAllCallbacksFromLibrary = supportAllCallbacksFromLibrary;
this.rewritePrefix = rewritePrefix;
this.emulateLibraryInterface = emulateLibraryInterface;
this.retargetCoreLibMember = retargetCoreLibMember;
@@ -204,6 +217,7 @@
private Set<DexType> wrapperConversions = Sets.newIdentityHashSet();
private List<Pair<DexType, DexString>> dontRewriteInvocation = new ArrayList<>();
private List<String> extraKeepRules = Collections.emptyList();
+ private boolean supportAllCallbacksFromLibrary = FALL_BACK_SUPPORT_ALL_CALLBACKS_FROM_LIBRARY;
private Builder(DexItemFactory dexItemFactory, Reporter reporter, Origin origin) {
this.factory = dexItemFactory;
@@ -344,6 +358,10 @@
return factory.createType(DescriptorUtils.javaTypeToDescriptor(stringClass));
}
+ public void setSupportAllCallbacksFromLibrary(boolean supportAllCallbacksFromLibrary) {
+ this.supportAllCallbacksFromLibrary = supportAllCallbacksFromLibrary;
+ }
+
public DesugaredLibraryConfiguration build() {
validate();
return new DesugaredLibraryConfiguration(
@@ -351,6 +369,7 @@
libraryCompilation,
synthesizedLibraryClassesPackagePrefix,
identifier,
+ supportAllCallbacksFromLibrary,
ImmutableMap.copyOf(rewritePrefix),
ImmutableMap.copyOf(emulateLibraryInterface),
ImmutableMap.copyOf(retargetCoreLibMember),
@@ -373,5 +392,6 @@
origin));
}
}
+
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java
index 7586210..8b81bf4 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java
@@ -47,6 +47,7 @@
static final String DONT_REWRITE_KEY = "dont_rewrite";
static final String BACKPORT_KEY = "backport";
static final String SHRINKER_CONFIG_KEY = "shrinker_config";
+ static final String SUPPORT_ALL_CALLBACKS_FROM_LIBRARY_KEY = "support_all_callbacks_from_library";
private final DexItemFactory dexItemFactory;
private final Reporter reporter;
@@ -107,6 +108,9 @@
throw reporter.fatalError(
new StringDiagnostic(
"Unsupported desugared library configuration version, please upgrade the D8/R8"
+ + formatVersion
+ + " - "
+ + MAX_SUPPORTED_VERSION
+ " compiler.",
origin));
}
@@ -149,6 +153,12 @@
}
configurationBuilder.setExtraKeepRules(extraKeepRules);
}
+
+ if (jsonConfig.has(SUPPORT_ALL_CALLBACKS_FROM_LIBRARY_KEY)) {
+ boolean supportAllCallbacksFromLibrary =
+ jsonConfig.get(SUPPORT_ALL_CALLBACKS_FROM_LIBRARY_KEY).getAsBoolean();
+ configurationBuilder.setSupportAllCallbacksFromLibrary(supportAllCallbacksFromLibrary);
+ }
configurationAmender.accept(configurationBuilder);
DesugaredLibraryConfiguration config = configurationBuilder.build();
configurationBuilder = null;
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java
index b02b2e9..c7465ca 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java
@@ -21,6 +21,8 @@
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.ir.desugar.DesugaredLibraryConfiguration;
+import com.android.tools.r8.ir.desugar.DesugaredLibraryConfigurationParser;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.InternalOptions;
@@ -67,6 +69,11 @@
return buildDesugaredLibrary(apiLevel, "", false);
}
+ protected Path buildDesugaredLibrary(
+ AndroidApiLevel apiLevel, Consumer<InternalOptions> optionsModifier) {
+ return buildDesugaredLibrary(apiLevel, "", false, ImmutableList.of(), optionsModifier);
+ }
+
protected Path buildDesugaredLibrary(AndroidApiLevel apiLevel, String keepRules) {
return buildDesugaredLibrary(apiLevel, keepRules, true);
}
@@ -186,6 +193,21 @@
return desugaredLib;
}
+ protected DesugaredLibraryConfiguration configurationWithSupportAllCallbacksFromLibrary(
+ InternalOptions options,
+ boolean libraryCompilation,
+ TestParameters parameters,
+ boolean supportAllCallbacksFromLibrary) {
+ return new DesugaredLibraryConfigurationParser(
+ options.dexItemFactory(),
+ options.reporter,
+ libraryCompilation,
+ parameters.getApiLevel().getLevel())
+ .parse(
+ StringResource.fromFile(ToolHelper.DESUGAR_LIB_JSON_FOR_TESTING),
+ builder -> builder.setSupportAllCallbacksFromLibrary(supportAllCallbacksFromLibrary));
+ }
+
public interface KeepRuleConsumer extends StringConsumer {
String get();
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ConversionIntroduceInterfaceMethodTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ConversionIntroduceInterfaceMethodTest.java
index 14f3ded..e90d8d2 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ConversionIntroduceInterfaceMethodTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ConversionIntroduceInterfaceMethodTest.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
import java.nio.file.Path;
import java.util.Collection;
+import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.function.Consumer;
@@ -34,6 +35,7 @@
private final TestParameters parameters;
private final boolean shrinkDesugaredLibrary;
+ private final boolean supportAllCallbacksFromLibrary;
private static final AndroidApiLevel MIN_SUPPORTED = AndroidApiLevel.N;
private static final String EXPECTED_RESULT =
@@ -42,16 +44,24 @@
"forEach called",
"action called from java consumer",
"forEach called");
+ private static final String FAILING_EXPECTED_RESULT =
+ StringUtils.lines(
+ "action called from j$ consumer", "forEach called", "action called from java consumer");
private static Path CUSTOM_LIB;
- @Parameters(name = "{0}, shrinkDesugaredLibrary: {1}")
+ @Parameters(name = "{0}, shrink: {1}, supportCallbacks: {2}")
public static List<Object[]> data() {
return buildParameters(
- getConversionParametersUpToExcluding(MIN_SUPPORTED), BooleanUtils.values());
+ getConversionParametersUpToExcluding(MIN_SUPPORTED),
+ BooleanUtils.values(),
+ BooleanUtils.values());
}
public ConversionIntroduceInterfaceMethodTest(
- TestParameters parameters, boolean shrinkDesugaredLibrary) {
+ TestParameters parameters,
+ boolean shrinkDesugaredLibrary,
+ boolean supportAllCallbacksFromLibrary) {
+ this.supportAllCallbacksFromLibrary = supportAllCallbacksFromLibrary;
this.shrinkDesugaredLibrary = shrinkDesugaredLibrary;
this.parameters = parameters;
}
@@ -75,6 +85,11 @@
.addLibraryClasses(CustomLibClass.class)
.addOptionsModification(opt -> opt.testing.trackDesugaredAPIConversions = true)
.enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
+ .addOptionsModification(
+ opt ->
+ opt.desugaredLibraryConfiguration =
+ configurationWithSupportAllCallbacksFromLibrary(
+ opt, false, parameters, supportAllCallbacksFromLibrary))
.compile()
.inspect(this::assertDoubleForEach)
.inspect(this::assertWrapperMethodsPresent)
@@ -85,11 +100,13 @@
shrinkDesugaredLibrary)
.addRunClasspathFiles(CUSTOM_LIB)
.run(parameters.getRuntime(), Executor.class)
- .assertSuccessWithOutput(EXPECTED_RESULT);
+ .apply(
+ r ->
+ r.assertSuccessWithOutput(
+ supportAllCallbacksFromLibrary ? EXPECTED_RESULT : FAILING_EXPECTED_RESULT));
}
private void assertDoubleForEach(CodeInspector inspector) {
- System.out.println(inspector.allClasses().size());
FoundClassSubject myCollection =
inspector.allClasses().stream()
.filter(
@@ -103,7 +120,7 @@
.collect(toSingle());
assertEquals(
"Missing duplicated forEach",
- 2,
+ supportAllCallbacksFromLibrary ? 2 : 1,
IterableUtils.size(
myCollection
.getDexProgramClass()
@@ -133,6 +150,11 @@
.addKeepMainRule(Executor.class)
.addLibraryClasses(CustomLibClass.class)
.enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
+ .addOptionsModification(
+ opt ->
+ opt.desugaredLibraryConfiguration =
+ configurationWithSupportAllCallbacksFromLibrary(
+ opt, false, parameters, supportAllCallbacksFromLibrary))
.compile()
.inspect(this::assertDoubleForEach)
.inspect(this::assertWrapperMethodsPresent)
@@ -143,7 +165,10 @@
shrinkDesugaredLibrary)
.addRunClasspathFiles(CUSTOM_LIB)
.run(parameters.getRuntime(), Executor.class)
- .assertSuccessWithOutput(EXPECTED_RESULT);
+ .apply(
+ r ->
+ r.assertSuccessWithOutput(
+ supportAllCallbacksFromLibrary ? EXPECTED_RESULT : FAILING_EXPECTED_RESULT));
}
static class CustomLibClass {
@@ -197,7 +222,7 @@
@Override
public Iterator<E> iterator() {
- return null;
+ return (Iterator<E>) Collections.singletonList(null).iterator();
}
@NotNull
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/DuplicateAPIDesugaredLibTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/DuplicateAPIDesugaredLibTest.java
index 1f433d4..812cd34 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/DuplicateAPIDesugaredLibTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/DuplicateAPIDesugaredLibTest.java
@@ -6,55 +6,84 @@
import static junit.framework.TestCase.assertEquals;
+import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestRuntime.DexRuntime;
import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.Box;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import java.nio.file.Path;
+import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiConsumer;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+@RunWith(Parameterized.class)
public class DuplicateAPIDesugaredLibTest extends DesugaredLibraryTestBase {
+ private final TestParameters parameters;
+ private final boolean shrinkDesugaredLibrary;
+ private static final AndroidApiLevel MIN_SUPPORTED = AndroidApiLevel.N;
+
+ @Parameterized.Parameters(name = "{0}, shrinkDesugaredLibrary: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getConversionParametersUpToExcluding(MIN_SUPPORTED), BooleanUtils.values());
+ }
+
+ public DuplicateAPIDesugaredLibTest(TestParameters parameters, boolean shrinkDesugaredLibrary) {
+ this.shrinkDesugaredLibrary = shrinkDesugaredLibrary;
+ this.parameters = parameters;
+ }
+
@Test
public void testLib() throws Exception {
- Box<Path> desugaredLibBox = new Box<>();
- Path customLib =
- testForD8()
- .addProgramClasses(CustomLibClass.class)
- .setMinApi(AndroidApiLevel.B)
- .compile()
- .writeToZip();
- String stdOut =
- testForD8()
- .setMinApi(AndroidApiLevel.B)
- .addProgramClasses(Executor.class)
- .addLibraryClasses(CustomLibClass.class)
- .enableCoreLibraryDesugaring(AndroidApiLevel.B)
- .compile()
- .addDesugaredCoreLibraryRunClassPath(
- (AndroidApiLevel api) -> {
- desugaredLibBox.set(this.buildDesugaredLibrary(api));
- return desugaredLibBox.get();
- },
- AndroidApiLevel.B)
- .addRunClasspathFiles(customLib)
- .run(new DexRuntime(DexVm.ART_9_0_0_HOST), Executor.class)
- .assertSuccess()
- .getStdOut();
- assertDupMethod(new CodeInspector(desugaredLibBox.get()));
- assertLines2By2Correct(stdOut);
+ for (Boolean supportAllCallbacksFromLibrary : BooleanUtils.values()) {
+ Box<Path> desugaredLibBox = new Box<>();
+ Path customLib =
+ testForD8()
+ .addProgramClasses(CustomLibClass.class)
+ .setMinApi(AndroidApiLevel.B)
+ .compile()
+ .writeToZip();
+ String stdOut =
+ testForD8()
+ .setMinApi(AndroidApiLevel.B)
+ .addProgramClasses(Executor.class)
+ .addLibraryClasses(CustomLibClass.class)
+ .enableCoreLibraryDesugaring(AndroidApiLevel.B)
+ .compile()
+ .addDesugaredCoreLibraryRunClassPath(
+ (AndroidApiLevel api) -> {
+ desugaredLibBox.set(
+ this.buildDesugaredLibrary(
+ api,
+ opt ->
+ opt.desugaredLibraryConfiguration =
+ configurationWithSupportAllCallbacksFromLibrary(
+ opt, true, parameters, supportAllCallbacksFromLibrary)));
+ return desugaredLibBox.get();
+ },
+ AndroidApiLevel.B)
+ .addRunClasspathFiles(customLib)
+ .run(new DexRuntime(DexVm.ART_9_0_0_HOST), Executor.class)
+ .assertSuccess()
+ .getStdOut();
+ assertDupMethod(new CodeInspector(desugaredLibBox.get()), supportAllCallbacksFromLibrary);
+ assertLines2By2Correct(stdOut);
+ }
}
- private void assertDupMethod(CodeInspector inspector) {
+ private void assertDupMethod(CodeInspector inspector, boolean supportAllCallbacksFromLibrary) {
ClassSubject clazz = inspector.clazz("j$.util.concurrent.ConcurrentHashMap");
assertEquals(
- 2,
+ supportAllCallbacksFromLibrary ? 2 : 1,
clazz.virtualMethods().stream().filter(m -> m.getOriginalName().equals("forEach")).count());
}
