From 8713a8e2a1916424bff771131c4b8df9ab6a0ee1 Mon Sep 17 00:00:00 2001 From: mohamadk Date: Mon, 28 Nov 2022 15:16:05 +0800 Subject: [PATCH] make desugar dependencies deterministic, desugar dependencies are added to the result jar file as metadata, this desugar dependency object contain few list that we found out the order of this list can be different in different build with same inputs, therefore final result will have a different hash and it cause cache miss in the builds solution is to make the lists in this object sorted. so for the same input we always get same output --- .../dependencies/MetadataCollectorTest.java | 86 +++++++++++-------- .../dependencies/MetadataCollector.java | 63 +++++++++++--- 2 files changed, 104 insertions(+), 45 deletions(-) diff --git a/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java b/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java index 64dd8715116938..b394c7c614ad5d 100644 --- a/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java +++ b/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java @@ -25,6 +25,10 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.util.Arrays; +import java.util.function.Function; +import java.util.stream.Collectors; + /** Unit tests for {@link MetadataCollector}. */ @RunWith(JUnit4.class) public class MetadataCollectorTest { @@ -42,65 +46,79 @@ public void testAssumeCompanionClass() throws Exception { collector.assumeCompanionClass("a", "a$$CC"); DesugarDepsInfo info = extractProto(collector); - assertThat(info.getAssumePresentList()) - .containsExactly( - Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("b$$CC")).build(), - Dependency.newBuilder().setOrigin(wrapType("b")).setTarget(wrapType("b$$CC")).build(), - Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("a$$CC")).build()); + + assertThat(info.getAssumePresentList().get(0)).isEqualTo(dependency("a", "a$$CC")); + assertThat(info.getAssumePresentList().get(1)).isEqualTo(dependency("a", "b$$CC")); + assertThat(info.getAssumePresentList().get(2)).isEqualTo(dependency("b", "b$$CC")); } @Test public void testMissingImplementedInterface() throws Exception { MetadataCollector collector = new MetadataCollector(true); collector.missingImplementedInterface("a", "b"); - collector.missingImplementedInterface("a", "c"); collector.missingImplementedInterface("c", "b"); + collector.missingImplementedInterface("a", "c"); DesugarDepsInfo info = extractProto(collector); - assertThat(info.getMissingInterfaceList()) - .containsExactly( - Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("b")).build(), - Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("c")).build(), - Dependency.newBuilder().setOrigin(wrapType("c")).setTarget(wrapType("b")).build()); + assertThat(info.getMissingInterfaceList().get(0)).isEqualTo(dependency("a", "b")); + assertThat(info.getMissingInterfaceList().get(1)).isEqualTo(dependency("a", "c")); + assertThat(info.getMissingInterfaceList().get(2)).isEqualTo(dependency("c", "b")); } @Test public void testRecordExtendedInterfaces() throws Exception { MetadataCollector collector = new MetadataCollector(false); - collector.recordExtendedInterfaces("a", "b", "c"); - collector.recordExtendedInterfaces("b"); collector.recordExtendedInterfaces("c", "d"); + collector.recordExtendedInterfaces("a", "c", "b"); + collector.recordExtendedInterfaces("b"); DesugarDepsInfo info = extractProto(collector); - assertThat(info.getInterfaceWithSupertypesList()) - .containsExactly( - InterfaceDetails.newBuilder() - .setOrigin(wrapType("a")) - .addAllExtendedInterface(ImmutableList.of(wrapType("b"), wrapType("c"))) - .build(), - InterfaceDetails.newBuilder() - .setOrigin(wrapType("c")) - .addAllExtendedInterface(ImmutableList.of(wrapType("d"))) - .build()); + + assertThat(info.getInterfaceWithSupertypesList().get(0)) + .isEqualTo(interfaceDetails("a", "b", "c")); + assertThat(info.getInterfaceWithSupertypesList().get(0).getExtendedInterfaceList().get(0)) + .isEqualTo(wrapType("b")); + assertThat(info.getInterfaceWithSupertypesList().get(0).getExtendedInterfaceList().get(1)) + .isEqualTo(wrapType("c")); + + assertThat(info.getInterfaceWithSupertypesList().get(1)).isEqualTo(interfaceDetails("c", "d")); } @Test public void testRecordDefaultMethods() throws Exception { MetadataCollector collector = new MetadataCollector(false); - collector.recordDefaultMethods("a", 0); collector.recordDefaultMethods("b", 1); + collector.recordDefaultMethods("a", 0); DesugarDepsInfo info = extractProto(collector); - assertThat(info.getInterfaceWithCompanionList()) - .containsExactly( - InterfaceWithCompanion.newBuilder() - .setOrigin(wrapType("a")) - .setNumDefaultMethods(0) - .build(), - InterfaceWithCompanion.newBuilder() - .setOrigin(wrapType("b")) - .setNumDefaultMethods(1) - .build()); + assertThat(info.getInterfaceWithCompanionList().get(0)) + .isEqualTo(interfaceWithCompanion("a", 0)); + assertThat(info.getInterfaceWithCompanionList().get(1)) + .isEqualTo(interfaceWithCompanion("b", 1)); + } + + private DesugarDeps.InterfaceWithCompanion interfaceWithCompanion(String origin, int count) { + return DesugarDeps.InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType(origin)) + .setNumDefaultMethods(count) + .build(); + } + + private DesugarDeps.InterfaceDetails interfaceDetails(String originName, String... interfaces) { + return InterfaceDetails.newBuilder() + .setOrigin(wrapType(originName)) + .addAllExtendedInterface( + Arrays.stream(interfaces) + .map(MetadataCollectorTest::wrapType) + .collect(Collectors.toList())) + .build(); + } + + private DesugarDeps.Dependency dependency(String origin, String target) { + return DesugarDeps.Dependency.newBuilder() + .setOrigin(wrapType(origin)) + .setTarget(wrapType(target)) + .build(); } private static DesugarDeps.Type wrapType(String name) { diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java index 2a8a0e3bd95819..d2116d119fd6f3 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java @@ -22,13 +22,23 @@ import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceDetails; import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceWithCompanion; import com.google.devtools.build.android.r8.DependencyCollector; + +import java.util.ArrayList; +import java.util.Comparator; + import javax.annotation.Nullable; /** Dependency collector that emits collected metadata as a {@link DesugarDepsInfo} proto. */ public final class MetadataCollector implements DependencyCollector { private final boolean tolerateMissingDeps; - private final DesugarDepsInfo.Builder info = DesugarDeps.DesugarDepsInfo.newBuilder(); + private final DesugarDeps.DesugarDepsInfo.Builder info = DesugarDeps.DesugarDepsInfo.newBuilder(); + + private final ArrayList assumePresents = new ArrayList(); + private final ArrayList missingInterfaces = new ArrayList(); + private final ArrayList interfacesWithSupertypes = new ArrayList(); + private final ArrayList interfacesWithCompanion = + new ArrayList(); public MetadataCollector(boolean tolerateMissingDeps) { this.tolerateMissingDeps = tolerateMissingDeps; @@ -43,8 +53,8 @@ private static boolean isInterfaceCompanionClass(String name) { public void assumeCompanionClass(String origin, String target) { checkArgument( isInterfaceCompanionClass(target), "target not a companion: %s -> %s", origin, target); - info.addAssumePresent( - Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target))); + assumePresents.add( + Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)).build()); } @Override @@ -59,37 +69,68 @@ public void missingImplementedInterface(String origin, String target) { "Couldn't find interface %s on the classpath for desugaring %s", target, origin); - info.addMissingInterface( - Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target))); + missingInterfaces.add( + Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)).build()); } @Override public void recordExtendedInterfaces(String origin, String... targets) { if (targets.length > 0) { InterfaceDetails.Builder details = InterfaceDetails.newBuilder().setOrigin(wrapType(origin)); + ArrayList types = new ArrayList<>(); for (String target : targets) { - details.addExtendedInterface(wrapType(target)); + types.add(wrapType(target)); } - info.addInterfaceWithSupertypes(details); + types.sort(Comparator.comparing(DesugarDeps.Type::getBinaryName)); + details.addAllExtendedInterface(types); + interfacesWithSupertypes.add(details.build()); } } @Override public void recordDefaultMethods(String origin, int count) { checkArgument(!isInterfaceCompanionClass(origin), "seems to be a companion: %s", origin); - info.addInterfaceWithCompanion( + interfacesWithCompanion.add( InterfaceWithCompanion.newBuilder() .setOrigin(wrapType(origin)) - .setNumDefaultMethods(count)); + .setNumDefaultMethods(count) + .build()); } @Override @Nullable public byte[] toByteArray() { - DesugarDepsInfo result = info.build(); - return DesugarDepsInfo.getDefaultInstance().equals(result) ? null : result.toByteArray(); + DesugarDeps.DesugarDepsInfo result = buildInfo(); + return DesugarDeps.DesugarDepsInfo.getDefaultInstance().equals(result) + ? null + : result.toByteArray(); } + private DesugarDeps.DesugarDepsInfo buildInfo() { + assumePresents.sort(dependencyComparator); + missingInterfaces.sort(dependencyComparator); + + interfacesWithSupertypes.sort(interfaceDetailComparator); + + interfacesWithCompanion.sort(interFaceWithCompanionComparator); + + info.addAllAssumePresent(assumePresents); + info.addAllMissingInterface(missingInterfaces); + info.addAllInterfaceWithSupertypes(interfacesWithSupertypes); + info.addAllInterfaceWithCompanion(interfacesWithCompanion); + return info.build(); + } + + Comparator dependencyComparator = + Comparator.comparing((DesugarDeps.Dependency o) -> o.getOrigin().getBinaryName()) + .thenComparing((DesugarDeps.Dependency o) -> o.getTarget().getBinaryName()); + + Comparator interfaceDetailComparator = + Comparator.comparing((DesugarDeps.InterfaceDetails o) -> o.getOrigin().getBinaryName()); + + Comparator interFaceWithCompanionComparator = + Comparator.comparing(o -> o.getOrigin().getBinaryName()); + private static DesugarDeps.Type wrapType(String internalName) { return DesugarDeps.Type.newBuilder().setBinaryName(internalName).build(); }