From 49e702774006c00a70573ba4cca34020139a8ff7 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 --- .../build/android/desugar/dependencies/BUILD | 15 +++ .../DesugarDependencyInfoWrapperTest.java | 94 +++++++++++++++++++ .../dependencies/MetadataCollectorTest.java | 62 ++++++------ .../DesugarDependencyInfoWrapper.java | 67 +++++++++++++ .../dependencies/MetadataCollector.java | 53 ++++++----- 5 files changed, 241 insertions(+), 50 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapperTest.java create mode 100644 src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapper.java diff --git a/src/test/java/com/google/devtools/build/android/desugar/dependencies/BUILD b/src/test/java/com/google/devtools/build/android/desugar/dependencies/BUILD index ee05a7ca0d1f85..e3024ec1f29c47 100644 --- a/src/test/java/com/google/devtools/build/android/desugar/dependencies/BUILD +++ b/src/test/java/com/google/devtools/build/android/desugar/dependencies/BUILD @@ -28,3 +28,18 @@ java_test( "//third_party:truth", ], ) + +java_test( + name = "DesugarDependencyInfoWrapperTest", + size = "small", + srcs = [ + "DesugarDependencyInfoWrapperTest.java", + ], + deps = [ + "//src/main/protobuf:desugar_deps_java_proto", + "//src/tools/android/java/com/google/devtools/build/android/desugar/dependencies", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) diff --git a/src/test/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapperTest.java b/src/test/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapperTest.java new file mode 100644 index 00000000000000..0eb07243e7dd78 --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapperTest.java @@ -0,0 +1,94 @@ +package com.google.devtools.build.android.desugar.dependencies; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.android.desugar.proto.DesugarDeps; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DesugarDependencyInfoWrapperTest { + + @Test + public void testOrderForAssumeCompanionClass() throws Exception { + DesugarDependencyInfoWrapper infoWrapper = new DesugarDependencyInfoWrapper(); + infoWrapper.assumeCompanionClass(dependency("a", "b$$CC")); + infoWrapper.assumeCompanionClass(dependency("b", "b$$CC")); + infoWrapper.assumeCompanionClass(dependency("a", "a$$CC")); + + DesugarDeps.DesugarDepsInfo info = extractProto(infoWrapper); + 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 { + DesugarDependencyInfoWrapper infoWrapper = new DesugarDependencyInfoWrapper(); + infoWrapper.missingImplementedInterface(dependency("a", "b$$CC")); + infoWrapper.missingImplementedInterface(dependency("b", "b$$CC")); + infoWrapper.missingImplementedInterface(dependency("a", "a$$CC")); + + DesugarDeps.DesugarDepsInfo info = extractProto(infoWrapper); + assertThat(info.getMissingInterfaceList().get(0)).isEqualTo(dependency("a", "a$$CC")); + assertThat(info.getMissingInterfaceList().get(1)).isEqualTo(dependency("a", "b$$CC")); + assertThat(info.getMissingInterfaceList().get(2)).isEqualTo(dependency("b", "b$$CC")); + } + + @Test + public void testRecordExtendedInterfaces() throws Exception { + DesugarDependencyInfoWrapper infoWrapper = new DesugarDependencyInfoWrapper(); + + infoWrapper.recordExtendedInterfaces(interfaceDetails("a")); + infoWrapper.recordExtendedInterfaces(interfaceDetails("c")); + infoWrapper.recordExtendedInterfaces(interfaceDetails("a")); + + DesugarDeps.DesugarDepsInfo info = extractProto(infoWrapper); + assertThat(info.getInterfaceWithSupertypesList().get(0)).isEqualTo(interfaceDetails("a")); + assertThat(info.getInterfaceWithSupertypesList().get(1)).isEqualTo(interfaceDetails("a")); + assertThat(info.getInterfaceWithSupertypesList().get(2)).isEqualTo(interfaceDetails("c")); + } + + @Test + public void testRecordDefaultMethods() throws Exception { + DesugarDependencyInfoWrapper infoWrapper = new DesugarDependencyInfoWrapper(); + + infoWrapper.recordDefaultMethods(interfaceWithCompanion("a",1)); + infoWrapper.recordDefaultMethods(interfaceWithCompanion("c",2)); + infoWrapper.recordDefaultMethods(interfaceWithCompanion("a",3)); + + DesugarDeps.DesugarDepsInfo info = extractProto(infoWrapper); + assertThat(info.getInterfaceWithCompanionList().get(0)).isEqualTo(interfaceWithCompanion("a",1)); + assertThat(info.getInterfaceWithCompanionList().get(1)).isEqualTo(interfaceWithCompanion("a",3)); + assertThat(info.getInterfaceWithCompanionList().get(2)).isEqualTo(interfaceWithCompanion("c",2)); + } + + private DesugarDeps.InterfaceWithCompanion interfaceWithCompanion(String origin, int count) { + return DesugarDeps.InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType(origin)) + .setNumDefaultMethods(count) + .build(); + } + + private DesugarDeps.InterfaceDetails interfaceDetails(String originName) { + return DesugarDeps.InterfaceDetails.newBuilder().setOrigin(wrapType(originName)).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) { + return DesugarDeps.Type.newBuilder().setBinaryName(name).build(); + } + + private DesugarDeps.DesugarDepsInfo extractProto(DesugarDependencyInfoWrapper collector) + throws Exception { + return DesugarDeps.DesugarDepsInfo.parseFrom(collector.toByteArray()); + } +} 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..de1ba8e395b7e3 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 @@ -43,10 +43,10 @@ public void testAssumeCompanionClass() throws Exception { 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()); + .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()); } @Test @@ -58,10 +58,10 @@ public void testMissingImplementedInterface() throws Exception { 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()); + .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()); } @Test @@ -72,16 +72,24 @@ public void testRecordExtendedInterfaces() throws Exception { collector.recordExtendedInterfaces("c", "d"); 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.newBuilder() + .setOrigin(wrapType("a")) + .addAllExtendedInterface(ImmutableList.of(wrapType("b"), wrapType("c"))) + .build()); + 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.newBuilder() + .setOrigin(wrapType("c")) + .addAllExtendedInterface(ImmutableList.of(wrapType("d"))) + .build()); } @Test @@ -92,15 +100,15 @@ public void testRecordDefaultMethods() throws Exception { DesugarDepsInfo info = extractProto(collector); assertThat(info.getInterfaceWithCompanionList()) - .containsExactly( - InterfaceWithCompanion.newBuilder() - .setOrigin(wrapType("a")) - .setNumDefaultMethods(0) - .build(), - InterfaceWithCompanion.newBuilder() - .setOrigin(wrapType("b")) - .setNumDefaultMethods(1) - .build()); + .containsExactly( + InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType("a")) + .setNumDefaultMethods(0) + .build(), + InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType("b")) + .setNumDefaultMethods(1) + .build()); } private static DesugarDeps.Type wrapType(String name) { diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapper.java b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapper.java new file mode 100644 index 00000000000000..ff5e9131b8ace5 --- /dev/null +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/DesugarDependencyInfoWrapper.java @@ -0,0 +1,67 @@ +package com.google.devtools.build.android.desugar.dependencies; + +import com.google.devtools.build.android.desugar.proto.DesugarDeps; + +import java.util.ArrayList; +import java.util.Comparator; + +/** + * a wrapper that is responsible to sort desugar dependency collector items since these items will + * be add to the jar file as metadata it should be deterministic so that it won't cause cache misses + */ +public class DesugarDependencyInfoWrapper { + public final DesugarDeps.DesugarDepsInfo.Builder info = DesugarDeps.DesugarDepsInfo.newBuilder(); + + public ArrayList assumePresent = new ArrayList<>(); + public ArrayList missingInterface = new ArrayList<>(); + public ArrayList interfaceWithSupertypes = new ArrayList<>(); + public ArrayList interfaceWithCompanion = new ArrayList<>(); + + public void assumeCompanionClass(DesugarDeps.Dependency dependency) { + assumePresent.add(dependency); + } + + public void missingImplementedInterface(DesugarDeps.Dependency dependency) { + missingInterface.add(dependency); + } + + public void recordExtendedInterfaces(DesugarDeps.InterfaceDetails interfaceDetails) { + interfaceWithSupertypes.add(interfaceDetails); + } + + public void recordDefaultMethods(DesugarDeps.InterfaceWithCompanion interfaceWithCompanion) { + this.interfaceWithCompanion.add(interfaceWithCompanion); + } + + public byte[] toByteArray() { + DesugarDeps.DesugarDepsInfo result = buildInfo(); + return DesugarDeps.DesugarDepsInfo.getDefaultInstance().equals(result) + ? null + : result.toByteArray(); + } + + private DesugarDeps.DesugarDepsInfo buildInfo() { + assumePresent.sort(dependencyComparator); + missingInterface.sort(dependencyComparator); + + interfaceWithSupertypes.sort(interfaceDetailComparator); + + interfaceWithCompanion.sort(interFaceWithCompanionComparator); + + info.addAllAssumePresent(assumePresent); + info.addAllMissingInterface(missingInterface); + info.addAllInterfaceWithSupertypes(interfaceWithSupertypes); + info.addAllInterfaceWithCompanion(interfaceWithCompanion); + 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()); +} 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..bec18bc48996fa 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,17 @@ 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 com.google.devtools.build.android.desugar.dependencies.DesugarDependencyInfoWrapper info = new com.google.devtools.build.android.desugar.dependencies.DesugarDependencyInfoWrapper(); public MetadataCollector(boolean tolerateMissingDeps) { this.tolerateMissingDeps = tolerateMissingDeps; @@ -36,58 +40,61 @@ public MetadataCollector(boolean tolerateMissingDeps) { private static boolean isInterfaceCompanionClass(String name) { return name.endsWith(INTERFACE_COMPANION_SUFFIX) - || name.endsWith(D8_INTERFACE_COMPANION_SUFFIX); + || name.endsWith(D8_INTERFACE_COMPANION_SUFFIX); } @Override 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))); + isInterfaceCompanionClass(target), "target not a companion: %s -> %s", origin, target); + info.assumeCompanionClass( + Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)).build()); } @Override public void missingImplementedInterface(String origin, String target) { checkArgument( - !isInterfaceCompanionClass(target), - "target seems to be a companion: %s -> %s", - origin, - target); + !isInterfaceCompanionClass(target), + "target seems to be a companion: %s -> %s", + origin, + target); checkState( - tolerateMissingDeps, - "Couldn't find interface %s on the classpath for desugaring %s", - target, - origin); - info.addMissingInterface( - Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target))); + tolerateMissingDeps, + "Couldn't find interface %s on the classpath for desugaring %s", + target, + origin); + info.missingImplementedInterface( + 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); + info.recordExtendedInterfaces(details.build()); } } @Override public void recordDefaultMethods(String origin, int count) { checkArgument(!isInterfaceCompanionClass(origin), "seems to be a companion: %s", origin); - info.addInterfaceWithCompanion( - InterfaceWithCompanion.newBuilder() - .setOrigin(wrapType(origin)) - .setNumDefaultMethods(count)); + info.recordDefaultMethods( + InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType(origin)) + .setNumDefaultMethods(count) + .build()); } @Override @Nullable public byte[] toByteArray() { - DesugarDepsInfo result = info.build(); - return DesugarDepsInfo.getDefaultInstance().equals(result) ? null : result.toByteArray(); + return info.toByteArray(); } private static DesugarDeps.Type wrapType(String internalName) {