From d42dccb9bd4fc0053fd84e425d00bc047040e1b0 Mon Sep 17 00:00:00 2001 From: Ago Allikmaa Date: Thu, 26 Nov 2020 11:07:07 +0200 Subject: [PATCH] Make ByteBuddy task deterministic by using linked sets/maps. (#1770) * Make ByteBuddy task deterministic by using linked sets/maps. * Add class-level comments explaining the use of Linked maps/sets. --- .../javaagent/tooling/muzzle/Reference.java | 38 ++++++++++--------- .../muzzle/collector/ReferenceCollector.java | 8 +++- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/Reference.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/Reference.java index 39ff7bd4c0..4be2cce8cc 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/Reference.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/Reference.java @@ -9,7 +9,7 @@ import io.opentelemetry.javaagent.tooling.Utils; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; import java.util.Set; @@ -17,7 +17,11 @@ import net.bytebuddy.jar.asm.Opcodes; import net.bytebuddy.jar.asm.Type; import org.checkerframework.checker.nullness.qual.Nullable; -/** This class represents a reference to a Java class used in an instrumentation advice code. */ +/** + * This class represents a reference to a Java class used in an instrumentation advice code. {@link + * LinkedHashSet} is used for all sets to guarantee a deterministic order of iteration, so that + * bytecode generated based on them would also be deterministic. + */ public final class Reference { private final Set sources; private final String className; @@ -96,7 +100,7 @@ public final class Reference { } private static Set merge(Set set1, Set set2) { - Set set = new HashSet<>(); + Set set = new LinkedHashSet<>(); set.addAll(set1); set.addAll(set2); return set; @@ -112,7 +116,7 @@ public final class Reference { merged.set(i, merged.get(i).merge(method)); } } - return new HashSet<>(merged); + return new LinkedHashSet<>(merged); } private static Set mergeFields(Set fields1, Set fields2) { @@ -125,7 +129,7 @@ public final class Reference { merged.set(i, merged.get(i).merge(field)); } } - return new HashSet<>(merged); + return new LinkedHashSet<>(merged); } private static Set mergeFlags(Set flags1, Set flags2) { @@ -344,14 +348,14 @@ public final class Reference { public Method( Source[] sources, Flag[] flags, String name, Type returnType, Type[] parameterTypes) { this( - new HashSet<>(Arrays.asList(sources)), - new HashSet<>(Arrays.asList(flags)), + new LinkedHashSet<>(Arrays.asList(sources)), + new LinkedHashSet<>(Arrays.asList(flags)), name, returnType, Arrays.asList(parameterTypes)); } - public Method( + private Method( Set sources, Set flags, String name, @@ -389,11 +393,11 @@ public final class Reference { throw new IllegalStateException("illegal merge " + this + " != " + anotherMethod); } - Set mergedSources = new HashSet<>(); + Set mergedSources = new LinkedHashSet<>(); mergedSources.addAll(sources); mergedSources.addAll(anotherMethod.sources); - Set mergedFlags = new HashSet<>(); + Set mergedFlags = new LinkedHashSet<>(); mergedFlags.addAll(flags); mergedFlags.addAll(anotherMethod.flags); @@ -434,8 +438,8 @@ public final class Reference { private final Type type; public Field(Source[] sources, Flag[] flags, String name, Type fieldType) { - this.sources = new HashSet<>(Arrays.asList(sources)); - this.flags = new HashSet<>(Arrays.asList(flags)); + this.sources = new LinkedHashSet<>(Arrays.asList(sources)); + this.flags = new LinkedHashSet<>(Arrays.asList(flags)); this.name = name; type = fieldType; } @@ -491,11 +495,11 @@ public final class Reference { } public static class Builder { - private final Set sources = new HashSet<>(); - private final Set flags = new HashSet<>(); + private final Set sources = new LinkedHashSet<>(); + private final Set flags = new LinkedHashSet<>(); private final String className; private String superName = null; - private final Set interfaces = new HashSet<>(); + private final Set interfaces = new LinkedHashSet<>(); private final List fields = new ArrayList<>(); private final List methods = new ArrayList<>(); @@ -567,8 +571,8 @@ public final class Reference { className, superName, interfaces, - new HashSet<>(fields), - new HashSet<>(methods)); + new LinkedHashSet<>(fields), + new LinkedHashSet<>(methods)); } } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java index 4280f19cef..02b68ce259 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java @@ -19,8 +19,8 @@ import java.io.InputStream; import java.net.URLConnection; import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -28,8 +28,12 @@ import java.util.Queue; import java.util.Set; import net.bytebuddy.jar.asm.ClassReader; +/** + * {@link LinkedHashMap} is used for reference map to guarantee a deterministic order of iteration, + * so that bytecode generated based on it would also be deterministic. + */ public class ReferenceCollector { - private final Map references = new HashMap<>(); + private final Map references = new LinkedHashMap<>(); private final MutableGraph helperSuperClassGraph = GraphBuilder.directed().build(); private final Set visitedClasses = new HashSet<>();