Skip to content

Commit 6b0ee86

Browse files
richardstartintrask
richardstartin
authored and
trask
committed
Exclude JDK class references at build time, reduce allocation in ReferenceMatcher (DataDog/dd-trace-java#1613)
1 parent cab03e0 commit 6b0ee86

File tree

4 files changed

+74
-48
lines changed

4 files changed

+74
-48
lines changed

‎agent-tooling/src/main/java/io/opentelemetry/auto/tooling/muzzle/Reference.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.io.StringWriter;
2222
import java.util.ArrayList;
2323
import java.util.Arrays;
24+
import java.util.EnumSet;
2425
import java.util.HashSet;
2526
import java.util.List;
2627
import java.util.Set;
@@ -612,7 +613,7 @@ public int hashCode() {
612613

613614
public static class Builder {
614615
private final Set<Source> sources = new HashSet<>();
615-
private final Set<Flag> flags = new HashSet<>();
616+
private final Set<Flag> flags = EnumSet.noneOf(Flag.class);
616617
private final String className;
617618
private String superName = null;
618619
private final Set<String> interfaces = new HashSet<>();

‎agent-tooling/src/main/java/io/opentelemetry/auto/tooling/muzzle/ReferenceCreator.java

+7-4
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,13 @@ public Map<String, Reference> getReferences() {
188188
}
189189

190190
private void addReference(final Reference ref) {
191-
if (references.containsKey(ref.getClassName())) {
192-
references.put(ref.getClassName(), references.get(ref.getClassName()).merge(ref));
193-
} else {
194-
references.put(ref.getClassName(), ref);
191+
if (!ref.getClassName().startsWith("java.")) {
192+
Reference reference = references.get(ref.getClassName());
193+
if (null == reference) {
194+
references.put(ref.getClassName(), ref);
195+
} else {
196+
references.put(ref.getClassName(), reference.merge(ref));
197+
}
195198
}
196199
}
197200

‎agent-tooling/src/main/java/io/opentelemetry/auto/tooling/muzzle/ReferenceMatcher.java

+63-39
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,13 @@ public List<Reference.Mismatch> getMismatchedReferenceSources(ClassLoader loader
102102
loader = Utils.getBootstrapProxy();
103103
}
104104

105-
final List<Mismatch> mismatches = new ArrayList<>();
105+
List<Mismatch> mismatches = Collections.emptyList();
106106

107107
for (final Reference reference : references) {
108108
// Don't reference-check helper classes.
109109
// They will be injected by the instrumentation's HelperInjector.
110110
if (!helperClassNames.contains(reference.getClassName())) {
111-
mismatches.addAll(checkMatch(reference, loader));
111+
mismatches = lazyAddAll(mismatches, checkMatch(reference, loader));
112112
}
113113
}
114114

@@ -126,10 +126,8 @@ private static List<Reference.Mismatch> checkMatch(
126126
final TypePool typePool =
127127
AgentTooling.poolStrategy()
128128
.typePool(AgentTooling.locationStrategy().classFileLocator(loader), loader);
129-
final List<Mismatch> mismatches = new ArrayList<>();
130129
try {
131-
final TypePool.Resolution resolution =
132-
typePool.describe(Utils.getClassName(reference.getClassName()));
130+
final TypePool.Resolution resolution = typePool.describe(reference.getClassName());
133131
if (!resolution.isResolved()) {
134132
return Collections.<Mismatch>singletonList(
135133
new Mismatch.MissingClass(
@@ -141,41 +139,45 @@ private static List<Reference.Mismatch> checkMatch(
141139
// bytebuddy throws an illegal state exception with this message if it cannot resolve types
142140
// TODO: handle missing type resolutions without catching bytebuddy's exceptions
143141
final String className = e.getMessage().replace("Cannot resolve type description for ", "");
144-
mismatches.add(
142+
return Collections.<Mismatch>singletonList(
145143
new Mismatch.MissingClass(reference.getSources().toArray(new Source[0]), className));
146144
} else {
147145
// Shouldn't happen. Fail the reference check and add a mismatch for debug logging.
148-
mismatches.add(new Mismatch.ReferenceCheckError(e, reference, loader));
146+
return Collections.<Mismatch>singletonList(
147+
new Mismatch.ReferenceCheckError(e, reference, loader));
149148
}
150149
}
151-
return mismatches;
152150
}
153151

154152
public static List<Reference.Mismatch> checkMatch(
155153
final Reference reference, final TypeDescription typeOnClasspath) {
156-
final List<Mismatch> mismatches = new ArrayList<>();
154+
List<Mismatch> mismatches = Collections.emptyList();
157155

158156
for (final Reference.Flag flag : reference.getFlags()) {
159157
if (!flag.matches(typeOnClasspath.getModifiers())) {
160158
final String desc = reference.getClassName();
161-
mismatches.add(
162-
new Mismatch.MissingFlag(
163-
reference.getSources().toArray(new Source[0]),
164-
desc,
165-
flag,
166-
typeOnClasspath.getModifiers()));
159+
mismatches =
160+
lazyAdd(
161+
mismatches,
162+
new Mismatch.MissingFlag(
163+
reference.getSources().toArray(new Source[0]),
164+
desc,
165+
flag,
166+
typeOnClasspath.getModifiers()));
167167
}
168168
}
169169

170170
for (final Reference.Field fieldRef : reference.getFields()) {
171171
final FieldDescription.InDefinedShape fieldDescription = findField(fieldRef, typeOnClasspath);
172172
if (fieldDescription == null) {
173-
mismatches.add(
174-
new Reference.Mismatch.MissingField(
175-
fieldRef.getSources().toArray(new Reference.Source[0]),
176-
reference.getClassName(),
177-
fieldRef.getName(),
178-
fieldRef.getType().getInternalName()));
173+
mismatches =
174+
lazyAdd(
175+
mismatches,
176+
new Reference.Mismatch.MissingField(
177+
fieldRef.getSources().toArray(new Reference.Source[0]),
178+
reference.getClassName(),
179+
fieldRef.getName(),
180+
fieldRef.getType().getInternalName()));
179181
} else {
180182
for (final Reference.Flag flag : fieldRef.getFlags()) {
181183
if (!flag.matches(fieldDescription.getModifiers())) {
@@ -184,12 +186,14 @@ public static List<Reference.Mismatch> checkMatch(
184186
+ "#"
185187
+ fieldRef.getName()
186188
+ fieldRef.getType().getInternalName();
187-
mismatches.add(
188-
new Mismatch.MissingFlag(
189-
fieldRef.getSources().toArray(new Source[0]),
190-
desc,
191-
flag,
192-
fieldDescription.getModifiers()));
189+
mismatches =
190+
lazyAdd(
191+
mismatches,
192+
new Mismatch.MissingFlag(
193+
fieldRef.getSources().toArray(new Source[0]),
194+
desc,
195+
flag,
196+
fieldDescription.getModifiers()));
193197
}
194198
}
195199
}
@@ -199,27 +203,30 @@ public static List<Reference.Mismatch> checkMatch(
199203
final MethodDescription.InDefinedShape methodDescription =
200204
findMethod(methodRef, typeOnClasspath);
201205
if (methodDescription == null) {
202-
mismatches.add(
203-
new Reference.Mismatch.MissingMethod(
204-
methodRef.getSources().toArray(new Reference.Source[0]),
205-
methodRef.getName(),
206-
methodRef.getDescriptor()));
206+
mismatches =
207+
lazyAdd(
208+
mismatches,
209+
new Reference.Mismatch.MissingMethod(
210+
methodRef.getSources().toArray(new Reference.Source[0]),
211+
methodRef.getName(),
212+
methodRef.getDescriptor()));
207213
} else {
208214
for (final Reference.Flag flag : methodRef.getFlags()) {
209215
if (!flag.matches(methodDescription.getModifiers())) {
210216
final String desc =
211217
reference.getClassName() + "#" + methodRef.getName() + methodRef.getDescriptor();
212-
mismatches.add(
213-
new Mismatch.MissingFlag(
214-
methodRef.getSources().toArray(new Source[0]),
215-
desc,
216-
flag,
217-
methodDescription.getModifiers()));
218+
mismatches =
219+
lazyAdd(
220+
mismatches,
221+
new Mismatch.MissingFlag(
222+
methodRef.getSources().toArray(new Source[0]),
223+
desc,
224+
flag,
225+
methodDescription.getModifiers()));
218226
}
219227
}
220228
}
221229
}
222-
223230
return mismatches;
224231
}
225232

@@ -293,4 +300,21 @@ private static MethodDescription.InDefinedShape findMethod(
293300
}
294301
return null;
295302
}
303+
304+
// optimization to avoid ArrayList allocation in the common case when there are no mismatches
305+
private static List<Mismatch> lazyAdd(List<Mismatch> mismatches, Mismatch mismatch) {
306+
List<Mismatch> result = mismatches.isEmpty() ? new ArrayList<Mismatch>() : mismatches;
307+
result.add(mismatch);
308+
return result;
309+
}
310+
311+
// optimization to avoid ArrayList allocation in the common case when there are no mismatches
312+
private static List<Mismatch> lazyAddAll(List<Mismatch> mismatches, List<Mismatch> toAdd) {
313+
if (!toAdd.isEmpty()) {
314+
List<Mismatch> result = mismatches.isEmpty() ? new ArrayList<Mismatch>() : mismatches;
315+
result.addAll(toAdd);
316+
return result;
317+
}
318+
return mismatches;
319+
}
296320
}

‎testing/src/test/groovy/muzzle/ReferenceCreatorTest.groovy

+2-4
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,18 @@ class ReferenceCreatorTest extends AgentTestRunner {
2929
Map<String, Reference> references = ReferenceCreator.createReferencesFrom(MethodBodyAdvice.getName(), this.getClass().getClassLoader())
3030

3131
expect:
32-
references.get('java.lang.Object') != null
33-
references.get('java.lang.String') != null
3432
references.get('muzzle.TestClasses$MethodBodyAdvice$A') != null
3533
references.get('muzzle.TestClasses$MethodBodyAdvice$B') != null
3634
references.get('muzzle.TestClasses$MethodBodyAdvice$SomeInterface') != null
3735
references.get('muzzle.TestClasses$MethodBodyAdvice$SomeImplementation') != null
38-
references.keySet().size() == 6
36+
references.keySet().size() == 4
3937

4038
// interface flags
4139
references.get('muzzle.TestClasses$MethodBodyAdvice$B').getFlags().contains(Reference.Flag.NON_INTERFACE)
4240
references.get('muzzle.TestClasses$MethodBodyAdvice$SomeInterface').getFlags().contains(Reference.Flag.INTERFACE)
4341

4442
// class access flags
45-
references.get('java.lang.Object').getFlags().contains(Reference.Flag.PUBLIC)
43+
references.get('muzzle.TestClasses$MethodBodyAdvice$A').getFlags().contains(Reference.Flag.PACKAGE_OR_HIGHER)
4644
references.get('muzzle.TestClasses$MethodBodyAdvice$B').getFlags().contains(Reference.Flag.PACKAGE_OR_HIGHER)
4745

4846
// method refs

0 commit comments

Comments
 (0)