This is an automated email from the ASF dual-hosted git repository. wusheng pushed a commit to branch fix/companion-class-closure-wiring in repository https://gitbox.apache.org/repos/asf/skywalking-graalvm-distro.git
commit 78f144f017fbe15eeb14c805e65e2508cdd21c0c Author: Wu Sheng <[email protected]> AuthorDate: Thu Mar 19 20:03:42 2026 +0800 Remove LambdaMetafactory closure wiring after upstream companion class fix Upstream PR #13750 replaced LambdaMetafactory-based closure wiring with companion classes that self-initialize via static {} blocks. This removes the now-dead wireClosures() code from the distro runtime DSL and tests, bumps the submodule, and re-enables the previously skipped meter_oap_instance_metrics_aggregation e2e test. --- oap-graalvm-server/src/test/CLAUDE.md | 7 +- .../graalvm/PrecompiledMALExecutionTest.java | 65 --------------- .../graalvm/mal/MALScriptComparisonBase.java | 76 ----------------- oap-libs-for-graalvm/CLAUDE.md | 2 +- .../skywalking/oap/meter/analyzer/v2/dsl/DSL.java | 94 ++-------------------- skywalking | 2 +- test/e2e/cases/so11y/so11y-cases.yaml | 3 +- 7 files changed, 13 insertions(+), 236 deletions(-) diff --git a/oap-graalvm-server/src/test/CLAUDE.md b/oap-graalvm-server/src/test/CLAUDE.md index 24d488a..ab8526b 100644 --- a/oap-graalvm-server/src/test/CLAUDE.md +++ b/oap-graalvm-server/src/test/CLAUDE.md @@ -34,9 +34,10 @@ Each metric expression is run through two independent paths: **Path B (Pre-compiled)**: Loads the `.class` file from per-file configs under `META-INF/mal-v2/` (mirroring original YAML directory structure) via -`Class.forName()` using expression text as lookup key, then wires closure -fields (TagFunction, ForEachFunction, PropertiesExtractor, DecorateFunction) -via `LambdaMetafactory`. +`Class.forName()` using expression text as lookup key. Closure fields +(TagFunction, ForEachFunction, PropertiesExtractor, DecorateFunction) are +self-wired via companion classes in the `static {}` initializer — no external +`LambdaMetafactory` wiring needed. If both paths produce identical `Result` (same success flag, same sample values within 0.001 tolerance, same labels), then the build-time compilation is diff --git a/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/PrecompiledMALExecutionTest.java b/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/PrecompiledMALExecutionTest.java index 7e5671d..a44aaf2 100644 --- a/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/PrecompiledMALExecutionTest.java +++ b/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/PrecompiledMALExecutionTest.java @@ -21,12 +21,6 @@ import com.google.common.collect.ImmutableMap; import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; -import java.lang.invoke.CallSite; -import java.lang.invoke.LambdaMetafactory; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; @@ -37,7 +31,6 @@ import org.apache.skywalking.oap.meter.analyzer.v2.dsl.Result; import org.apache.skywalking.oap.meter.analyzer.v2.dsl.Sample; import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamily; import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamilyBuilder; -import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamilyFunctions; import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity; import org.apache.skywalking.oap.server.core.config.NamingControl; import org.apache.skywalking.oap.server.core.config.group.EndpointNameGrouping; @@ -190,67 +183,9 @@ class PrecompiledMALExecutionTest { Class<?> exprClass = Class.forName(className); MalExpression malExpr = (MalExpression) exprClass.getDeclaredConstructor().newInstance(); - wireClosures(exprClass, malExpr); return new Expression(metricName, expression, malExpr); } - private static final Map<String, ClosureInfo> CLOSURE_TYPES = new HashMap<>(); - - private record ClosureInfo(Class<?> interfaceClass, String samName, - MethodType samType, MethodType instantiatedType, - MethodType methodType) { - } - - static { - CLOSURE_TYPES.put( - SampleFamilyFunctions.TagFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.TagFunction.class, "apply", - MethodType.methodType(Object.class, Object.class), - MethodType.methodType(Map.class, Map.class), - MethodType.methodType(Map.class, Map.class))); - CLOSURE_TYPES.put( - SampleFamilyFunctions.PropertiesExtractor.class.getName(), - new ClosureInfo(SampleFamilyFunctions.PropertiesExtractor.class, "apply", - MethodType.methodType(Object.class, Object.class), - MethodType.methodType(Map.class, Map.class), - MethodType.methodType(Map.class, Map.class))); - CLOSURE_TYPES.put( - SampleFamilyFunctions.ForEachFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.ForEachFunction.class, "accept", - MethodType.methodType(void.class, String.class, Map.class), - MethodType.methodType(void.class, String.class, Map.class), - MethodType.methodType(void.class, String.class, Map.class))); - CLOSURE_TYPES.put( - SampleFamilyFunctions.DecorateFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.DecorateFunction.class, "accept", - MethodType.methodType(void.class, Object.class), - MethodType.methodType(void.class, Object.class), - MethodType.methodType(void.class, Object.class))); - } - - private static void wireClosures(final Class<?> clazz, final Object instance) throws Exception { - try { - MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(clazz, MethodHandles.lookup()); - for (Field field : clazz.getFields()) { - ClosureInfo info = CLOSURE_TYPES.get(field.getType().getName()); - if (info == null) { - continue; - } - String methodName = field.getName() + "_" + info.samName; - MethodHandle mh = lookup.findVirtual(clazz, methodName, info.methodType); - CallSite site = LambdaMetafactory.metafactory( - lookup, info.samName, - MethodType.methodType(info.interfaceClass, clazz), - info.samType, mh, info.instantiatedType); - field.set(instance, site.getTarget().invoke(instance)); - } - } catch (Exception e) { - throw e; - } catch (Throwable t) { - throw new RuntimeException("Failed to wire closures for " + clazz.getName(), t); - } - } - private static Map<String, String> loadExpressionMap() throws Exception { Map<String, String> map = new HashMap<>(); ClassLoader cl = PrecompiledMALExecutionTest.class.getClassLoader(); diff --git a/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/mal/MALScriptComparisonBase.java b/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/mal/MALScriptComparisonBase.java index 924c581..88b189d 100644 --- a/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/mal/MALScriptComparisonBase.java +++ b/oap-graalvm-server/src/test/java/org/apache/skywalking/oap/server/graalvm/mal/MALScriptComparisonBase.java @@ -22,12 +22,6 @@ import com.google.common.collect.ImmutableMap; import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; -import java.lang.invoke.CallSite; -import java.lang.invoke.LambdaMetafactory; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.ArrayList; @@ -48,7 +42,6 @@ import org.apache.skywalking.oap.meter.analyzer.v2.dsl.Result; import org.apache.skywalking.oap.meter.analyzer.v2.dsl.Sample; import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamily; import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamilyBuilder; -import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamilyFunctions; import org.apache.skywalking.oap.meter.analyzer.v2.prometheus.rule.MetricsRule; import org.apache.skywalking.oap.meter.analyzer.v2.prometheus.rule.Rule; import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity; @@ -288,7 +281,6 @@ abstract class MALScriptComparisonBase { Class<?> exprClass = Class.forName(className); MalExpression malExpr = (MalExpression) exprClass.getDeclaredConstructor().newInstance(); - wireClosures(exprClass, malExpr); return new Expression(metricName, expression, malExpr); } catch (Exception e) { throw new AssertionError( @@ -297,74 +289,6 @@ abstract class MALScriptComparisonBase { } } - // --------------------------------------------------------------- - // Closure wiring for pre-compiled classes - // --------------------------------------------------------------- - - private record ClosureInfo(Class<?> interfaceClass, String samName, - MethodType samType, MethodType instantiatedType, - MethodType methodType) { - } - - private static final Map<String, ClosureInfo> CLOSURE_TYPES = new HashMap<>(); - - static { - CLOSURE_TYPES.put( - SampleFamilyFunctions.TagFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.TagFunction.class, "apply", - MethodType.methodType(Object.class, Object.class), - MethodType.methodType(Map.class, Map.class), - MethodType.methodType(Map.class, Map.class))); - - CLOSURE_TYPES.put( - SampleFamilyFunctions.PropertiesExtractor.class.getName(), - new ClosureInfo(SampleFamilyFunctions.PropertiesExtractor.class, "apply", - MethodType.methodType(Object.class, Object.class), - MethodType.methodType(Map.class, Map.class), - MethodType.methodType(Map.class, Map.class))); - - CLOSURE_TYPES.put( - SampleFamilyFunctions.ForEachFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.ForEachFunction.class, "accept", - MethodType.methodType(void.class, String.class, Map.class), - MethodType.methodType(void.class, String.class, Map.class), - MethodType.methodType(void.class, String.class, Map.class))); - - CLOSURE_TYPES.put( - SampleFamilyFunctions.DecorateFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.DecorateFunction.class, "accept", - MethodType.methodType(void.class, Object.class), - MethodType.methodType(void.class, Object.class), - MethodType.methodType(void.class, Object.class))); - } - - private static void wireClosures(final Class<?> clazz, final Object instance) { - try { - final MethodHandles.Lookup lookup = - MethodHandles.privateLookupIn(clazz, MethodHandles.lookup()); - - for (final Field field : clazz.getFields()) { - final ClosureInfo info = CLOSURE_TYPES.get(field.getType().getName()); - if (info == null) { - continue; - } - final String methodName = field.getName() + "_" + info.samName; - final MethodHandle mh = lookup.findVirtual( - clazz, methodName, info.methodType); - final CallSite site = LambdaMetafactory.metafactory( - lookup, - info.samName, - MethodType.methodType(info.interfaceClass, clazz), - info.samType, - mh, - info.instantiatedType); - field.set(instance, site.getTarget().invoke(instance)); - } - } catch (Throwable e) { - throw new AssertionError("Failed to wire closures for " + clazz.getName(), e); - } - } - // --------------------------------------------------------------- // Input builders // --------------------------------------------------------------- diff --git a/oap-libs-for-graalvm/CLAUDE.md b/oap-libs-for-graalvm/CLAUDE.md index fdc5cce..7f0c7ae 100644 --- a/oap-libs-for-graalvm/CLAUDE.md +++ b/oap-libs-for-graalvm/CLAUDE.md @@ -58,7 +58,7 @@ JAVA_HOME=/Users/wusheng/.sdkman/candidates/java/25-graal make build-distro | Replacement Class | Upstream Source | Change | Staleness Tracked | |---|---|---|---| -| `DSL` | `meter-analyzer/.../v2/dsl/DSL.java` | Load pre-compiled `MalExpression` from per-file configs (`META-INF/mal-v2/`); look up by expression text; wires closure fields via `LambdaMetafactory` after instantiation | No | +| `DSL` | `meter-analyzer/.../v2/dsl/DSL.java` | Load pre-compiled `MalExpression` from per-file configs (`META-INF/mal-v2/`); look up by expression text; closure fields are self-wired by companion classes in the static initializer (no `LambdaMetafactory`) | No | | `FilterExpression` | `meter-analyzer/.../v2/dsl/FilterExpression.java` | Load pre-compiled `MalFilter` from v2 manifest | No | ### log-analyzer-for-graalvm (1 class) diff --git a/oap-libs-for-graalvm/meter-analyzer-for-graalvm/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/dsl/DSL.java b/oap-libs-for-graalvm/meter-analyzer-for-graalvm/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/dsl/DSL.java index 4cd9880..4421acb 100644 --- a/oap-libs-for-graalvm/meter-analyzer-for-graalvm/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/dsl/DSL.java +++ b/oap-libs-for-graalvm/meter-analyzer-for-graalvm/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/dsl/DSL.java @@ -21,12 +21,6 @@ import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.lang.invoke.CallSite; -import java.lang.invoke.LambdaMetafactory; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; @@ -48,6 +42,11 @@ import lombok.extern.slf4j.Slf4j; * * <p>At runtime, all per-file configs are loaded once and indexed by * expression text for O(1) lookup in {@link #parse(String, String, String)}. + * + * <p>Closure fields (TagFunction, ForEachFunction, etc.) are now self-wired + * via companion classes generated at build time. The main class static + * initializer instantiates each companion class directly — no external + * LambdaMetafactory wiring is needed. */ @Slf4j public final class DSL { @@ -76,7 +75,6 @@ public final class DSL { try { final Class<?> exprClass = Class.forName(className); final MalExpression malExpr = (MalExpression) exprClass.getDeclaredConstructor().newInstance(); - wireClosures(exprClass, malExpr); final int count = LOADED_COUNT.incrementAndGet(); log.debug("Loaded pre-compiled MAL expression [{}/{}]: {} -> {}", count, EXPRESSION_MAP.size(), metricName, className); @@ -179,86 +177,4 @@ public final class DSL { log.warn("Failed to load MAL v2 per-file config: {}", configPath, e); } } - - /** - * Closure type metadata for LambdaMetafactory wiring. - * Maps functional interface type name to SAM method info. - */ - private static final Map<String, ClosureInfo> CLOSURE_TYPES = new HashMap<>(); - - static { - // TagFunction extends Function<Map, Map> - CLOSURE_TYPES.put( - SampleFamilyFunctions.TagFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.TagFunction.class, "apply", - MethodType.methodType(Object.class, Object.class), - MethodType.methodType(Map.class, Map.class), - MethodType.methodType(Map.class, Map.class))); - - // PropertiesExtractor extends Function<Map, Map> - CLOSURE_TYPES.put( - SampleFamilyFunctions.PropertiesExtractor.class.getName(), - new ClosureInfo(SampleFamilyFunctions.PropertiesExtractor.class, "apply", - MethodType.methodType(Object.class, Object.class), - MethodType.methodType(Map.class, Map.class), - MethodType.methodType(Map.class, Map.class))); - - // ForEachFunction — not generic, SAM = instantiated - CLOSURE_TYPES.put( - SampleFamilyFunctions.ForEachFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.ForEachFunction.class, "accept", - MethodType.methodType(void.class, String.class, Map.class), - MethodType.methodType(void.class, String.class, Map.class), - MethodType.methodType(void.class, String.class, Map.class))); - - // DecorateFunction extends Consumer<MeterEntity> - CLOSURE_TYPES.put( - SampleFamilyFunctions.DecorateFunction.class.getName(), - new ClosureInfo(SampleFamilyFunctions.DecorateFunction.class, "accept", - MethodType.methodType(void.class, Object.class), - MethodType.methodType(void.class, Object.class), - MethodType.methodType(void.class, Object.class))); - } - - private record ClosureInfo(Class<?> interfaceClass, String samName, - MethodType samType, MethodType instantiatedType, - MethodType methodType) { - } - - /** - * Wire closure fields on a pre-compiled MalExpression instance. - * - * <p>Generated classes have public fields typed as functional interfaces - * (TagFunction, ForEachFunction, etc.) with corresponding methods that - * implement the closure body. MALClassGenerator normally wires these via - * LambdaMetafactory after compilation. When loading from the manifest JAR, - * we replicate that wiring here. - */ - static void wireClosures(final Class<?> clazz, - final Object instance) { - try { - final MethodHandles.Lookup lookup = - MethodHandles.privateLookupIn(clazz, MethodHandles.lookup()); - - for (final Field field : clazz.getFields()) { - final ClosureInfo info = CLOSURE_TYPES.get(field.getType().getName()); - if (info == null) { - continue; - } - final String methodName = field.getName() + "_" + info.samName; - final MethodHandle mh = lookup.findVirtual( - clazz, methodName, info.methodType); - final CallSite site = LambdaMetafactory.metafactory( - lookup, - info.samName, - MethodType.methodType(info.interfaceClass, clazz), - info.samType, - mh, - info.instantiatedType); - field.set(instance, site.getTarget().invoke(instance)); - } - } catch (Throwable e) { - log.warn("Failed to wire closures for {}", clazz.getName(), e); - } - } } diff --git a/skywalking b/skywalking index 64a1795..726ebcc 160000 --- a/skywalking +++ b/skywalking @@ -1 +1 @@ -Subproject commit 64a1795d8a582f2216f47bfe572b3ab649733c01 +Subproject commit 726ebcc321dbd3c258963fc4bc23d320b903f6d9 diff --git a/test/e2e/cases/so11y/so11y-cases.yaml b/test/e2e/cases/so11y/so11y-cases.yaml index e1fd1c3..17c350e 100644 --- a/test/e2e/cases/so11y/so11y-cases.yaml +++ b/test/e2e/cases/so11y/so11y-cases.yaml @@ -29,7 +29,8 @@ # OAP application-level metrics (available in native image) - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression=meter_oap_instance_trace_count --instance-name=http://localhost:1234 --service-name=oap-server expected: ../../../../skywalking/test/e2e-v2/cases/so11y/expected/metrics-has-value-label-trace.yml - # meter_oap_instance_metrics_aggregation skipped: .tag() closure NPE in pre-compiled MAL + - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression=meter_oap_instance_metrics_aggregation --instance-name=http://localhost:1234 --service-name=oap-server + expected: ../../../../skywalking/test/e2e-v2/cases/so11y/expected/metrics-has-value.yml - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression=meter_oap_instance_persistence_prepare_count --instance-name=http://localhost:1234 --service-name=oap-server expected: ../../../../skywalking/test/e2e-v2/cases/so11y/expected/metrics-has-value.yml - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression=meter_oap_instance_persistence_execute_count --instance-name=http://localhost:1234 --service-name=oap-server
