From 44b617b5830eed596fc3d6861231998aa9234d99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=90=A7=E6=98=93=E5=AE=A2?= <kmter@live.com>
Date: Tue, 18 Jan 2022 11:08:46 +0800
Subject: [PATCH] Fix inefficient forEach loop (#13742)

### Motivation
There are some methods implemented with an inefficient forEach loop, so fix it to get better performance. It is similar to #12953

### Modifications

I rewrite it with the `for` loop.

(cherry picked from commit 9c94cd7803f3653e5aa170f80f5591fe68cc0366)
---
 .../pulsar/broker/service/AbstractTopic.java       | 14 +++++++-------
 .../broker/service/persistent/PersistentTopic.java | 13 +++++++------
 .../collections/ConcurrentSortedLongPairSet.java   | 14 ++++++++------
 .../ConcurrentSortedLongPairSetTest.java           | 12 ++++++++----
 4 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
index 2576050cadc7c..65296a7f57426 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
@@ -31,7 +31,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLongFieldUpdater;
 import java.util.concurrent.atomic.LongAdder;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -264,14 +263,15 @@ public void enableCnxAutoRead() {
     }
 
     protected boolean hasLocalProducers() {
-        AtomicBoolean foundLocal = new AtomicBoolean(false);
-        producers.values().forEach(producer -> {
+        if (producers.isEmpty()) {
+            return false;
+        }
+        for (Producer producer : producers.values()) {
             if (!producer.isRemote()) {
-                foundLocal.set(true);
+                return true;
             }
-        });
-
-        return foundLocal.get();
+        }
+        return false;
     }
 
     @Override
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
index 52beacdd545e8..11ec5f2e0d1ba 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
@@ -596,14 +596,15 @@ public void updatePropertiesFailed(ManagedLedgerException exception, Object ctx)
     }
 
     private boolean hasRemoteProducers() {
-        AtomicBoolean foundRemote = new AtomicBoolean(false);
-        producers.values().forEach(producer -> {
+        if (producers.isEmpty()) {
+            return false;
+        }
+        for (Producer producer : producers.values()) {
             if (producer.isRemote()) {
-                foundRemote.set(true);
+                return true;
             }
-        });
-
-        return foundRemote.get();
+        }
+        return false;
     }
 
     public void startReplProducers() {
diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSet.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSet.java
index d3321f9ad35ac..95e2302dcb7b9 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSet.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSet.java
@@ -166,13 +166,15 @@ public String toString() {
 
     @Override
     public boolean isEmpty() {
-        AtomicBoolean isEmpty = new AtomicBoolean(true);
-        longPairSets.forEach((item1, longPairSet) -> {
-            if (isEmpty.get() && !longPairSet.isEmpty()) {
-                isEmpty.set(false);
+        if (longPairSets.isEmpty()) {
+            return true;
+        }
+        for (ConcurrentLongPairSet subSet : longPairSets.values()) {
+            if (!subSet.isEmpty()) {
+                return false;
             }
-        });
-        return isEmpty.get();
+        }
+        return true;
     }
 
     @Override
diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSetTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSetTest.java
index 821bb8819554b..fcb9884a795ad 100644
--- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSetTest.java
+++ b/pulsar-common/src/test/java/org/apache/pulsar/common/util/collections/ConcurrentSortedLongPairSetTest.java
@@ -22,7 +22,7 @@
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertTrue;
-
+import com.google.common.collect.Lists;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Random;
@@ -30,13 +30,10 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-
 import lombok.Cleanup;
 import org.apache.pulsar.common.util.collections.ConcurrentLongPairSet.LongPair;
 import org.testng.annotations.Test;
 
-import com.google.common.collect.Lists;
-
 public class ConcurrentSortedLongPairSetTest {
 
     @Test
@@ -241,4 +238,11 @@ public void testToString() {
         assertEquals(set.toString(), toString);
     }
 
+    @Test
+    public void testIsEmpty() {
+        LongPairSet set = new ConcurrentSortedLongPairSet();
+        assertTrue(set.isEmpty());
+        set.add(1, 1);
+        assertFalse(set.isEmpty());
+    }
 }