From 631a19f901b6567c37cbc5a01985f3d786297704 Mon Sep 17 00:00:00 2001 From: PapaCharlie Date: Mon, 27 Nov 2023 14:01:07 -0500 Subject: [PATCH 1/2] Remove usage of Optional from SimpleLoadBalancer According to benchmark results, this is creating significant garbage. Because this is not exposed as an external API, directly returning null instead of an Optional is safe and will be significantly less expensive. --- .../balancer/simple/SimpleLoadBalancer.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/balancer/simple/SimpleLoadBalancer.java b/d2/src/main/java/com/linkedin/d2/balancer/simple/SimpleLoadBalancer.java index ca7d6e2219..7a8f7d4dc8 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/simple/SimpleLoadBalancer.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/simple/SimpleLoadBalancer.java @@ -41,9 +41,9 @@ import com.linkedin.d2.balancer.properties.UriProperties; import com.linkedin.d2.balancer.strategies.LoadBalancerStrategy; import com.linkedin.d2.balancer.subsetting.SubsettingState; -import com.linkedin.d2.balancer.util.CustomAffinityRoutingURIProvider; import com.linkedin.d2.balancer.util.ClientFactoryProvider; import com.linkedin.d2.balancer.util.ClusterInfoProvider; +import com.linkedin.d2.balancer.util.CustomAffinityRoutingURIProvider; import com.linkedin.d2.balancer.util.HostOverrideList; import com.linkedin.d2.balancer.util.HostToKeyMapper; import com.linkedin.d2.balancer.util.KeysAndHosts; @@ -72,7 +72,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Random; import java.util.Set; import java.util.TreeMap; @@ -88,9 +87,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static com.linkedin.d2.discovery.util.LogUtil.debug; -import static com.linkedin.d2.discovery.util.LogUtil.info; -import static com.linkedin.d2.discovery.util.LogUtil.warn; +import static com.linkedin.d2.discovery.util.LogUtil.*; public class SimpleLoadBalancer implements LoadBalancer, HashRingProvider, ClientFactoryProvider, PartitionInfoProvider, WarmUpService, ClusterInfoProvider @@ -949,10 +946,10 @@ private Map getPotentialClientsSubsetting(String serviceName { possibleTrackerClient.setSubsetWeight(partitionId, weightedSubset.get(possibleUri)); } - return Optional.of(possibleTrackerClient); + return possibleTrackerClient; } } - return Optional.empty(); + return null; }); } @@ -961,13 +958,13 @@ private Map getPotentialClientsNotSubsetting(String serviceN ClusterProperties clusterProperties, Set possibleUris) { return getPotentialClients(serviceProperties, clusterProperties, possibleUris, - possibleUri -> Optional.ofNullable(_state.getClient(serviceName, possibleUri))); + possibleUri -> _state.getClient(serviceName, possibleUri)); } private Map getPotentialClients(ServiceProperties serviceProperties, ClusterProperties clusterProperties, Set possibleUris, - Function> trackerClientFinder) + Function trackerClientFinder) { Map clientsToLoadBalance = new HashMap<>(possibleUris.size()); for (URI possibleUri : possibleUris) @@ -975,8 +972,11 @@ private Map getPotentialClients(ServiceProperties servicePro // don't pay attention to this uri if it's banned if (!serviceProperties.isBanned(possibleUri) && !clusterProperties.isBanned(possibleUri)) { - trackerClientFinder.apply(possibleUri) - .ifPresent(trackerClient -> clientsToLoadBalance.put(possibleUri, trackerClient)); + TrackerClient trackerClient = trackerClientFinder.apply(possibleUri); + if (trackerClient != null) + { + clientsToLoadBalance.put(possibleUri, trackerClient); + } } else { From f7894b48cb6049d14ca21dd649db5c8a9a56b233 Mon Sep 17 00:00:00 2001 From: PapaCharlie Date: Mon, 27 Nov 2023 21:52:51 -0500 Subject: [PATCH 2/2] Bump gradle.properties and changelog --- CHANGELOG.md | 7 ++++++- gradle.properties | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd373768f1..618e5016aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ When updating the changelog, remember to be very clear about what behavior has c and what APIs have changed, if applicable. ## [Unreleased] + +## [29.48.2] - 2023-11-27 +- Remove usage of Optional from SimpleLoadBalancer + ## [29.48.1] - 2023-11-27 - Update SimpleLoadBalancer to use for loop instead of Map @@ -5565,7 +5569,8 @@ patch operations can re-use these classes for generating patch messages. ## [0.14.1] -[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.48.1...master +[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.48.2...master +[29.48.2]: https://github.com/linkedin/rest.li/compare/v29.48.1...v29.48.2 [29.48.1]: https://github.com/linkedin/rest.li/compare/v29.48.0...v29.48.1 [29.48.0]: https://github.com/linkedin/rest.li/compare/v29.47.0...v29.48.0 [29.47.0]: https://github.com/linkedin/rest.li/compare/v29.46.9...v29.47.0 diff --git a/gradle.properties b/gradle.properties index ec4d20ea21..4224297576 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=29.48.0 +version=29.48.2 group=com.linkedin.pegasus org.gradle.configureondemand=true org.gradle.parallel=true