Avoid expensive refreshable creation in a potentially hot path

palantir · May 6, 2024 · 9536ba6 · 9536ba6
1 parent 24409ae
commit 9536ba6
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/dialogue-core/src/main/java/com/palantir/dialogue/core/ContentDecodingChannel.java b/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;

diff --git a/dialogue-core/src/main/java/com/palantir/dialogue/core/DialogueChannel.java b/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);