diff --git a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/fullData/sources/FullDataSourceV2.java b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/fullData/sources/FullDataSourceV2.java index cb65b2d64..3be3eccec 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/fullData/sources/FullDataSourceV2.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/dataObjects/fullData/sources/FullDataSourceV2.java @@ -73,7 +73,7 @@ public class FullDataSourceV2 public static final byte DATA_FORMAT_VERSION = 1; - public static final PhantomArrayListPool ARRAY_LIST_POOL = new PhantomArrayListPool("FullDataV2", false); + public static final PhantomArrayListPool ARRAY_LIST_POOL = new PhantomArrayListPool("FullDataV2"); @@ -269,11 +269,6 @@ public class FullDataSourceV2 { ListUtil.clearAndSetSize(this.columnWorldCompressionMode, WIDTH * WIDTH); } - - - // the pooled arrays have all been set, - // the checkout object is no longer needed - this.pooledArraysCheckout = null; } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListCheckout.java b/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListCheckout.java index 6649d31c9..bb9c5a474 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListCheckout.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListCheckout.java @@ -5,10 +5,12 @@ import com.seibel.distanthorizons.coreapi.util.StringUtil; import it.unimi.dsi.fastutil.bytes.ByteArrayList; import it.unimi.dsi.fastutil.longs.LongArrayList; import it.unimi.dsi.fastutil.shorts.ShortArrayList; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.lang.ref.SoftReference; import java.util.ArrayList; +import java.util.concurrent.atomic.AtomicBoolean; /** * This keeps track of all the poolable @@ -20,7 +22,16 @@ import java.util.ArrayList; public class PhantomArrayListCheckout implements AutoCloseable { /** defines which pool the arrays should be returned too */ + @NotNull private final PhantomArrayListPool owningPool; + + /** + * soft reference used by the {@link PhantomArrayListPool} so this checkout can be + * freed if there isn't enough memory. + */ + @NotNull + public final SoftReference ownerSoftReference; + /** Will be null if the parent pool doesn't want leak stack tracing */ @Nullable public final String allocationStackTrace; @@ -28,7 +39,6 @@ public class PhantomArrayListCheckout implements AutoCloseable private final ArrayList byteArrayLists = new ArrayList<>(); private final ArrayList shortArrayLists = new ArrayList<>(); private final ArrayList longArrayLists = new ArrayList<>(); - private final ArrayList> longArrayRefLists = new ArrayList<>(); @@ -36,7 +46,7 @@ public class PhantomArrayListCheckout implements AutoCloseable // constructor // //=============// - public PhantomArrayListCheckout(PhantomArrayListPool owningPool) + public PhantomArrayListCheckout(@NotNull PhantomArrayListPool owningPool) { if (owningPool.logGarbageCollectedStacks) { @@ -50,6 +60,7 @@ public class PhantomArrayListCheckout implements AutoCloseable } this.owningPool = owningPool; + this.ownerSoftReference = new SoftReference<>(this); } @@ -60,11 +71,7 @@ public class PhantomArrayListCheckout implements AutoCloseable public void addByteArrayList(ByteArrayList list) { this.byteArrayLists.add(list); } public void addShortArrayList(ShortArrayList list) { this.shortArrayLists.add(list); } - public void addLongArrayListRef(LongArrayList list, SoftReference listRef) - { - this.longArrayLists.add(list); - this.longArrayRefLists.add(listRef); - } + public void addLongArrayListRef(LongArrayList list) { this.longArrayLists.add(list); } @@ -100,7 +107,6 @@ public class PhantomArrayListCheckout implements AutoCloseable public ArrayList getAllByteArrays() { return this.byteArrayLists; } public ArrayList getAllShortArrays() { return this.shortArrayLists; } public ArrayList getAllLongArrays() { return this.longArrayLists; } - public ArrayList> getAllLongArrayRefs() { return this.longArrayRefLists; } @@ -109,10 +115,7 @@ public class PhantomArrayListCheckout implements AutoCloseable //================// @Override - public void close() - { - this.owningPool.returnCheckout(this); - } + public void close() { this.owningPool.returnCheckout(this); } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListParent.java b/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListParent.java index d4e0bd4d4..50dca02a7 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListParent.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListParent.java @@ -28,7 +28,7 @@ public abstract class PhantomArrayListParent implements AutoCloseable * It's recommended to set this as null after the child's constructor * finishes to show the pooled arrays have all been accessed */ - protected PhantomArrayListCheckout pooledArraysCheckout; + protected final PhantomArrayListCheckout pooledArraysCheckout; @@ -57,19 +57,7 @@ public abstract class PhantomArrayListParent implements AutoCloseable //================// @Override - public void close() //throws Exception - { - try - { - this.phantomReference.clear(); - PhantomArrayListCheckout checkout = this.phantomArrayListPool.phantomRefToCheckout.remove(this.phantomReference); - this.phantomArrayListPool.returnCheckout(checkout); - } - catch (Exception e) - { - LOGGER.error("Unable to close Phantom Array", e); - } - } + public void close() { this.phantomArrayListPool.returnParentPhantomRef(this.phantomReference); } } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListPool.java b/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListPool.java index 0f0d7728a..1a5a33349 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListPool.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListPool.java @@ -4,7 +4,6 @@ import com.seibel.distanthorizons.core.api.internal.ClientApi; import com.seibel.distanthorizons.core.config.Config; import com.seibel.distanthorizons.core.logging.DhLoggerBuilder; import com.seibel.distanthorizons.core.logging.f3.F3Screen; -import com.seibel.distanthorizons.core.util.LodUtil; import com.seibel.distanthorizons.core.util.ThreadUtil; import com.seibel.distanthorizons.core.util.objects.Pair; import com.seibel.distanthorizons.coreapi.ModInfo; @@ -14,7 +13,9 @@ import it.unimi.dsi.fastutil.longs.LongArrayList; import it.unimi.dsi.fastutil.shorts.ShortArrayList; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import java.lang.ref.PhantomReference; import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; @@ -25,8 +26,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.BiConsumer; -import java.util.function.Supplier; /** * DH uses a lot of potentially large arrays of {@link Byte}s and {@link Long}s. @@ -46,6 +45,12 @@ import java.util.function.Supplier; * This is less efficient since it may allow a lot of additional arrays to * be created while we wait for the garbage collector to run, but * does prevent any leaks from {@link PhantomArrayListParent} that weren't closed. + * + *

+ * Use Notes:
+ * If possible all checkouts for a given pool should be the same size, + * since {@link PhantomArrayListCheckout}'s are shared, getting the same size checkout each time + * prevents accidentally returning a larger checkout than necessary, which wastes memory. */ public class PhantomArrayListPool { @@ -83,10 +88,7 @@ public class PhantomArrayListPool public final ReferenceQueue phantomRefQueue = new ReferenceQueue<>(); - - private final ConcurrentLinkedQueue pooledByteArrays = new ConcurrentLinkedQueue<>(); - private final ConcurrentLinkedQueue pooledShortArrays = new ConcurrentLinkedQueue<>(); - private final ConcurrentLinkedQueue> pooledLongArrays = new ConcurrentLinkedQueue<>(); + private final ConcurrentLinkedQueue> pooledCheckoutsRefs = new ConcurrentLinkedQueue<>(); /** counts how many byte arrays have been created by this pool */ private final AtomicInteger totalByteArrayCountRef = new AtomicInteger(0); @@ -102,6 +104,15 @@ public class PhantomArrayListPool /** used for debugging, represents an estimate for how many bytes the long[] pool contains */ private long lastLongPoolSizeInBytes = -1; + /** used for debugging, represents an estimate for how many byte[]'s are currently in this pool*/ + private int lastBytePoolCount = 0; + /** used for debugging, represents an estimate for how many short[]'s are currently in this pool*/ + private int lastShortPoolCount = 0; + /** used for debugging, represents an estimate for how many long[]'s are currently in this pool*/ + private int lastLongPoolCount = 0; + /** used for debugging, represents an estimate for how many checkouts are currently in this pool*/ + private int lastCheckoutPoolCount = 0; + /** For pools backed by {@link SoftReference}'s we may need to decrease the size when elements are garbage collected */ private boolean clearLastRefPoolSizes = false; @@ -132,30 +143,77 @@ public class PhantomArrayListPool // get checkout // //==============// + /** + * If possible all checkouts for a given pool should be the same size, + * since {@link PhantomArrayListCheckout}'s are shared, returning the same size + * prevents accidentally returning a larger checkout than necessary, which wastes memory. + */ public PhantomArrayListCheckout checkoutArrays(int byteArrayCount, int shortArrayCount, int longArrayCount) { - PhantomArrayListCheckout checkout = new PhantomArrayListCheckout(this); + PhantomArrayListCheckout checkout = null; + while (checkout == null) + { + SoftReference checkoutRef = this.pooledCheckoutsRefs.poll(); + if (checkoutRef == null) + { + // pool is empty, create new checkout + checkout = new PhantomArrayListCheckout(this); + } + else + { + checkout = checkoutRef.get(); + if (checkout != null) + { + // use pooled checkout + } + else + { + // this reference is pointing to null, + // the checkout must have been garbage collected, + // that means we don't have enough memory + if (!lowMemoryWarningLogged) + { + lowMemoryWarningLogged = true; + + // orange text + String message = "\u00A76" + "Distant Horizons: Insufficient memory detected." + "\u00A7r \n" + + "This may cause stuttering or crashing. \n" + + "Potential causes: \n" + + "1. your allocated memory isn't high enough \n" + + "2. your DH CPU preset is too high \n" + + "3. your DH quality preset is too high"; + + LOGGER.warn(message); + if (Config.Common.Logging.Warning.showPoolInsufficientMemoryWarning.get()) + { + ClientApi.INSTANCE.showChatMessageNextFrame(message); + } + } + + this.clearLastRefPoolSizes = true; + } + } + } + + + // get any missing arrays // byte - for (int i = 0; i < byteArrayCount; i++) + for (int i = checkout.getByteArrayCount(); i < byteArrayCount; i++) { - checkout.addByteArrayList(getPooledArray(this.pooledByteArrays, () -> this.createEmptyByteArrayList())); + checkout.addByteArrayList(this.createEmptyByteArrayList()); } // short - for (int i = 0; i < shortArrayCount; i++) + for (int i = checkout.getShortArrayCount(); i < shortArrayCount; i++) { - checkout.addShortArrayList(getPooledArray(this.pooledShortArrays, () -> this.createEmptyShortArrayList())); + checkout.addShortArrayList(this.createEmptyShortArrayList()); } // long - for (int i = 0; i < longArrayCount; i++) + for (int i = checkout.getLongArrayCount(); i < longArrayCount; i++) { - addRefPooledArray( - this.pooledLongArrays, - this::createEmptyLongArrayList, - this::onLongArrayListGarbageCollected, - checkout::addLongArrayListRef); + checkout.addLongArrayListRef(this.createEmptyLongArrayList()); } return checkout; @@ -184,91 +242,6 @@ public class PhantomArrayListPool } - // garbage collection handlers // - - /** should only happen if Java doesn't have enough memory */ - private void onLongArrayListGarbageCollected() - { - this.clearLastRefPoolSizes = true; - this.totalLongArrayCountRef.getAndDecrement(); - } - - - // internal pool handlers // - - private static > T getPooledArray(ConcurrentLinkedQueue pool, Supplier emptyArrayCreatorFunc) - { - T array = pool.poll(); - if (array != null) - { - array.clear(); - return array; - } - else - { - // no pooled sources exist - return emptyArrayCreatorFunc.get(); - } - } - private static > void addRefPooledArray( - ConcurrentLinkedQueue> arrayPool, - Supplier emptyArrayCreatorFunc, - Runnable arrayGarbageCollectedFunc, - BiConsumer> putArrayFunc) - { - T array = null; - SoftReference arrayRef = arrayPool.poll(); - - // find the first non-null pooled array - while (arrayRef != null && array == null) - { - array = arrayRef.get(); - if (array == null) - { - // this reference is pointing to null, - // the array must have been garbage collected, - // that means we don't have enough memory - if (!lowMemoryWarningLogged) - { - lowMemoryWarningLogged = true; - - // orange text - String message = "\u00A76" + "Distant Horizons: Insufficient memory detected." + "\u00A7r \n" + - "This may cause stuttering or crashing. \n" + - "Either: your allocated memory isn't high enough, \n" + - "your DH CPU preset is too high, or your DH quality preset is too high."; - - LOGGER.warn(message); - if (Config.Common.Logging.Warning.showPoolInsufficientMemoryWarning.get()) - { - ClientApi.INSTANCE.showChatMessageNextFrame(message); - } - } - - arrayGarbageCollectedFunc.run(); - - // try the next reference - arrayRef = arrayPool.poll(); - } - } - - - if (array != null) - { - LodUtil.assertTrue(arrayRef != null, "How did we get an array without it's reference?"); - array.clear(); - } - else - { - // no pooled sources exist - array = emptyArrayCreatorFunc.get(); - arrayRef = new SoftReference<>(array); - } - - putArrayFunc.accept(array, arrayRef); - } - - //==================// // phantom recovery // @@ -405,21 +378,29 @@ public class PhantomArrayListPool // return checkout // //=================// - public void returnCheckout(PhantomArrayListCheckout checkout) + public void returnParentPhantomRef(@NotNull PhantomReference parentRef) { + try + { + parentRef.clear(); + // will be null if the this parent has already been returned + PhantomArrayListCheckout checkout = this.phantomRefToCheckout.remove(parentRef); + this.returnCheckout(checkout); + } + catch (Exception e) + { + LOGGER.error("Unable to close Phantom Array, error: ["+e.getMessage()+"].", e); + } + } + public void returnCheckout(@Nullable PhantomArrayListCheckout checkout) + { if (checkout == null) { throw new IllegalArgumentException("Null phantom checkout, object is being closed multiple times."); } - - // In James' testing pooling the checkout object wasn't necessary - // since it is relatively small and short lived, thus - // the GC can handle quickly discarding it. - - this.pooledByteArrays.addAll(checkout.getAllByteArrays()); - this.pooledShortArrays.addAll(checkout.getAllShortArrays()); - this.pooledLongArrays.addAll(checkout.getAllLongArrayRefs()); + SoftReference checkoutRef = checkout.ownerSoftReference; + this.pooledCheckoutsRefs.add(checkoutRef); //LOGGER.info("Returned ["+checkout.byteArrayLists.size()+"/"+this.pooledByteArrays.size()+"] bytes and ["+checkout.longArrayLists.size()+"/"+this.pooledLongArrays.size()+"] longs.");\ } @@ -444,9 +425,9 @@ public class PhantomArrayListPool totalShortArrayCount += pool.totalShortArrayCountRef.get(); totalLongArrayCount += pool.totalLongArrayCountRef.get(); - pooledByteArraySize += pool.pooledByteArrays.size(); - pooledShortArraySize += pool.pooledShortArrays.size(); - pooledLongArraySize += pool.pooledLongArrays.size(); + pooledByteArraySize += pool.lastBytePoolCount; + pooledShortArraySize += pool.lastShortPoolCount; + pooledLongArraySize += pool.lastLongPoolCount; lastBytePoolSizeInBytes += pool.lastBytePoolSizeInBytes; lastShortPoolSizeInBytes += pool.lastShortPoolSizeInBytes; @@ -475,7 +456,7 @@ public class PhantomArrayListPool addDebugMenuStringsToList(messageList, this.name, this.totalByteArrayCountRef.get(), this.totalShortArrayCountRef.get(), this.totalLongArrayCountRef.get(), - this.pooledByteArrays.size(), this.pooledShortArrays.size(), this.pooledLongArrays.size(), + this.lastBytePoolCount, this.lastShortPoolCount, this.lastLongPoolCount, this.lastBytePoolSizeInBytes, this.lastShortPoolSizeInBytes, this.lastLongPoolSizeInBytes ); } @@ -529,25 +510,60 @@ public class PhantomArrayListPool */ public void recalculateSizeForDebugging() { - // byte - long bytePoolByteSize = estimateMemoryUsage(this.pooledByteArrays, Byte.BYTES); - this.lastBytePoolSizeInBytes = Math.max(bytePoolByteSize, this.lastBytePoolSizeInBytes); + long bytePoolByteSize = 0; + long shortPoolByteSize = 0; + long longPoolByteSize = 0; - // short - long shortPoolByteSize = estimateMemoryUsage(this.pooledShortArrays, Short.BYTES); - this.lastShortPoolSizeInBytes = Math.max(shortPoolByteSize, this.lastShortPoolSizeInBytes); + int bytePoolCount = 0; + int shortPoolCount = 0; + int longPoolCount = 0; - // long + + // checkouts // + for (SoftReference pooledCheckoutRef : this.pooledCheckoutsRefs) + { + PhantomArrayListCheckout pooledCheckout = pooledCheckoutRef.get(); + if (pooledCheckout == null) + { + continue; + } + + bytePoolByteSize += estimateMemoryUsage(pooledCheckout.getAllByteArrays(), Byte.BYTES); + bytePoolCount += pooledCheckout.getAllByteArrays().size(); + shortPoolByteSize += estimateMemoryUsage(pooledCheckout.getAllShortArrays(), Short.BYTES); + shortPoolCount += pooledCheckout.getAllShortArrays().size(); + longPoolByteSize += estimateMemoryUsage(pooledCheckout.getAllLongArrays(), Long.BYTES); + longPoolCount += pooledCheckout.getAllLongArrays().size(); + } + + + // clear old values if something was garbage collected if (this.clearLastRefPoolSizes) { + this.lastBytePoolSizeInBytes = 0; + this.lastShortPoolSizeInBytes = 0; this.lastLongPoolSizeInBytes = 0; this.clearLastRefPoolSizes = false; } - long longPoolByteSize = estimateRefMemoryUsage(this.pooledLongArrays, Long.BYTES); + + this.lastCheckoutPoolCount = this.pooledCheckoutsRefs.size(); + + // byte // + // math.max is used since the pool should only grow until a soft reference is freed, + // and it's easier to understand if this constantly grows instead of jumping around + this.lastBytePoolSizeInBytes = Math.max(bytePoolByteSize, this.lastBytePoolSizeInBytes); + this.lastBytePoolCount = bytePoolCount; + + // short // + this.lastShortPoolSizeInBytes = Math.max(shortPoolByteSize, this.lastShortPoolSizeInBytes); + this.lastShortPoolCount = shortPoolCount; + + // long // this.lastLongPoolSizeInBytes = Math.max(longPoolByteSize, this.lastLongPoolSizeInBytes); + this.lastLongPoolCount = longPoolCount; } - private static > long estimateMemoryUsage(ConcurrentLinkedQueue pool, long elementSizeInBytes) + private static > long estimateMemoryUsage(Iterable pool, long elementSizeInBytes) { long longByteSize = 0; for (T array : pool) diff --git a/core/src/main/java/com/seibel/distanthorizons/core/sql/dto/FullDataSourceV2DTO.java b/core/src/main/java/com/seibel/distanthorizons/core/sql/dto/FullDataSourceV2DTO.java index 15abd48e5..c1f09d091 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/sql/dto/FullDataSourceV2DTO.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/sql/dto/FullDataSourceV2DTO.java @@ -123,8 +123,6 @@ public class FullDataSourceV2DTO this.compressedColumnGenStepByteArray = this.pooledArraysCheckout.getByteArray(1, 0); this.compressedWorldCompressionModeByteArray = this.pooledArraysCheckout.getByteArray(2, 0); this.compressedMappingByteArray = this.pooledArraysCheckout.getByteArray(3, 0); - - this.pooledArraysCheckout = null; } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/util/RenderDataPointReducingList.java b/core/src/main/java/com/seibel/distanthorizons/core/util/RenderDataPointReducingList.java index dbbd5f3f0..dbdd50189 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/util/RenderDataPointReducingList.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/util/RenderDataPointReducingList.java @@ -137,8 +137,6 @@ public class RenderDataPointReducingList extends PhantomArrayListParent this.sortingArray = this.pooledArraysCheckout.getShortArray(0, 0); if (ASSERTS) this.checkLinks(); - this.pooledArraysCheckout = null; - return; } @@ -154,8 +152,6 @@ public class RenderDataPointReducingList extends PhantomArrayListParent java.util.Arrays.fill(this.links.elements(), DEFAULT_LINKS); this.data = this.pooledArraysCheckout.getLongArray(1, arrayCapacity); - this.pooledArraysCheckout = null; - int sizeWithoutAir = 0; for (int index = 0; index < size; index++) {