diff --git a/changelog/@unreleased/pr-2274.v2.yml b/changelog/@unreleased/pr-2274.v2.yml new file mode 100644 index 000000000..4a9d3e06c --- /dev/null +++ b/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 diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java index 3eace7cb3..be1497d10 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java @@ -482,10 +482,15 @@ public String toString() { public Supplier 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 stickyChannels(); } @@ -502,6 +507,11 @@ public ListenableFuture execute(Endpoint endpoint, Request request) { return dialogueChannel.execute(endpoint, request); } + @Override + public EndpointChannel endpoint(Endpoint endpoint) { + return dialogueChannel.endpoint(endpoint); + } + @Override public Supplier stickyChannels() { return dialogueChannel.stickyChannels(); 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 index 90262c75a..fd386d5e9 100644 --- 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 index 6b7a59057..a79c08cc8 100644 --- 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> 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 targetChannels = createHostChannels(cf, targetUris); + return NodeSelectionStrategyChannel.create(cf, targetChannels); })); LimitedChannel stickyValidationChannel = new StickyValidationChannel(nodeSelectionChannel);