This is an automated email from the ASF dual-hosted git repository.

penghui pushed a commit to branch branch-2.8
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit 44b617b5830eed596fc3d6861231998aa9234d99
Author: 萧易客 <[email protected]>
AuthorDate: Tue Jan 18 11:08:46 2022 +0800

    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)
---
 .../org/apache/pulsar/broker/service/AbstractTopic.java    | 14 +++++++-------
 .../pulsar/broker/service/persistent/PersistentTopic.java  | 13 +++++++------
 .../util/collections/ConcurrentSortedLongPairSet.java      | 14 ++++++++------
 .../util/collections/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 2576050..65296a7 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.CompletableFuture;
 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 abstract class AbstractTopic implements Topic {
     }
 
     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 52beacd..11ec5f2 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 class PersistentTopic extends AbstractTopic
     }
 
     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 d3321f9..95e2302 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 class ConcurrentSortedLongPairSet implements 
LongPairSet {
 
     @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 821bb88..fcb9884 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.assertEquals;
 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.Set;
 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 class ConcurrentSortedLongPairSetTest {
         assertEquals(set.toString(), toString);
     }
 
+    @Test
+    public void testIsEmpty() {
+        LongPairSet set = new ConcurrentSortedLongPairSet();
+        assertTrue(set.isEmpty());
+        set.add(1, 1);
+        assertFalse(set.isEmpty());
+    }
 }

Reply via email to