[GitHub] [accumulo] keith-turner commented on a change in pull request #1357: Use ConcurrentHashMap instead of synchronized blocks

[GitBox]

keith-turner commented on a change in pull request #1357: Use ConcurrentHashMap 
instead of synchronized blocks
URL: https://github.com/apache/accumulo/pull/1357#discussion_r323941807
 
 

 ##########
 File path: 
server/tserver/src/main/java/org/apache/accumulo/tserver/RowLocks.java
 ##########
 @@ -58,36 +61,33 @@ public void unlock() {
   }
 
   private RowLock getRowLock(ArrayByteSequence rowSeq) {
-    RowLock lock = rowLocks.get(rowSeq);
-    if (lock == null) {
-      lock = new RowLock(new ReentrantLock(), rowSeq);
-      rowLocks.put(rowSeq, lock);
-    }
-
-    lock.count++;
-    return lock;
+    return rowLocks.compute(rowSeq, (key, value) -> {
+      if (value == null) {
+        return new RowLock(new ReentrantLock(), rowSeq);
+      }
+      value.count++;
+      return value;
+    });
   }
 
   private void returnRowLock(RowLock lock) {
-    if (lock.count == 0)
-      throw new IllegalStateException();
-    lock.count--;
-
-    if (lock.count == 0) {
-      rowLocks.remove(lock.rowSeq);
-    }
+    Objects.requireNonNull(lock);
+    rowLocks.compute(lock.rowSeq, (key, value) -> {
+      Preconditions.checkState(value == lock);
+      Preconditions.checkState(value.count > 0);
+      final int newCount = --value.count;
 
 Review comment:
   I think it would be nice to inline `newCount` 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services