Some list creation cleanups

apple · Dec 13, 2023 · f9575cc · f9575cc
1 parent 138b605
commit f9575cc
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 22 deletions.
diff --git a/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/DefaultHost.java b/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/DefaultHost.java
@@ -45,6 +45,7 @@
 import static io.servicetalk.concurrent.api.Single.succeeded;
 import static io.servicetalk.concurrent.internal.FlowControlUtils.addWithOverflowProtection;
 import static java.lang.Math.min;
+import static java.util.Collections.emptyList;
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.atomic.AtomicReferenceFieldUpdater.newUpdater;
 
@@ -98,7 +99,7 @@ private enum State {
     private final ConnectionFactory<Addr, ? extends C> connectionFactory;
     private final int linearSearchSpace;
     private final ListenableAsyncCloseable closeable;
-    private volatile ConnState connState = new ConnState(new ArrayList<>(), State.ACTIVE, 0, null);
+    private volatile ConnState connState = new ConnState(emptyList(), State.ACTIVE, 0, null);
 
     DefaultHost(final String lbDescription, final Addr address,
                 final ConnectionFactory<Addr, ? extends C> connectionFactory,
@@ -579,10 +580,16 @@ ConnState removeConnection(C connection) {
             if (index < 0) {
                 return this;
             }
-            List<C> newList = new ArrayList<>(connections.size() - 1);
-            for (int i = 0; i < connections.size(); i++) {
-                if (i != index) {
-                    newList.add(connections.get(i));
+            // Create the new list.
+            final List<C> newList;
+            if (connections.size() == 1) {
+                newList = emptyList();
+            } else {
+                newList = new ArrayList<>(connections.size() - 1);
+                for (int i = 0; i < connections.size(); i++) {
+                    if (i != index) {
+                        newList.add(connections.get(i));
+                    }
                 }
             }
             return new ConnState(newList, state, failedConnections, healthCheck);

diff --git a/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java b/servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/DefaultLoadBalancer.java
@@ -387,7 +387,7 @@ private Host<ResolvedAddress, C> createHost(ResolvedAddress addr) {
                             return;
                         }
                         final List<Host<ResolvedAddress, C>> nextHosts = listWithHostRemoved(
-                                currentHosts, current -> current == host);
+                                currentHosts, host);
                         // we only need to do anything else if we actually removed the host
                         if (nextHosts.size() != currentHosts.size()) {
                             loadBalancerObserver.hostObserver().onExpiredHostRemoved(host.address());
@@ -401,25 +401,22 @@ private Host<ResolvedAddress, C> createHost(ResolvedAddress addr) {
             return host;
         }
 
-        private List<Host<ResolvedAddress, C>> listWithHostRemoved(
-                List<Host<ResolvedAddress, C>> oldHostsTyped, Predicate<Host<ResolvedAddress, C>> hostPredicate) {
-            if (oldHostsTyped.isEmpty()) {
-                // this can happen when an expired host is removed during closing of the DefaultLoadBalancer,
-                // but all of its connections have already been closed
+        private List<Host<ResolvedAddress, C>> listWithHostRemoved(List<Host<ResolvedAddress, C>> oldHostsTyped,
+                                                                   Host<ResolvedAddress, C> toRemove) {
+            final int index = oldHostsTyped.indexOf(toRemove);
+            if (index < 0) {
+                // Element doesn't exist: just return the old list.
                 return oldHostsTyped;
             }
-            // We keep the old size as the capacity hint because the penalty for a resize in the case that the
-            // element isn't in the list is much worse than the penalty for an unused array slot.
-            final List<Host<ResolvedAddress, C>> newHosts = new ArrayList<>(oldHostsTyped.size());
+            if (oldHostsTyped.size() == 1) {
+                // We're removing the last host in the list so we can just return the empty list.
+                return emptyList();
+            }
+            // Copy the remaining live elements to a new list.
+            final List<Host<ResolvedAddress, C>> newHosts = new ArrayList<>(oldHostsTyped.size() - 1);
             for (int i = 0; i < oldHostsTyped.size(); ++i) {
-                final Host<ResolvedAddress, C> current = oldHostsTyped.get(i);
-                if (hostPredicate.test(current)) {
-                    for (int x = i + 1; x < oldHostsTyped.size(); ++x) {
-                        newHosts.add(oldHostsTyped.get(x));
-                    }
-                    return newHosts.isEmpty() ? emptyList() : newHosts;
-                } else {
-                    newHosts.add(current);
+                if (i != index) {
+                    newHosts.add(oldHostsTyped.get(i));
                 }
             }
             return newHosts;