You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2017/12/07 20:12:22 UTC

svn commit: r1817414 - /jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ClientPool.java

Author: pmouawad
Date: Thu Dec  7 20:12:21 2017
New Revision: 1817414

URL: http://svn.apache.org/viewvc?rev=1817414&view=rev
Log:
Cleanup code and useless methods.
Try to improve a bit synchro. My understanding is that CopyOnWriteList would not perform good here as there is only 1 read vs many writes.

Modified:
    jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ClientPool.java

Modified: jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ClientPool.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ClientPool.java?rev=1817414&r1=1817413&r2=1817414&view=diff
==============================================================================
--- jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ClientPool.java (original)
+++ jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ClientPool.java Thu Dec  7 20:12:21 2017
@@ -18,10 +18,11 @@
 package org.apache.jmeter.protocol.jms.client;
 
 import java.io.Closeable;
-import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.jorphan.util.JOrphanUtils;
 
 /**
  *
@@ -29,25 +30,33 @@ import java.util.concurrent.ConcurrentHa
  * this is to make it easier to clean up all the instances at the end of a test.
  * If we didn't do this, threads might become zombie.
  * 
- * N.B. This class needs to be fully synchronized as it is called from sample threads
+ * N.B. This class is thread safe as it is called from sample threads
  * and the thread that runs testEnded() methods.
  */
 public class ClientPool {
 
-    //GuardedBy("this")
-    private static final ArrayList<Closeable> clients = new ArrayList<>();
-
-    //GuardedBy("this")
-    private static final Map<Object, Object> client_map = new ConcurrentHashMap<>();
+    private static final List<Closeable> CLIENTS = Collections.synchronizedList(new ArrayList<>());
 
+    private ClientPool() {
+        super();
+    }
+    
     /**
      * Add a ReceiveClient to the ClientPool. This is so that we can make sure
      * to close all clients and make sure all threads are destroyed.
      *
      * @param client the ReceiveClient to add
      */
-    public static synchronized void addClient(Closeable client) {
-        clients.add(client);
+    public static void addClient(Closeable client) {
+        CLIENTS.add(client);
+    }
+
+    /**
+     * Remove publisher from clients
+     * @param publisher {@link Publisher}
+     */
+    public static void removeClient(Publisher publisher) {
+        CLIENTS.remove(publisher);
     }
 
     /**
@@ -57,32 +66,10 @@ public class ClientPool {
      * manufacturer of the JMS server may have bugs and some threads may become
      * zombie. In those cases, it is not JMeter's responsibility.
      */
-    public static synchronized void clearClient() {
-        for (Closeable client : clients) {
-            try {
-                client.close();
-            } catch (IOException ignored) {
-            }
+    public static void clearClient() {
+        synchronized (CLIENTS) {
+            CLIENTS.forEach(JOrphanUtils::closeQuietly);            
         }
-        clients.clear();
-        client_map.clear();
-    }
-
-    // TODO Method with 0 reference, really useful ?
-    public static void put(Object key, Object client) {
-        client_map.put(key, client);
-    }
-
-    // TODO Method with 0 reference, really useful ?
-    public static Object get(Object key) {
-        return client_map.get(key);
-    }
-
-    /**
-     * Remove publisher from clients
-     * @param publisher {@link Publisher}
-     */
-    public static synchronized void removeClient(Publisher publisher) {
-        clients.remove(publisher);
+        CLIENTS.clear();
     }
 }