Remove VirtualField#computeIfNull() method
MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java
package io.opentelemetry.instrumentation.api.field;
import io.opentelemetry.instrumentation.api.internal.RuntimeVirtualFieldSupplier;
-import java.util.function.Supplier;
import org.checkerframework.checker.nullness.qual.Nullable;
/**
@@ -47,12 +46,4 @@ public abstract class VirtualField<T, F> {
/** Sets the new value of this virtual field. */
public abstract void set(T object, @Nullable F fieldValue);
-
- /**
- * Sets the new value of this virtual field if the current value is {@code null}.
- *
- * @return The old field value if it was present, or the result of evaluating passed {@code
- * fieldValueSupplier}.
- */
- public abstract F computeIfNull(T object, Supplier<F> fieldValueSupplier);
}

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java
@@ -9,7 +9,6 @@ import io.opentelemetry.instrumentation.api.caching.Cache;
import io.opentelemetry.instrumentation.api.field.VirtualField;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Supplier;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -71,11 +70,6 @@ public final class RuntimeVirtualFieldSupplier {
cache.put(object, fieldValue);
}
}
-
- @Override
- public F computeIfNull(T object, Supplier<F> fieldValueSupplier) {
- return cache.computeIfAbsent(object, k -> fieldValueSupplier.get());
- }
}
private RuntimeVirtualFieldSupplier() {}

MODIFY
javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/ExecutorAdviceHelper.java
javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/ExecutorAdviceHelper.java
@@ -43,8 +43,17 @@ public final class ExecutorAdviceHelper {
*/
public static <T> PropagatedContext attachContextToTask(
Context context, VirtualField<T, PropagatedContext> virtualField, T task) {
- PropagatedContext propagatedContext =
- virtualField.computeIfNull(task, PropagatedContext.FACTORY);
+
+ // note that this is not an atomic operation and one PropagatedContext may overwrite another if
+ // the task is submitted to >1 executors at roughly the same time; but we're perfectly fine with
+ // that happening - in the event of this happening one of those tasks would lose the original
+ // context anyway
+ PropagatedContext propagatedContext = virtualField.get(task);
+ if (propagatedContext == null) {
+ propagatedContext = new PropagatedContext();
+ virtualField.set(task, propagatedContext);
+ }
+
if (ContextPropagationDebug.isThreadPropagationDebuggerEnabled()) {
context =
ContextPropagationDebug.appendLocations(context, new Exception().getStackTrace(), task);

MODIFY
javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/PropagatedContext.java
javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/PropagatedContext.java
@@ -7,7 +7,6 @@ package io.opentelemetry.javaagent.instrumentation.api.concurrent;
import io.opentelemetry.context.Context;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
-import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -19,13 +18,11 @@ public final class PropagatedContext {
private static final AtomicReferenceFieldUpdater<PropagatedContext, Context> contextUpdater =
AtomicReferenceFieldUpdater.newUpdater(PropagatedContext.class, Context.class, "context");
- static final Supplier<PropagatedContext> FACTORY = PropagatedContext::new;
-
// Used by AtomicReferenceFieldUpdater
@SuppressWarnings("UnusedVariable")
private volatile Context context;
- private PropagatedContext() {}
+ PropagatedContext() {}
void setContext(Context context) {
boolean result = contextUpdater.compareAndSet(this, null, context);

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/VirtualFieldImplementationsGenerator.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/VirtualFieldImplementationsGenerator.java
@@ -15,7 +15,6 @@ import io.opentelemetry.javaagent.tooling.Utils;
import io.opentelemetry.javaagent.tooling.muzzle.VirtualFieldMappings;
import java.util.HashMap;
import java.util.Map;
-import java.util.function.Supplier;
import net.bytebuddy.ByteBuddy;
import net.bytebuddy.ClassFileVersion;
import net.bytebuddy.asm.AsmVisitorWrapper;
@@ -126,9 +125,6 @@ final class VirtualFieldImplementationsGenerator {
} else if ("realPut".equals(name)) {
generateRealPutMethod(name);
return null;
- } else if ("realSynchronizeInstance".equals(name)) {
- generateRealSynchronizeInstanceMethod(name);
- return null;
} else {
return super.visitMethod(access, name, descriptor, signature, exceptions);
}
@@ -247,53 +243,6 @@ final class VirtualFieldImplementationsGenerator {
mv.visitEnd();
}
- /**
- * Provides implementation for {@code realSynchronizeInstance} method that looks like
- * below.
- *
- * <blockquote>
- *
- * <pre>
- * private Object realSynchronizeInstance(final Object key) {
- * if (key instanceof $accessorInterfaceInternalName) {
- * return key;
- * } else {
- * return mapSynchronizeInstance(key);
- * }
- * }
- * </pre>
- *
- * </blockquote>
- *
- * @param name name of the method being visited
- */
- private void generateRealSynchronizeInstanceMethod(String name) {
- MethodVisitor mv = getMethodVisitor(name);
- mv.visitCode();
- mv.visitVarInsn(Opcodes.ALOAD, 1);
- mv.visitTypeInsn(Opcodes.INSTANCEOF, accessorInterfaceInternalName);
- Label elseLabel = new Label();
- mv.visitJumpInsn(Opcodes.IFEQ, elseLabel);
- mv.visitVarInsn(Opcodes.ALOAD, 1);
- mv.visitInsn(Opcodes.ARETURN);
- mv.visitLabel(elseLabel);
- if (frames) {
- mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
- }
- mv.visitVarInsn(Opcodes.ALOAD, 0);
- mv.visitVarInsn(Opcodes.ALOAD, 1);
- mv.visitMethodInsn(
- Opcodes.INVOKESPECIAL,
- instrumentedTypeInternalName,
- "mapSynchronizeInstance",
- Utils.getMethodDefinition(instrumentedType, "mapSynchronizeInstance")
- .getDescriptor(),
- /* isInterface= */ false);
- mv.visitInsn(Opcodes.ARETURN);
- mv.visitMaxs(0, 0);
- mv.visitEnd();
- }
-
private MethodVisitor getMethodVisitor(String methodName) {
return cv.visitMethod(
Opcodes.ACC_PRIVATE,
@@ -328,23 +277,6 @@ final class VirtualFieldImplementationsGenerator {
return realGet(object);
}
- @Override
- public Object computeIfNull(Object object, Supplier<Object> fieldValueSupplier) {
- Object existingContext = realGet(object);
- if (null != existingContext) {
- return existingContext;
- }
- synchronized (realSynchronizeInstance(object)) {
- existingContext = realGet(object);
- if (null != existingContext) {
- return existingContext;
- }
- Object context = fieldValueSupplier.get();
- realPut(object, context);
- return context;
- }
- }
-
@Override
public void set(Object object, Object fieldValue) {
realPut(object, fieldValue);
@@ -359,11 +291,6 @@ final class VirtualFieldImplementationsGenerator {
// to be generated
}
- private Object realSynchronizeInstance(Object key) {
- // to be generated
- return null;
- }
-
private Object mapGet(Object key) {
return map.get(key);
}
@@ -376,10 +303,6 @@ final class VirtualFieldImplementationsGenerator {
}
}
- private Object mapSynchronizeInstance(Object key) {
- return map;
- }
-
public static VirtualField getVirtualField(Class keyClass, Class contextClass) {
// We do not actually check the keyClass here - but that should be fine since compiler would
// check things for us.

MODIFY
testing-common/integration-tests/src/main/java/context/Context.java
testing-common/integration-tests/src/main/java/context/Context.java
package context;
-import java.util.function.Supplier;
-
public class Context {
- public static final Supplier<Context> FACTORY = Context::new;
-
public int count = 0;
}

MODIFY
testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java
testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java
@@ -53,9 +53,16 @@ public class ContextTestInstrumentation implements TypeInstrumentation {
@Advice.OnMethodExit
public static void methodExit(
@Advice.This KeyClass thiz, @Advice.Return(readOnly = false) int contextCount) {
+
VirtualField<KeyClass, Context> virtualField =
VirtualField.find(KeyClass.class, Context.class);
- Context context = virtualField.computeIfNull(thiz, Context.FACTORY);
+
+ Context context = virtualField.get(thiz);
+ if (context == null) {
+ context = new Context();
+ virtualField.set(thiz, context);
+ }
+
contextCount = ++context.count;
}
}

Remove VirtualField#computeIfNull() method (#4354)