From ca2935998ac381947e0d60c24daebe06a7930909 Mon Sep 17 00:00:00 2001 From: joecqupt Date: Tue, 10 Nov 2026 09:27:33 +0000 Subject: [PATCH 1/2] xds: Precompute per-route service configs on routing updates --- xds/build.gradle | 2 + .../xds/XdsRouteServiceConfigBenchmark.java | 85 +++++++++++++++++++ .../java/io/grpc/xds/XdsNameResolver.java | 60 +++++++------ .../java/io/grpc/xds/XdsNameResolverTest.java | 12 +++ 4 files changed, 132 insertions(+), 27 deletions(-) create mode 100644 xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java diff --git a/xds/build.gradle b/xds/build.gradle index c6325f7fc2d..1cee83a8a3a 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -6,6 +6,7 @@ plugins { id "com.google.protobuf" id "com.gradleup.shadow" + id "me.champeau.jmh" id "ru.vyarus.animalsniffer" } @@ -45,6 +46,7 @@ dependencies { project(':grpc-stub') compileOnly sourceSets.thirdparty.output testCompileOnly sourceSets.thirdparty.output + jmhCompileOnly sourceSets.thirdparty.output implementation project(':grpc-stub'), project(':grpc-core'), project(':grpc-util'), diff --git a/xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java b/xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java new file mode 100644 index 00000000000..a0f8028393b --- /dev/null +++ b/xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java @@ -0,0 +1,85 @@ +/* + * Copyright 2026 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.xds; + +import com.google.common.collect.ImmutableList; +import com.google.protobuf.util.Durations; +import io.grpc.NameResolver.ServiceConfigParser; +import io.grpc.Status.Code; +import io.grpc.internal.AutoConfiguredLoadBalancerFactory; +import io.grpc.internal.ScParser; +import io.grpc.xds.VirtualHost.Route.RouteAction; +import io.grpc.xds.VirtualHost.Route.RouteAction.RetryPolicy; +import io.grpc.xds.VirtualHost.Route.RouteMatch; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; + +/** + * Compares per-RPC method config generation/parsing with reading the precomputed route config. + * This isolates config handling; it does not measure routing, interceptors, or transport latency. + * Run with JMH's {@code -prof gc} to compare allocation per operation. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +public class XdsRouteServiceConfigBenchmark { + @Param({"EMPTY", "TIMEOUT", "RETRY", "TIMEOUT_RETRY"}) + public String configType; + + private ServiceConfigParser parser; + private Long timeoutNanos; + private RetryPolicy retryPolicy; + private XdsNameResolver.RouteData route; + + @Setup + public void setUp() { + parser = new ScParser(true, 5, 5, new AutoConfiguredLoadBalancerFactory("pick_first")); + timeoutNanos = configType.contains("TIMEOUT") ? TimeUnit.SECONDS.toNanos(5) : null; + retryPolicy = configType.contains("RETRY") + ? RetryPolicy.create(3, ImmutableList.of(Code.UNAVAILABLE), + Durations.fromMillis(100), Durations.fromSeconds(1), null) + : null; + route = new XdsNameResolver.RouteData( + RouteMatch.withPathExactOnly("/service/method"), + RouteAction.forCluster("cluster", ImmutableList.of(), timeoutNanos, retryPolicy, false), + new XdsNameResolver.PassthroughClientInterceptor(), + parser.parseServiceConfig( + XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNanos, retryPolicy))); + if (route.parsedServiceConfig.getError() != null) { + throw new IllegalStateException(route.parsedServiceConfig.getError().toString()); + } + } + + @Benchmark + public Object generateAndParsePerCall() { + return parser.parseServiceConfig( + XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNanos, retryPolicy)) + .getConfig(); + } + + @Benchmark + public Object precomputedConfig() { + return route.parsedServiceConfig.getConfig(); + } +} diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 328995d22da..7fa1d75c71c 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -476,21 +476,7 @@ public Result selectConfig(PickSubchannelArgs args) { } while (!retainCluster(cluster)); final RouteAction routeAction = selectedRoute.routeAction; - Long timeoutNanos = null; - if (enableTimeout) { - timeoutNanos = routeAction.timeoutNano(); - if (timeoutNanos == null) { - timeoutNanos = routingCfg.fallbackTimeoutNano; - } - if (timeoutNanos <= 0) { - timeoutNanos = null; - } - } - RetryPolicy retryPolicy = routeAction.retryPolicy(); - // TODO(chengyuanzhang): avoid service config generation and parsing for each call. - Map rawServiceConfig = - generateServiceConfigWithMethodConfig(timeoutNanos, retryPolicy); - ConfigOrError parsedServiceConfig = serviceConfigParser.parseServiceConfig(rawServiceConfig); + ConfigOrError parsedServiceConfig = selectedRoute.parsedServiceConfig; Object config = parsedServiceConfig.getConfig(); if (config == null) { releaseCluster(cluster); @@ -781,15 +767,31 @@ private void updateRoutes( Map rlsPluginConfigMap = new HashMap<>(); for (Route route : routes) { RouteAction action = route.routeAction(); + ConfigOrError parsedServiceConfig = null; + if (action != null) { + Long timeoutNanos = null; + if (enableTimeout) { + timeoutNanos = action.timeoutNano(); + if (timeoutNanos == null) { + timeoutNanos = httpMaxStreamDurationNano; + } + if (timeoutNanos <= 0) { + timeoutNanos = null; + } + } + // These settings depend only on the routing snapshot, not on the individual RPC. + parsedServiceConfig = serviceConfigParser.parseServiceConfig( + generateServiceConfigWithMethodConfig(timeoutNanos, action.retryPolicy())); + } String prefixedName; if (action == null) { - routesData.add(new RouteData(route.routeMatch(), null, ImmutableList.of())); + routesData.add(new RouteData(route.routeMatch(), null, ImmutableList.of(), null)); } else if (action.cluster() != null) { prefixedName = prefixedClusterName(action.cluster()); clusters.add(prefixedName); clusterNameMap.put(prefixedName, action.cluster()); ClientInterceptor filters = createFilters(filterConfigs, virtualHost, route, null); - routesData.add(new RouteData(route.routeMatch(), route.routeAction(), filters)); + routesData.add(new RouteData(route.routeMatch(), action, filters, parsedServiceConfig)); } else if (action.weightedClusters() != null) { ImmutableList.Builder filterList = ImmutableList.builder(); for (ClusterWeight weightedCluster : action.weightedClusters()) { @@ -799,7 +801,7 @@ private void updateRoutes( filterList.add(createFilters(filterConfigs, virtualHost, route, weightedCluster)); } routesData.add( - new RouteData(route.routeMatch(), route.routeAction(), filterList.build())); + new RouteData(route.routeMatch(), action, filterList.build(), parsedServiceConfig)); } else if (action.namedClusterSpecifierPluginConfig() != null) { PluginConfig pluginConfig = action.namedClusterSpecifierPluginConfig().config(); if (pluginConfig instanceof RlsPluginConfig) { @@ -809,7 +811,7 @@ private void updateRoutes( rlsPluginConfigMap.put(prefixedName, (RlsPluginConfig) pluginConfig); } ClientInterceptor filters = createFilters(filterConfigs, virtualHost, route, null); - routesData.add(new RouteData(route.routeMatch(), route.routeAction(), filters)); + routesData.add(new RouteData(route.routeMatch(), action, filters, parsedServiceConfig)); } else { // Discard route } @@ -864,7 +866,7 @@ private void updateRoutes( } // Make newly added clusters selectable by config selector and deleted clusters no longer // selectable. - routingConfig = new RoutingConfig(xdsConfig, httpMaxStreamDurationNano, routesData.build()); + routingConfig = new RoutingConfig(xdsConfig, routesData.build()); for (String cluster : deletedClusters) { int count = clusterRefs.get(cluster).refCount.decrementAndGet(); if (count == 0) { @@ -955,21 +957,18 @@ private void cleanUpRoutes(Status error) { */ private static class RoutingConfig { final XdsConfig xdsConfig; - final long fallbackTimeoutNano; final ImmutableList routes; final Status errorStatus; private RoutingConfig( - XdsConfig xdsConfig, long fallbackTimeoutNano, ImmutableList routes) { + XdsConfig xdsConfig, ImmutableList routes) { this.xdsConfig = checkNotNull(xdsConfig, "xdsConfig"); - this.fallbackTimeoutNano = fallbackTimeoutNano; this.routes = checkNotNull(routes, "routes"); this.errorStatus = null; } private RoutingConfig(Status errorStatus) { this.xdsConfig = null; - this.fallbackTimeoutNano = 0; this.routes = null; this.errorStatus = checkNotNull(errorStatus, "errorStatus"); checkArgument(!errorStatus.isOk(), "errorStatus should not be okay"); @@ -987,20 +986,27 @@ static final class RouteData { * clusters. */ final ImmutableList filterChoices; + /** Parsed method config shared by RPCs using this route; null for non-forwarding actions. */ + @Nullable + final ConfigOrError parsedServiceConfig; - RouteData(RouteMatch routeMatch, @Nullable RouteAction routeAction, ClientInterceptor filter) { - this(routeMatch, routeAction, ImmutableList.of(filter)); + RouteData(RouteMatch routeMatch, @Nullable RouteAction routeAction, ClientInterceptor filter, + @Nullable ConfigOrError parsedServiceConfig) { + this(routeMatch, routeAction, ImmutableList.of(filter), parsedServiceConfig); } RouteData( RouteMatch routeMatch, @Nullable RouteAction routeAction, - ImmutableList filterChoices) { + ImmutableList filterChoices, + @Nullable ConfigOrError parsedServiceConfig) { this.routeMatch = checkNotNull(routeMatch, "routeMatch"); checkArgument( routeAction == null || !filterChoices.isEmpty(), "filter may be empty only for non-forwarding action"); this.routeAction = routeAction; + this.parsedServiceConfig = routeAction == null ? null + : checkNotNull(parsedServiceConfig, "parsedServiceConfig"); if (routeAction != null && routeAction.weightedClusters() != null) { checkArgument( routeAction.weightedClusters().size() == filterChoices.size(), diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index e57d0860588..eb307483720 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -822,6 +822,18 @@ public void resolved_simpleCallSucceeds() { verifyNoMoreInteractions(mockListener); } + @Test + public void resolved_configReused() { + InternalConfigSelector selector = resolveToClusters(); + Result first = selector.selectConfig( + newPickSubchannelArgs(call1.methodDescriptor, new Metadata(), CallOptions.DEFAULT)); + Result second = selector.selectConfig( + newPickSubchannelArgs(call1.methodDescriptor, new Metadata(), CallOptions.DEFAULT)); + assertThat(first.getStatus().isOk()).isTrue(); + assertThat(second.getStatus().isOk()).isTrue(); + assertThat(second.getConfig()).isSameInstanceAs(first.getConfig()); + } + @Test public void resolved_simpleCallFailedToRoute_noMatchingRoute() { InternalConfigSelector configSelector = resolveToClusters(); From 270974008ca3c4dc4f31c4c1bbf0c35e3babaf81 Mon Sep 17 00:00:00 2001 From: joecqupt Date: Wed, 11 Nov 2026 13:30:54 +0000 Subject: [PATCH 2/2] xds: Remove standalone route config benchmark Keep the change focused on route config precomputation and its unit test. Preserve benchmark source and results in the PR discussion instead of adding JMH build configuration to the xds module. --- xds/build.gradle | 2 - .../xds/XdsRouteServiceConfigBenchmark.java | 85 ------------------- 2 files changed, 87 deletions(-) delete mode 100644 xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java diff --git a/xds/build.gradle b/xds/build.gradle index 1cee83a8a3a..c6325f7fc2d 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -6,7 +6,6 @@ plugins { id "com.google.protobuf" id "com.gradleup.shadow" - id "me.champeau.jmh" id "ru.vyarus.animalsniffer" } @@ -46,7 +45,6 @@ dependencies { project(':grpc-stub') compileOnly sourceSets.thirdparty.output testCompileOnly sourceSets.thirdparty.output - jmhCompileOnly sourceSets.thirdparty.output implementation project(':grpc-stub'), project(':grpc-core'), project(':grpc-util'), diff --git a/xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java b/xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java deleted file mode 100644 index a0f8028393b..00000000000 --- a/xds/src/jmh/java/io/grpc/xds/XdsRouteServiceConfigBenchmark.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright 2026 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc.xds; - -import com.google.common.collect.ImmutableList; -import com.google.protobuf.util.Durations; -import io.grpc.NameResolver.ServiceConfigParser; -import io.grpc.Status.Code; -import io.grpc.internal.AutoConfiguredLoadBalancerFactory; -import io.grpc.internal.ScParser; -import io.grpc.xds.VirtualHost.Route.RouteAction; -import io.grpc.xds.VirtualHost.Route.RouteAction.RetryPolicy; -import io.grpc.xds.VirtualHost.Route.RouteMatch; -import java.util.concurrent.TimeUnit; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Param; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.Setup; -import org.openjdk.jmh.annotations.State; - -/** - * Compares per-RPC method config generation/parsing with reading the precomputed route config. - * This isolates config handling; it does not measure routing, interceptors, or transport latency. - * Run with JMH's {@code -prof gc} to compare allocation per operation. - */ -@State(Scope.Thread) -@BenchmarkMode(Mode.AverageTime) -@OutputTimeUnit(TimeUnit.NANOSECONDS) -public class XdsRouteServiceConfigBenchmark { - @Param({"EMPTY", "TIMEOUT", "RETRY", "TIMEOUT_RETRY"}) - public String configType; - - private ServiceConfigParser parser; - private Long timeoutNanos; - private RetryPolicy retryPolicy; - private XdsNameResolver.RouteData route; - - @Setup - public void setUp() { - parser = new ScParser(true, 5, 5, new AutoConfiguredLoadBalancerFactory("pick_first")); - timeoutNanos = configType.contains("TIMEOUT") ? TimeUnit.SECONDS.toNanos(5) : null; - retryPolicy = configType.contains("RETRY") - ? RetryPolicy.create(3, ImmutableList.of(Code.UNAVAILABLE), - Durations.fromMillis(100), Durations.fromSeconds(1), null) - : null; - route = new XdsNameResolver.RouteData( - RouteMatch.withPathExactOnly("/service/method"), - RouteAction.forCluster("cluster", ImmutableList.of(), timeoutNanos, retryPolicy, false), - new XdsNameResolver.PassthroughClientInterceptor(), - parser.parseServiceConfig( - XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNanos, retryPolicy))); - if (route.parsedServiceConfig.getError() != null) { - throw new IllegalStateException(route.parsedServiceConfig.getError().toString()); - } - } - - @Benchmark - public Object generateAndParsePerCall() { - return parser.parseServiceConfig( - XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNanos, retryPolicy)) - .getConfig(); - } - - @Benchmark - public Object precomputedConfig() { - return route.parsedServiceConfig.getConfig(); - } -}