[geode] branch develop updated: GEODE-6584: remove unecessary synchronization (#3387)

[he...@apache.org]

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

heybales pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 6eb32e2  GEODE-6584: remove unecessary synchronization (#3387)
6eb32e2 is described below

commit 6eb32e22dbeefd63a8ea0ceb11101b9e6378ef83
Author: Helena Bales <hb...@pivotal.io>
AuthorDate: Mon Apr 8 13:46:37 2019 -0700

    GEODE-6584: remove unecessary synchronization (#3387)
    
    Remove synchronization around changing values that could be volatile and
    that are atomic. Results in a performance gain.
    
    * make processMessages in ServerConnection private and add getter and
    setter
    * refactor check for if the system is shutting down and stopping
    processing of messages in ServerConnection
---
 .../cache/tier/sockets/ServerConnection.java       | 22 ++++++++++++++--------
 .../tier/sockets/ServerConnectionCollection.java   | 10 ++++++++++
 .../tier/sockets/ProtobufServerConnectionTest.java |  4 ++--
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
index 4c90699..b489dbb 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
@@ -743,7 +743,15 @@ public abstract class ServerConnection implements Runnable {
   private boolean clientDisconnectedCleanly = false;
   private Throwable clientDisconnectedException;
   private int failureCount = 0;
-  boolean processMessages = true;
+  private volatile boolean processMessages = true;
+
+  public boolean getProcessMessages() {
+    return processMessages;
+  }
+
+  public void setProcessMessages(boolean newValue) {
+    processMessages = newValue;
+  }
 
   protected void doHandshake() {
     // hitesh:to create new connection handshake
@@ -773,14 +781,12 @@ public abstract class ServerConnection implements Runnable {
     }
     Message message;
     message = BaseCommand.readRequest(this);
-    synchronized (serverConnectionCollection) {
-      if (serverConnectionCollection.isTerminating) {
-        // Client is being disconnected, don't try to process message.
-        this.processMessages = false;
-        return;
-      }
-      serverConnectionCollection.connectionsProcessing.incrementAndGet();
+    if (!serverConnectionCollection.incrementConnectionsProcessing()) {
+      // Client is being disconnected, don't try to process message.
+      this.processMessages = false;
+      return;
     }
+
     ThreadState threadState = null;
     try {
       if (message != null) {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionCollection.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionCollection.java
index 670b5a1..e00b142 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionCollection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionCollection.java
@@ -41,4 +41,14 @@ public class ServerConnectionCollection {
   public void removeConnection(ServerConnection connection) {
     connectionSet.remove(connection);
   }
+
+  public boolean incrementConnectionsProcessing() {
+    if (!isTerminating) {
+      connectionsProcessing.incrementAndGet();
+
+      return true;
+    }
+
+    return false;
+  }
 }
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/cache/tier/sockets/ProtobufServerConnectionTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/cache/tier/sockets/ProtobufServerConnectionTest.java
index cdf7a2a..3ff7138 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/cache/tier/sockets/ProtobufServerConnectionTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/cache/tier/sockets/ProtobufServerConnectionTest.java
@@ -50,9 +50,9 @@ public class ProtobufServerConnectionTest {
   @Test
   public void testProcessFlag() throws Exception {
     ServerConnection serverConnection = IOExceptionThrowingServerConnection();
-    Assert.assertTrue(serverConnection.processMessages);
+    Assert.assertTrue(serverConnection.getProcessMessages());
     serverConnection.doOneMessage();
-    Assert.assertTrue(!serverConnection.processMessages);
+    Assert.assertTrue(!serverConnection.getProcessMessages());
   }
 
   @Test