(accumulo) branch 2.1 updated: reduce scan thread contention on native maps (#4910)

[kturner]

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

kturner pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/2.1 by this push:
     new 2cdefa30b8 reduce scan thread contention on native maps (#4910)
2cdefa30b8 is described below

commit 2cdefa30b83366cb3fc8115083333a95b51285d6
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Fri Sep 20 16:33:54 2024 -0400

    reduce scan thread contention on native maps (#4910)
    
    Observed many scan threads in the native map code attempting to create
    a native map iterator and blocking in registering a cleaner for the
    iterator.  The block was happening in 
jdk.internal.rf.PhantomCleanable.insert()
    which has a synchronized code block.  There is a single static cleaner
    object on the accumulo code so all scan threads that internact with a
    native map could end up contending on this single lock in the process.
    
    This change creates 8 cleaners and therefore 8 locks to reduce the
    contention between scan threads.
    
    
    Co-authored-by: Christopher Tubbs <ctubb...@apache.org>
---
 .../accumulo/tserver/NativeMapCleanerUtil.java     | 27 ++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMapCleanerUtil.java
 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMapCleanerUtil.java
index 944789c1aa..12c1509807 100644
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMapCleanerUtil.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMapCleanerUtil.java
@@ -20,6 +20,7 @@ package org.apache.accumulo.tserver;
 
 import static java.util.Objects.requireNonNull;
 
+import java.lang.ref.Cleaner;
 import java.lang.ref.Cleaner.Cleanable;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -43,14 +44,36 @@ public class NativeMapCleanerUtil {
     });
   }
 
+  // Chose 7 cleaners because each cleaner creates a thread, so do not want 
too many threads. This
+  // should reduce lock contention for scans by a 7th vs a single cleaner, so 
that is good
+  // reduction and 7 does not seem like too many threads to add. This array is 
indexed using
+  // pointers addresses from native code, so there is a good chance those are 
memory aligned on
+  // a multiple of 4, 8, 16, etc. So if changing the array size avoid 
multiples of 2.
+  private static final Cleaner[] NMI_CLEANERS = new Cleaner[7];
+
+  static {
+    for (int i = 0; i < NMI_CLEANERS.length; i++) {
+      NMI_CLEANERS[i] = Cleaner.create();
+    }
+  }
+
   public static Cleanable deleteNMIterator(Object obj, AtomicLong nmiPtr) {
     requireNonNull(nmiPtr);
-    return CleanerUtil.CLEANER.register(obj, () -> {
+
+    int cleanerIndex = (int) (nmiPtr.get() % NMI_CLEANERS.length);
+    if (cleanerIndex < 0) {
+      cleanerIndex += NMI_CLEANERS.length;
+    }
+
+    // This method can be called very frequently by many scan threads. The 
register call on cleaner
+    // acquires a lock which can cause lock contention between threads. Having 
multiple cleaners for
+    // this case lowers the amount of lock contention among scan threads. This 
locking was observed
+    // in jdk.internal.rf.PhantomCleanable.insert() which currently has a 
synchronized code block.
+    return NMI_CLEANERS[cleanerIndex].register(obj, () -> {
       long nmiPointer = nmiPtr.get();
       if (nmiPointer != 0) {
         NativeMap._deleteNMI(nmiPointer);
       }
     });
   }
-
 }