Skip to content

Commit b381b01

Browse files
authored
Reduce iterator allocations on hot paths (#10745)
1 parent 00f7bf1 commit b381b01

File tree

5 files changed

+35
-33
lines changed

5 files changed

+35
-33
lines changed

docs/contributing/style-guideline.md

+11-4
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,14 @@ It is ok to use `Optional` in places where it does not leak into public API sign
137137
Also, avoid `Optional` usage on the hot path (instrumentation code), unless the instrumented library
138138
itself uses it.
139139

140-
## `java.util.stream.Stream` usage
141-
142-
Avoid streams on the hot path (instrumentation code), unless the instrumented library itself uses
143-
them.
140+
## Hot path constraints
141+
142+
Avoid allocations whenever possible on the hot path (instrumentation code).
143+
This includes `Iterator` allocations from collections; note that
144+
`for(SomeType t : plainJavaArray)` does not allocate an iterator object.
145+
Non-allocating stream api usage on the hot path is acceptable but may not
146+
fit the surrounding code style; this is a judgement call. Note that
147+
some stream apis make it much more difficult to allocate efficiently
148+
(e.g., `collect` with presized sink data structures involves
149+
convoluted `Supplier` code, or lambdas passed to `forEach` might be
150+
capturing/allocating lambdas).

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

+15-20
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil;
1717
import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
1818
import java.time.Instant;
19-
import java.util.ArrayList;
20-
import java.util.List;
21-
import java.util.ListIterator;
2219
import java.util.concurrent.TimeUnit;
2320
import javax.annotation.Nullable;
2421

@@ -72,25 +69,25 @@ public static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> builder
7269
private final SpanNameExtractor<? super REQUEST> spanNameExtractor;
7370
private final SpanKindExtractor<? super REQUEST> spanKindExtractor;
7471
private final SpanStatusExtractor<? super REQUEST, ? super RESPONSE> spanStatusExtractor;
75-
private final List<? extends SpanLinksExtractor<? super REQUEST>> spanLinksExtractors;
76-
private final List<? extends AttributesExtractor<? super REQUEST, ? super RESPONSE>>
77-
attributesExtractors;
78-
private final List<? extends ContextCustomizer<? super REQUEST>> contextCustomizers;
79-
private final List<? extends OperationListener> operationListeners;
72+
private final SpanLinksExtractor<? super REQUEST>[] spanLinksExtractors;
73+
private final AttributesExtractor<? super REQUEST, ? super RESPONSE>[] attributesExtractors;
74+
private final ContextCustomizer<? super REQUEST>[] contextCustomizers;
75+
private final OperationListener[] operationListeners;
8076
private final ErrorCauseExtractor errorCauseExtractor;
8177
private final boolean enabled;
8278
private final SpanSuppressor spanSuppressor;
8379

80+
@SuppressWarnings({"rawtypes", "unchecked"})
8481
Instrumenter(InstrumenterBuilder<REQUEST, RESPONSE> builder) {
8582
this.instrumentationName = builder.instrumentationName;
8683
this.tracer = builder.buildTracer();
8784
this.spanNameExtractor = builder.spanNameExtractor;
8885
this.spanKindExtractor = builder.spanKindExtractor;
8986
this.spanStatusExtractor = builder.spanStatusExtractor;
90-
this.spanLinksExtractors = new ArrayList<>(builder.spanLinksExtractors);
91-
this.attributesExtractors = new ArrayList<>(builder.attributesExtractors);
92-
this.contextCustomizers = new ArrayList<>(builder.contextCustomizers);
93-
this.operationListeners = builder.buildOperationListeners();
87+
this.spanLinksExtractors = builder.spanLinksExtractors.toArray(new SpanLinksExtractor[0]);
88+
this.attributesExtractors = builder.attributesExtractors.toArray(new AttributesExtractor[0]);
89+
this.contextCustomizers = builder.contextCustomizers.toArray(new ContextCustomizer[0]);
90+
this.operationListeners = builder.buildOperationListeners().toArray(new OperationListener[0]);
9491
this.errorCauseExtractor = builder.errorCauseExtractor;
9592
this.enabled = builder.enabled;
9693
this.spanSuppressor = builder.buildSpanSuppressor();
@@ -193,12 +190,12 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan
193190
Span span = spanBuilder.setParent(context).startSpan();
194191
context = context.with(span);
195192

196-
if (!operationListeners.isEmpty()) {
193+
if (operationListeners.length != 0) {
197194
// operation listeners run after span start, so that they have access to the current span
198195
// for capturing exemplars
199196
long startNanos = getNanos(startTime);
200-
for (OperationListener operationListener : operationListeners) {
201-
context = operationListener.onStart(context, attributes, startNanos);
197+
for (int i = 0; i < operationListeners.length; i++) {
198+
context = operationListeners[i].onStart(context, attributes, startNanos);
202199
}
203200
}
204201

@@ -231,12 +228,10 @@ private void doEnd(
231228
}
232229
span.setAllAttributes(attributes);
233230

234-
if (!operationListeners.isEmpty()) {
231+
if (operationListeners.length != 0) {
235232
long endNanos = getNanos(endTime);
236-
ListIterator<? extends OperationListener> i =
237-
operationListeners.listIterator(operationListeners.size());
238-
while (i.hasPrevious()) {
239-
i.previous().onEnd(context, attributes, endNanos);
233+
for (int i = operationListeners.length - 1; i >= 0; i--) {
234+
operationListeners[i].onEnd(context, attributes, endNanos);
240235
}
241236
}
242237

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressors.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
6363

6464
static final class BySpanKey implements SpanSuppressor {
6565

66-
private final Set<SpanKey> spanKeys;
66+
private final SpanKey[] spanKeys;
6767

6868
BySpanKey(Set<SpanKey> spanKeys) {
69-
this.spanKeys = spanKeys;
69+
this.spanKeys = spanKeys.toArray(new SpanKey[0]);
7070
}
7171

7272
@Override

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/CapturedHttpHeadersUtil.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
package io.opentelemetry.instrumentation.api.semconv.http;
77

8-
import static java.util.Collections.unmodifiableList;
9-
108
import io.opentelemetry.api.common.AttributeKey;
119
import java.util.List;
1210
import java.util.Locale;
@@ -24,9 +22,11 @@ final class CapturedHttpHeadersUtil {
2422
private static final ConcurrentMap<String, AttributeKey<List<String>>> responseKeysCache =
2523
new ConcurrentHashMap<>();
2624

27-
static List<String> lowercase(List<String> names) {
28-
return unmodifiableList(
29-
names.stream().map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toList()));
25+
static String[] lowercase(List<String> names) {
26+
return names.stream()
27+
.map(s -> s.toLowerCase(Locale.ROOT))
28+
.collect(Collectors.toList())
29+
.toArray(new String[0]);
3030
}
3131

3232
static AttributeKey<List<String>> requestAttributeKey(String headerName) {

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpCommonAttributesExtractor.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ abstract class HttpCommonAttributesExtractor<
3737

3838
final GETTER getter;
3939
private final HttpStatusCodeConverter statusCodeConverter;
40-
private final List<String> capturedRequestHeaders;
41-
private final List<String> capturedResponseHeaders;
40+
private final String[] capturedRequestHeaders;
41+
private final String[] capturedResponseHeaders;
4242
private final Set<String> knownMethods;
4343

4444
HttpCommonAttributesExtractor(

0 commit comments

Comments
 (0)