Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove remaining threadlocals for loom support #3286

Merged
merged 6 commits into from Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.asciidoc
Expand Up @@ -33,7 +33,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:

[float]
===== Features
* Virtual thread support - {pull}3244[#3244]
* Virtual thread support - {pull}3244[#3244], {pull}3286[#3286]

[float]
===== Bug fixes
Expand Down
Expand Up @@ -49,12 +49,11 @@
*/
public class Transaction extends AbstractSpan<Transaction> implements co.elastic.apm.agent.tracer.Transaction<Transaction> {

private static final ThreadLocal<Labels.Mutable> labelsThreadLocal = new ThreadLocal<Labels.Mutable>() {
@Override
protected Labels.Mutable initialValue() {
return Labels.Mutable.of();
}
};
/**
* Mutable labels instance used when reporting transaction metrics.
* This is a field to prevent allocations.
*/
private final Labels.Mutable labelsMutable = Labels.Mutable.of();

/**
* Context
Expand Down Expand Up @@ -507,9 +506,8 @@ private void trackMetrics() {
if (type == null) {
return;
}
final Labels.Mutable labels = labelsThreadLocal.get();
labels.resetState();
labels.serviceName(getTraceContext().getServiceName())
labelsMutable.resetState();
labelsMutable.serviceName(getTraceContext().getServiceName())
.serviceVersion(getTraceContext().getServiceVersion())
.transactionName(name)
.transactionType(type);
Expand All @@ -529,8 +527,8 @@ private void trackMetrics() {
if (subtype.equals("")) {
subtype = null;
}
labels.spanType(spanType).spanSubType(subtype);
metricRegistry.updateTimer("span.self_time", labels, timer.getTotalTimeUs(), timer.getCount());
labelsMutable.spanType(spanType).spanSubType(subtype);
metricRegistry.updateTimer("span.self_time", labelsMutable, timer.getTotalTimeUs(), timer.getCount());
timer.resetState();
}
}
Expand Down
Expand Up @@ -18,9 +18,9 @@
*/
package co.elastic.apm.agent.sdk.internal.db.signature;

import co.elastic.apm.agent.sdk.weakconcurrent.DetachedThreadLocal;
import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
import co.elastic.apm.agent.sdk.internal.pooling.ObjectHandle;
import co.elastic.apm.agent.sdk.internal.pooling.ObjectPool;
import co.elastic.apm.agent.sdk.internal.pooling.ObjectPooling;

import javax.annotation.Nullable;
import java.util.concurrent.Callable;
Expand All @@ -44,7 +44,7 @@ public class SignatureParser {
*/
private static final int QUERY_LENGTH_CACHE_UPPER_THRESHOLD = 10_000;

private final DetachedThreadLocal<Scanner> scanner;
private final ObjectPool<? extends ObjectHandle<Scanner>> scannerPool;

/**
* Not using weak keys because ORMs like Hibernate generate equal SQL strings for the same query but don't reuse the same string instance.
Expand All @@ -64,20 +64,7 @@ public Scanner call() {
}

public SignatureParser(final Callable<Scanner> scannerAllocator) {
scanner = WeakConcurrent
.<Scanner>threadLocalBuilder()
.withDefaultValueSupplier(new WeakMap.DefaultValueSupplier<Thread, Scanner>() {
@Nullable
@Override
public Scanner getDefaultValue(Thread key) {
try {
return scannerAllocator.call();
} catch (Exception e) {
throw new RuntimeException(e);
}
}
})
.build();
scannerPool = ObjectPooling.createWithDefaultFactory(scannerAllocator);
}

public void querySignature(String query, StringBuilder signature, boolean preparedStatement) {
Expand All @@ -98,13 +85,15 @@ public void querySignature(String query, StringBuilder signature, @Nullable Stri
return;
}
}
Scanner scanner = this.scanner.get();
scanner.setQuery(query);
parse(scanner, query, signature, dbLink);
try (ObjectHandle<Scanner> pooledScanner = scannerPool.createInstance()) {
Scanner scanner = pooledScanner.get();
scanner.setQuery(query);
parse(scanner, query, signature, dbLink);

if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) {
// we don't mind a small overshoot due to race conditions
signatureCache.put(query, new String[]{signature.toString(), dbLink != null ? dbLink.toString() : ""});
if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) {
// we don't mind a small overshoot due to race conditions
signatureCache.put(query, new String[]{signature.toString(), dbLink != null ? dbLink.toString() : ""});
}
}
}

Expand Down
Expand Up @@ -18,43 +18,25 @@
*/
package co.elastic.apm.agent.okhttp;

import co.elastic.apm.agent.sdk.state.GlobalState;
import co.elastic.apm.agent.sdk.weakconcurrent.DetachedThreadLocal;
import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;

import javax.annotation.Nullable;

@GlobalState
public class OkHttpClientHelper {

/**
* Used to avoid allocations when calculating destination host name.
*/
public static final DetachedThreadLocal<StringBuilder> destinationHostName = WeakConcurrent
.<StringBuilder>threadLocalBuilder()
.withDefaultValueSupplier(new WeakMap.DefaultValueSupplier<Thread, StringBuilder>() {
@Override
public StringBuilder getDefaultValue(Thread t) {
return new StringBuilder();
}
})
.build();
public class OkHttpClientHelper {

/**
* NOTE: this method returns a StringBuilder instance that is kept as this class's ThreadLocal. Callers of this
* method MAY NOT KEEP A REFERENCE TO THE RETURNED OBJECT, only copy its contents.
* If this method needs to perform corrections on the hostname, it has to allocate a new StringBuilder.
* We accept this due to the fact that this method is called once per HTTP call, making the allocation neglectable
* overhead compared to the allocations performed for the HTTP call itself.
*
* @param originalHostName the original host name retrieved from the OkHttp client
* @return a StringBuilder instance that is kept as a ThreadLocal
* @return the potentially corrected host name
*/
@Nullable
public static CharSequence computeHostName(@Nullable String originalHostName) {
CharSequence hostName = originalHostName;
// okhttp represents IPv6 addresses without square brackets, as opposed to all others, so we should add them
if (originalHostName != null && originalHostName.contains(":") && !originalHostName.startsWith("[")) {
StringBuilder sb = destinationHostName.get();
sb.setLength(0);
StringBuilder sb = new StringBuilder(originalHostName.length() + 2);
sb.append("[").append(originalHostName).append("]");
hostName = sb;
}
Expand Down