You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by nn...@apache.org on 2021/06/08 23:51:11 UTC
[geode] branch support/1.13 updated: GEODE-9331: remove the
threadConnMaps ArrayList (#6535)
This is an automated email from the ASF dual-hosted git repository.
nnag pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push:
new 9291564 GEODE-9331: remove the threadConnMaps ArrayList (#6535)
9291564 is described below
commit 92915643a20687af83d0ca0322c24b52f1d4a41d
Author: Darrel Schneider <da...@vmware.com>
AuthorDate: Wed Jun 2 09:20:45 2021 -0700
GEODE-9331: remove the threadConnMaps ArrayList (#6535)
Removed the threadConnMaps ArrayList.
This removal also means that the HashMap is now
only referenced by a ThreadLocal so it no longer
is synchronized which simplified the code.
(cherry picked from commit 88918f1221e7bf90c88596d19c06ec41eec8315e)
---
.../apache/geode/internal/tcp/ConnectionTable.java | 93 +++++-----------------
1 file changed, 21 insertions(+), 72 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java b/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java
index 4ed1a17..bca4480 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java
@@ -16,8 +16,6 @@ package org.apache.geode.internal.tcp;
import java.io.Closeable;
import java.io.IOException;
-import java.lang.ref.Reference;
-import java.lang.ref.WeakReference;
import java.net.InetAddress;
import java.net.Socket;
import java.util.ArrayList;
@@ -82,18 +80,11 @@ public class ConnectionTable {
private final Map orderedConnectionMap = new ConcurrentHashMap();
/**
- * ordered connections local to this thread. Note that accesses to the resulting map must be
- * synchronized because of static cleanup.
+ * ordered connections local to this thread.
*/
static final ThreadLocal<Map> threadOrderedConnMap = new ThreadLocal<>();
/**
- * List of thread-owned ordered connection maps, for cleanup. Accesses to the maps in this list
- * need to be synchronized on their instance.
- */
- private final List threadConnMaps;
-
- /**
* Timer to kill idle threads. Guarded by this.
*/
private SystemTimer idleConnTimer;
@@ -200,7 +191,6 @@ public class ConnectionTable {
owner = conduit;
idleConnTimer = owner.idleConnectionTimeout != 0
? new SystemTimer(conduit.getDM().getSystem()) : null;
- threadConnMaps = new ArrayList();
threadConnectionMap = new ConcurrentHashMap();
p2pReaderThreadPool = createThreadPoolForIO(conduit.getDM().getSystem().isShareSockets());
socketCloser = new SocketCloser();
@@ -440,33 +430,14 @@ public class ConnectionTable {
if (m == null) {
// First time for this thread. Create thread local
m = new HashMap();
- synchronized (threadConnMaps) {
- if (closed) {
- owner.getCancelCriterion().checkCancelInProgress(null);
- throw new DistributedSystemDisconnectedException("Connection table is closed");
- }
- // check for stale references and remove them.
- for (Iterator it = threadConnMaps.iterator(); it.hasNext();) {
- Reference r = (Reference) it.next();
- if (r.get() == null) {
- it.remove();
- }
- }
- threadConnMaps.add(new WeakReference(m));
- }
threadOrderedConnMap.set(m);
} else {
- // Consult thread local.
- synchronized (m) {
- result = (Connection) m.get(id);
- }
- if (result != null && result.timedOut) {
- result = null;
+ // No need to sync map since it is only referenced by ThreadLocal.
+ result = (Connection) m.get(id);
+ if (result != null && !result.timedOut) {
+ return result;
}
}
- if (result != null) {
- return result;
- }
// OK, we have to create a new connection.
result = Connection.createSender(owner.getMembership(), this, true, id, false, startTime,
@@ -500,9 +471,8 @@ public class ConnectionTable {
}
// Finally, add the connection to our thread local map.
- synchronized (m) {
- m.put(id, result);
- }
+ // No need to sync map since it is only referenced by ThreadLocal.
+ m.put(id, result);
scheduleIdleTimeout(result);
return result;
@@ -667,22 +637,6 @@ public class ConnectionTable {
this.threadConnectionMap.clear();
}
}
- if (threadConnMaps != null) {
- synchronized (threadConnMaps) {
- for (Object threadConnMap : threadConnMaps) {
- Reference reference = (Reference) threadConnMap;
- Map map = (Map) reference.get();
- if (map != null) {
- synchronized (map) {
- for (Object o : map.values()) {
- closeCon("Connection table being destroyed", o);
- }
- }
- }
- }
- threadConnMaps.clear();
- }
- }
Executor localExec = p2pReaderThreadPool;
if (localExec != null) {
if (localExec instanceof ExecutorService) {
@@ -693,9 +647,8 @@ public class ConnectionTable {
Map map = threadOrderedConnMap.get();
if (map != null) {
- synchronized (map) {
- map.clear();
- }
+ // No need to synchronize map since it is only referenced by ThreadLocal.
+ map.clear();
}
socketCloser.close();
}
@@ -896,11 +849,9 @@ public class ConnectionTable {
removeFromThreadConMap(threadConnectionMap, stub, c);
Map m = threadOrderedConnMap.get();
if (m != null) {
- // Static cleanup thread might intervene, so we MUST synchronize
- synchronized (m) {
- if (m.get(stub) == c) {
- m.remove(stub);
- }
+ // No need to synchronize map since it is only referenced by ThreadLocal.
+ if (m.get(stub) == c) {
+ m.remove(stub);
}
}
}
@@ -953,17 +904,15 @@ public class ConnectionTable {
void removeAndCloseThreadOwnedSockets() {
Map m = threadOrderedConnMap.get();
if (m != null) {
- // Static cleanup may intervene; we MUST synchronize.
- synchronized (m) {
- Iterator it = m.entrySet().iterator();
- while (it.hasNext()) {
- Map.Entry me = (Map.Entry) it.next();
- DistributedMember stub = (DistributedMember) me.getKey();
- Connection c = (Connection) me.getValue();
- removeFromThreadConMap(threadConnectionMap, stub, c);
- it.remove();
- closeCon("thread finalization", c);
- }
+ // No need to synchronize map since it is only referenced by ThreadLocal.
+ Iterator it = m.entrySet().iterator();
+ while (it.hasNext()) {
+ Map.Entry me = (Map.Entry) it.next();
+ DistributedMember stub = (DistributedMember) me.getKey();
+ Connection c = (Connection) me.getValue();
+ removeFromThreadConMap(threadConnectionMap, stub, c);
+ it.remove();
+ closeCon("thread finalization", c);
}
}
}