[optimise] New implementation of previously buggy optimised CharArray…

…Pool from '[optimise] Simple fix to pathological char array pool'

exist-core/src/main/java/org/exist/util/CharArrayPool.java

@@ -7,6 +7,8 @@
 
 import net.jcip.annotations.ThreadSafe;
 
+import javax.annotation.Nullable;
+
 /**
  * A pool for char arrays.
  *
@@ -15,42 +17,110 @@
  * only char[] with length < MAX are kept in the pool. Larger char[] are rarely reused.
  *
  * The pool is bound to the current thread.
+ *
+ * @author <a href="mailto:[email protected]">Adam Retter</a>.
  */
 @ThreadSafe
 public class CharArrayPool {
 
+    private static class PoolData {
+        int maxIndex = -1;
+        int lastGetIndex = -1;
+        int lastReleaseIndex = -1;
+        final char[][] pool = new char[POOL_SIZE][];
+    }
+
     private static final int POOL_SIZE = 128;
-    private static final int MIN_SIZE = 8;
+    private static final int MIN_CHAR_ARRAY_SIZE = 8;
+    private static final int MAX_CHAR_ARRAY_SIZE = 128;
 
-    private static final ThreadLocal<PoolContent> POOLS = ThreadLocal.withInitial(PoolContent::new);
+    private static final ThreadLocal<PoolData> POOLS = ThreadLocal.withInitial(PoolData::new);
 
     private CharArrayPool() {
     }
 
-    public static char[] getCharArray(final int size) {
-        final PoolContent poolContent = POOLS.get();
-        if (POOL_SIZE > size) {
-            final int alloc = Math.max(size, MIN_SIZE);
-            final char[][] pool = poolContent.pool;
-            if (pool[alloc] != null) {
-                final char[] b = POOLS.get().pool[alloc];
-                pool[alloc] = null;
-                return b;
+    public static char[] getCharArray(int minSize) {
+        if (minSize > MAX_CHAR_ARRAY_SIZE) {
+            // the requested minSize is over the size that is permitted in the pool, so just return a new char[] array from outside the pool
+            return new char[minSize];
+        }
+
+        // ensure the minSize is at least MIN_CHAR_ARRAY_SIZE
+        minSize = Math.max(MIN_CHAR_ARRAY_SIZE, minSize);
+
+        @Nullable char[] array = null;
+        final PoolData localPoolData = POOLS.get();
+
+        // Get the array last released to the pool if it is of minSize (as it is likely that get/release is called multiple times for similar size strings)
+        if (localPoolData.lastReleaseIndex != -1) {
+            array = localPoolData.pool[localPoolData.lastReleaseIndex];
+            if (array.length >= minSize) {
+                localPoolData.pool[localPoolData.lastReleaseIndex] = null;    // remove array from pool
+                localPoolData.lastGetIndex = localPoolData.lastReleaseIndex;  // record the index of this get
+                localPoolData.lastReleaseIndex = -1;                          // mark lastReleaseIndex as invalid
+                return array;
+            }
+        }
+
+        // Attempt to find an array (of minSize) in the pool by reading from right-to-left (as items recently returned are more likely to be on the right)
+        for (int i = localPoolData.maxIndex; i > -1; i--) {
+            array = localPoolData.pool[i];
+            if (array != null && array.length >= minSize) {
+                localPoolData.pool[i] = null;    // remove array from pool
+                localPoolData.lastGetIndex = i;  // record the index of this get
+                return array;
             }
         }
-        return new char[Math.max(size, MIN_SIZE)];
+
+        // not found in the pool, so just return a new char[] array from outside the pool
+        return new char[minSize];
     }
 
-    public static void releaseCharArray(final char[] b) {
-        if (b == null || b.length > POOL_SIZE) {
+    public static void releaseCharArray(@Nullable final char[] array) {
+        if (array == null || array.length > MAX_CHAR_ARRAY_SIZE) {
+            // the array is over the size that is permitted in the pool, so do nothing as it should not be placed in the pool
             return;
         }
-        final PoolContent poolContent = POOLS.get();
-        poolContent.pool[b.length] = b;
-     }
 
-    private static class PoolContent {
-        final char[][] pool = new char[POOL_SIZE][];
-    }
+        final PoolData localPoolData = POOLS.get();
 
+        // If the pool is empty, place the array at the start of the pool
+        if (localPoolData.maxIndex == -1) {
+            localPoolData.pool[0] = array;
+            localPoolData.lastReleaseIndex = 0;  // record the index of this release
+            localPoolData.maxIndex = 0;
+            return;
+        }
+
+        // Get the index of the array last get'ed from the pool (as it is likely that get/release is called multiple times for similar size strings)
+        if (localPoolData.lastGetIndex != -1) {
+            localPoolData.pool[localPoolData.lastGetIndex] = array;                                     // place array in the pool
+            localPoolData.lastReleaseIndex = localPoolData.lastGetIndex;                                // record the index of this release
+            localPoolData.lastGetIndex = -1;                                                            // mark lastGetIndex as invalid
+            localPoolData.maxIndex = Math.max(localPoolData.maxIndex, localPoolData.lastReleaseIndex);  // update the maxIndex
+            return;
+        }
+
+        // Attempt to find an empty space in the pool by reading from right-to-left (as items recently released are more likely to be on the right)
+        for (int i = localPoolData.maxIndex; i > -1; i--) {
+            if (localPoolData.pool[i] == null) {
+                localPoolData.pool[i] = array;       // place array in the pool
+                localPoolData.lastReleaseIndex = i;  // record the index of this release
+
+                //TODO(AR) if we were to record the right-most not-null entry, we could potentially shrink the maxIndex of the pool here, which would reduce the search space in the next lookup(s)
+
+                return;
+            }
+        }
+
+        // couldn't find an existing space in the pool, if there is remaining space then append it
+        if (localPoolData.maxIndex < POOL_SIZE - 1) {
+            localPoolData.pool[++localPoolData.maxIndex] = array;     // place array in the pool
+            localPoolData.lastReleaseIndex = localPoolData.maxIndex;  // record the index of this release
+            return;
+        }
+
+        // NOTE(AR): If we reach here, the pool is at maximum capacity and is full, we might in future want to record stats about this so we could configure a larger/smaller pool?
+
+     }
 }