Skip to content

Commit

Permalink
make desugar dependencies deterministic,
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mohamadk authored and mohammadkahelghi-grabtaxi committed Nov 28, 2022
1 parent 1af61b2 commit 49e7027
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<DesugarDeps.Dependency> assumePresent = new ArrayList<>();
public ArrayList<DesugarDeps.Dependency> missingInterface = new ArrayList<>();
public ArrayList<DesugarDeps.InterfaceDetails> interfaceWithSupertypes = new ArrayList<>();
public ArrayList<DesugarDeps.InterfaceWithCompanion> 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<? super DesugarDeps.Dependency> dependencyComparator =
Comparator.comparing((DesugarDeps.Dependency o) -> o.getOrigin().getBinaryName())
.thenComparing((DesugarDeps.Dependency o) -> o.getTarget().getBinaryName());

Comparator<? super DesugarDeps.InterfaceDetails> interfaceDetailComparator =
Comparator.comparing((DesugarDeps.InterfaceDetails o) -> o.getOrigin().getBinaryName());

Comparator<? super DesugarDeps.InterfaceWithCompanion> interFaceWithCompanionComparator =
Comparator.comparing(o -> o.getOrigin().getBinaryName());
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,72 +22,79 @@
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;
}

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<DesugarDeps.Type> 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) {
Expand Down

0 comments on commit 49e7027

Please sign in to comment.