Handle a fragment in a client-side URI properly

benchmarks/jmh/src/jmh/java/com/linecorp/armeria/internal/common/RequestTargetBenchmark.java

@@ -0,0 +1,119 @@
+/*
+ * Copyright 2017 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you 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:
+ *
+ *   https://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 com.linecorp.armeria.internal.common;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.infra.Blackhole;
+
+import com.linecorp.armeria.common.RequestTarget;
+
+/**
+ * Microbenchmarks for {@link RequestTarget}.
+ */
+@State(Scope.Thread)
+public class RequestTargetBenchmark {
+
+    private static final String NO_CACHE_JVM_OPTS = "-Dcom.linecorp.armeria.parsedPathCacheSpec=off";
+
+    private String path1;
+    private String path2;
+
+    @Setup(Level.Invocation)
+    @SuppressWarnings("StringOperationCanBeSimplified")
+    public void setUp() {
+        // Create a new String for paths every time to avoid constant folding.
+        path1 = new String("/armeria/services/hello-world");
+        path2 = new String("/armeria/services/goodbye-world");
+    }
+
+    @Benchmark
+    public RequestTarget serverCached() {
+        return doServer();
+    }
+
+    @Benchmark
+    @Fork(jvmArgsAppend = NO_CACHE_JVM_OPTS)
+    public RequestTarget serverUncached() {
+        return doServer();
+    }
+
+    private RequestTarget doServer() {
+        final RequestTarget parsed = RequestTarget.forServer(path1);
+        RequestTargetCache.putForServer(path1, parsed);
+        return parsed;
+    }
+
+    @Benchmark
+    public RequestTarget serverCachedAndUncached(Blackhole bh) {
+        return doServerCachedAndUncached(bh);
+    }
+
+    @Benchmark
+    @Fork(jvmArgsAppend = NO_CACHE_JVM_OPTS)
+    public RequestTarget serverUncachedAndUncached(Blackhole bh) {
+        return doServerCachedAndUncached(bh);
+    }
+
+    private RequestTarget doServerCachedAndUncached(Blackhole bh) {
+        final RequestTarget parsed = RequestTarget.forServer(path1);
+        RequestTargetCache.putForServer(path1, parsed);
+        final RequestTarget parsed2 = RequestTarget.forServer(path2);
+        bh.consume(parsed2);
+        return parsed;
+    }
+
+    @Benchmark
+    public RequestTarget clientCached() {
+        return doServer();
+    }
+
+    @Benchmark
+    @Fork(jvmArgsAppend = NO_CACHE_JVM_OPTS)
+    public RequestTarget clientUncached() {
+        return doClient();
+    }
+
+    private RequestTarget doClient() {
+        final RequestTarget parsed = RequestTarget.forClient(path1);
+        RequestTargetCache.putForClient(path1, parsed);
+        return parsed;
+    }
+
+    @Benchmark
+    public RequestTarget clientCachedAndUncached(Blackhole bh) {
+        return doClientCachedAndUncached(bh);
+    }
+
+    @Benchmark
+    @Fork(jvmArgsAppend = NO_CACHE_JVM_OPTS)
+    public RequestTarget clientUncachedAndUncached(Blackhole bh) {
+        return doClientCachedAndUncached(bh);
+    }
+
+    private RequestTarget doClientCachedAndUncached(Blackhole bh) {
+        final RequestTarget parsed = RequestTarget.forClient(path1);
+        RequestTargetCache.putForClient(path1, parsed);
+        final RequestTarget parsed2 = RequestTarget.forClient(path2);
+        bh.consume(parsed2);
+        return parsed;
+    }
+}

benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java

@@ -32,6 +32,7 @@
 import com.linecorp.armeria.common.HttpStatus;
 import com.linecorp.armeria.common.RequestHeaders;
 import com.linecorp.armeria.common.RequestId;
+import com.linecorp.armeria.common.RequestTarget;
 import com.linecorp.armeria.common.SuccessFunction;
 import com.linecorp.armeria.server.logging.AccessLogWriter;
 
@@ -48,6 +49,8 @@ public class RoutersBenchmark {
     private static final RequestHeaders METHOD1_HEADERS =
             RequestHeaders.of(HttpMethod.POST, "/grpc.package.Service/Method1");
 
+    private static final RequestTarget METHOD1_REQ_TARGET = RequestTarget.forServer(METHOD1_HEADERS.path());
+
     static {
         final String defaultLogName = null;
         final String defaultServiceName = null;
@@ -61,32 +64,32 @@ public class RoutersBenchmark {
                                   SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
                                   false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
                                   SuccessFunction.always(), multipartUploadsLocation, ImmutableList.of(),
-                                  HttpHeaders.of(), (ctx) -> RequestId.random(), serviceErrorHandler),
+                                  HttpHeaders.of(), ctx -> RequestId.random(), serviceErrorHandler),
                 new ServiceConfig(route2, route2,
                                   SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
                                   false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
                                   SuccessFunction.always(), multipartUploadsLocation, ImmutableList.of(),
-                                  HttpHeaders.of(), (ctx) -> RequestId.random(), serviceErrorHandler));
+                                  HttpHeaders.of(), ctx -> RequestId.random(), serviceErrorHandler));
         FALLBACK_SERVICE = new ServiceConfig(Route.ofCatchAll(), Route.ofCatchAll(), SERVICE,
                                              defaultLogName, defaultServiceName,
                                              defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(),
                                              CommonPools.blockingTaskExecutor(),
                                              SuccessFunction.always(), multipartUploadsLocation,
-                                             ImmutableList.of(), HttpHeaders.of(), (ctx) -> RequestId.random(),
+                                             ImmutableList.of(), HttpHeaders.of(), ctx -> RequestId.random(),
                                              serviceErrorHandler);
         HOST = new VirtualHost(
                 "localhost", "localhost", 0, null, SERVICES, FALLBACK_SERVICE, RejectedRouteHandler.DISABLED,
                 unused -> NOPLogger.NOP_LOGGER, defaultServiceNaming, 0, 0, false,
                 AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), multipartUploadsLocation,
                 ImmutableList.of(),
-                (ctx) -> RequestId.random());
+                ctx -> RequestId.random());
         ROUTER = Routers.ofVirtualHost(HOST, SERVICES, RejectedRouteHandler.DISABLED);
     }
 
     @Benchmark
     public Routed<ServiceConfig> exactMatch() {
-        final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_HEADERS.path(),
-                                                            null, METHOD1_HEADERS, RoutingStatus.OK);
+        final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET,
+                                                            METHOD1_HEADERS, RoutingStatus.OK);
         final Routed<ServiceConfig> routed = ROUTER.find(ctx);
         if (routed.value() != SERVICES.get(0)) {
             throw new IllegalStateException("Routing error");
@@ -97,8 +100,8 @@ public Routed<ServiceConfig> exactMatch() {
     @Benchmark
     public Routed<ServiceConfig> exactMatch_wrapped() {
         final RoutingContext ctx = new RoutingContextWrapper(
-                DefaultRoutingContext.of(HOST, "localhost", METHOD1_HEADERS.path(),
-                                         null, METHOD1_HEADERS, RoutingStatus.OK));
+                DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET,
+                                         METHOD1_HEADERS, RoutingStatus.OK));
         final Routed<ServiceConfig> routed = ROUTER.find(ctx);
         if (routed.value() != SERVICES.get(0)) {
             throw new IllegalStateException("Routing error");

core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java

@@ -70,8 +70,6 @@ public void run(Throwable cause) { /* no-op */ }
         noopResponseCancellationScheduler.finishNow();
     }
 
-    @Nullable
-    private final String fragment;
     @Nullable
     private Endpoint endpoint;
     private ClientOptions options = ClientOptions.of();
@@ -81,12 +79,10 @@ public void run(Throwable cause) { /* no-op */ }
 
     ClientRequestContextBuilder(HttpRequest request) {
         super(false, request);
-        fragment = null;
     }
 
     ClientRequestContextBuilder(RpcRequest request, URI uri) {
         super(false, request, uri);
-        fragment = uri.getRawFragment();
     }
 
     @Override
@@ -157,7 +153,7 @@ public ClientRequestContext build() {
 
         final DefaultClientRequestContext ctx = new DefaultClientRequestContext(
                 eventLoop(), meterRegistry(), sessionProtocol(),
-                id(), method(), path(), query(), fragment, options, request(), rpcRequest(),
+                id(), method(), requestTarget(), options, request(), rpcRequest(),
                 requestOptions, responseCancellationScheduler,
                 isRequestStartTimeSet() ? requestStartTimeNanos() : System.nanoTime(),
                 isRequestStartTimeSet() ? requestStartTimeMicros() : SystemInfo.currentTimeMicros());