Avoid expensive refreshable creation in a potentially hot path (#2274)

Avoid expensive refreshable creation in a potentially hot path

changelog/@unreleased/pr-2274.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Avoid expensive refreshable creation in a potentially hot path
+  links:
+  - https://github.com/palantir/dialogue/pull/2274

dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java

@@ -482,10 +482,15 @@ public String toString() {
         public Supplier<Channel> stickyChannels() {
             return () -> this;
         }
+
+        @Override
+        public EndpointChannel endpoint(Endpoint _endpoint) {
+            return _request -> Futures.immediateFailedFuture(exceptionSupplier.get());
+        }
     }
 
     /* Abstracts away DialogueChannel so that we can handle no-service/no-uri case in #getInternalDialogueChannel. */
-    private interface InternalDialogueChannel extends Channel {
+    private interface InternalDialogueChannel extends Channel, EndpointChannelFactory {
         Supplier<Channel> stickyChannels();
     }
 
@@ -502,6 +507,11 @@ public ListenableFuture<Response> execute(Endpoint endpoint, Request request) {
             return dialogueChannel.execute(endpoint, request);
         }
 
+        @Override
+        public EndpointChannel endpoint(Endpoint endpoint) {
+            return dialogueChannel.endpoint(endpoint);
+        }
+
         @Override
         public Supplier<Channel> stickyChannels() {
             return dialogueChannel.stickyChannels();

dialogue-core/src/main/java/com/palantir/dialogue/core/ContentDecodingChannel.java

@@ -81,7 +81,9 @@ private static BooleanSupplier shouldSendAcceptGzip(Config cf, Endpoint endpoint
         // is handled by the client. Note that this will also opt out of response
         // compression when the target host resolves to multiple IP addresses.
         if (cf.mesh() == MeshMode.DEFAULT_NO_MESH) {
-            return cf.uris().map(targets -> targets.size() == 1)::get;
+            // Avoid using refreshable here, as this can be called very frequently, and
+            // refreshable.map can be expensive.
+            return () -> cf.uris().get().size() == 1;
         }
 
         return () -> false;

dialogue-core/src/main/java/com/palantir/dialogue/core/DialogueChannel.java

@@ -163,12 +163,6 @@ Builder ticker(Ticker value) {
         public DialogueChannel build() {
             Config cf = builder.build();
 
-            // Reloading currently forgets channel state (pinned target, channel scores, concurrency limits, etc...)
-            // In a future change we should attempt to retain this state for channels that are retained between
-            // updates.
-            Refreshable<ImmutableList<LimitedChannel>> channels =
-                    cf.uris().map(targetUris -> createHostChannels(cf, targetUris));
-
             DialogueClientMetrics clientMetrics =
                     DialogueClientMetrics.of(cf.clientConf().taggedMetricRegistry());
 
@@ -178,16 +172,20 @@ public DialogueChannel build() {
                     .clientType("dialogue-channel-non-reloading")
                     .build();
 
-            LimitedChannel nodeSelectionChannel = new SupplierChannel(channels.map(current -> {
+            // Reloading currently forgets channel state (pinned target, channel scores, concurrency limits, etc...)
+            // In a future change we should attempt to retain this state for channels that are retained between
+            // updates.
+            LimitedChannel nodeSelectionChannel = new SupplierChannel(cf.uris().map(targetUris -> {
                 reloadMeter.mark();
                 log.info(
                         "Reloaded channel '{}' targets. (uris: {}, numUris: {}, targets: {}, numTargets: {})",
                         SafeArg.of("channel", cf.channelName()),
                         UnsafeArg.of("uris", cf.clientConf().uris()),
                         SafeArg.of("numUris", cf.clientConf().uris().size()),
-                        UnsafeArg.of("targets", current),
-                        SafeArg.of("numTargets", current.size()));
-                return NodeSelectionStrategyChannel.create(cf, current);
+                        UnsafeArg.of("targets", targetUris),
+                        SafeArg.of("numTargets", targetUris.size()));
+                ImmutableList<LimitedChannel> targetChannels = createHostChannels(cf, targetUris);
+                return NodeSelectionStrategyChannel.create(cf, targetChannels);
             }));
 
             LimitedChannel stickyValidationChannel = new StickyValidationChannel(nodeSelectionChannel);