From e9abad2fb740de0f5c34ab6fb80d64a63a80f95a Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 2 Dec 2024 18:27:29 +0000 Subject: [PATCH] [optimise] New implementation of previously buggy optimised CharArrayPool from '[optimise] Simple fix to pathological char array pool' --- .../java/org/exist/util/CharArrayPool.java | 110 ++++++++++++++---- 1 file changed, 90 insertions(+), 20 deletions(-) diff --git a/exist-core/src/main/java/org/exist/util/CharArrayPool.java b/exist-core/src/main/java/org/exist/util/CharArrayPool.java index 3bec1281b02..2c3e2be7cd4 100644 --- a/exist-core/src/main/java/org/exist/util/CharArrayPool.java +++ b/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 Adam Retter. */ @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 POOLS = ThreadLocal.withInitial(PoolContent::new); + private static final ThreadLocal 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? + + } }