-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid expensive refreshable creation in a potentially hot path #2274
Conversation
Generate changelog in
|
198c606
to
f7174ec
Compare
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to create a refreshable.map target for each endpoint. We could extract a refreshable.map to a higher layer, but this is probably simpler (list.size is just a field read)
UnsafeArg.of("targets", targetUris), | ||
SafeArg.of("numTargets", targetUris.size())); | ||
ImmutableList<LimitedChannel> targetChannels = createHostChannels(cf, targetUris); | ||
return NodeSelectionStrategyChannel.create(cf, targetChannels); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combined two refreshable.map calls since we're really doing one thing here, should be easier to follow
} | ||
|
||
/* 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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the most important change, we have a lot of code which attempts to cast Channel to EndpointChannelFactory and use endpoint(endpoint)
on the result, failing that it will call the Channel.execute(Endpoint,Request)
method, which internally delegates to the preferred endpoint
method. Unfortunately that ends up re-computing a great deal of info on each invocation.
This cruft exists to avoid breaking dialogue API :-/
👍 |
Released 3.131.0 |
==COMMIT_MSG==
Avoid expensive refreshable creation in a potentially hot path
==COMMIT_MSG==