From fdb3c0da5e2824a84a24c92ebeec737427332abf Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 20 Sep 2023 11:50:41 +0200 Subject: [PATCH 1/6] Rewrote hotEntries map to be more compact and drop entries in background --- .../vallang/type/TypeFactory.java | 2 +- .../util/WeakReferenceHashConsingMap.java | 103 ++++++++++++++---- 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/type/TypeFactory.java b/src/main/java/io/usethesource/vallang/type/TypeFactory.java index 2686363c..cbe516c0 100644 --- a/src/main/java/io/usethesource/vallang/type/TypeFactory.java +++ b/src/main/java/io/usethesource/vallang/type/TypeFactory.java @@ -62,7 +62,7 @@ public class TypeFactory { /** * Caches all types to implement canonicalization */ - private final HashConsingMap fCache = new WeakReferenceHashConsingMap<>(8*1024, (int)TimeUnit.MINUTES.toSeconds(30)); + private final HashConsingMap fCache = new WeakReferenceHashConsingMap<>(32*1024, (int)TimeUnit.MINUTES.toSeconds(30)); private volatile @MonotonicNonNull TypeValues typeValues; // lazy initialize private static class InstanceHolder { diff --git a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java index bab89c27..d01badc1 100644 --- a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java +++ b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java @@ -13,10 +13,13 @@ package io.usethesource.vallang.util; import java.lang.ref.WeakReference; -import java.time.Duration; +import java.util.HashMap; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.Interner; import com.github.benmanes.caffeine.cache.Scheduler; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -35,12 +38,12 @@ public class WeakReferenceHashConsingMap implements H * So that we can use them as keys in Caffeine. * Caffeine doesn't support weakKeys with regular equality contract. */ - private static class WeakReferenceWrap extends WeakReference { + private static class WeakReferenceWrap extends WeakReference { private final int hash; - public WeakReferenceWrap(T referent) { + public WeakReferenceWrap(T referent, int hash) { super(referent); - this.hash = referent.hashCode(); + this.hash = hash; } @Override @@ -50,7 +53,7 @@ public int hashCode() { @Override public boolean equals(@Nullable Object obj) { - if (obj instanceof WeakReferenceWrap) { + if (obj instanceof WeakReferenceWrap) { WeakReferenceWrap wrappedObj = (WeakReferenceWrap) obj; if (wrappedObj.hash == hash) { Object self = super.get(); @@ -64,11 +67,34 @@ public boolean equals(@Nullable Object obj) { return false; } } + + private static class HotEntry { + private final T value; + private final int hash; + private volatile int lastUsed; + + HotEntry(T value, int hash) { + this.value = value; + this.hash = hash; + lastUsed = SecondsTicker.current(); + } + } /** - * We keep the most recently used entries in a strong cache for quick access + * We keep the most recently used entries in a simple open addressing map for quick access + * In case of hash collisions, the entry is overwritten, which doesn't matter too much + * The cleanup happens in a side thread. + * + * Note that even though everything is using atomic operations, + * threads can have different views on the contents of the hotEntries array. + * This is not a problem, as it acts like a thread local LRU in that case. + * + * The coldEntries is the only map that should keep the state consistent across threads. */ - private final Cache hotEntries; + private final HotEntry[] hotEntries; + private final int mask; + private final int expireAfter; + /** * All entries are also stored in a WeakReference, this helps with clearing memory * if entries are not referenced anymore @@ -80,39 +106,70 @@ public WeakReferenceHashConsingMap() { this(16, (int)TimeUnit.MINUTES.toSeconds(30)); } - private static long simulateNanoTicks() { - return TimeUnit.SECONDS.toNanos(SecondsTicker.current()); - - } - public WeakReferenceHashConsingMap(int size, int demoteAfterSeconds) { - hotEntries = Caffeine.newBuilder() - .ticker(WeakReferenceHashConsingMap::simulateNanoTicks) - .expireAfterAccess(Duration.ofSeconds(demoteAfterSeconds)) - .scheduler(Scheduler.systemScheduler()) - .initialCapacity(size) - .build(); + if (size <= 0) { + throw new IllegalArgumentException("Size should be a positive number"); + } + // size should be a power of two + size = Integer.highestOneBit(size - 1) << 1; + hotEntries = new HotEntry[size]; + this.mask = size - 1; + this.expireAfter = demoteAfterSeconds; coldEntries = Caffeine.newBuilder() .weakValues() .initialCapacity(size) + .executor(ForkJoinPool.commonPool()) .scheduler(Scheduler.systemScheduler()) .build(); + + cleanup(); } + private void cleanup() { + try { + final int now = SecondsTicker.current(); + final var hotEntries = this.hotEntries; + for (int i = 0; i < hotEntries.length; i++) { + var entry = hotEntries[i]; + if (entry != null && (now - entry.lastUsed >= this.expireAfter)) { + hotEntries[i] = null; + } + } + } finally { + CompletableFuture + .delayedExecutor(Math.max(1, this.expireAfter / 10), TimeUnit.SECONDS) + .execute(this::cleanup); + } + } + + private static int improve(int hash) { + // xxhash avalanching phase + hash ^= hash >>> 15; + hash *= 0x85EBCA77; + hash ^= hash >>> 13; + hash *= 0xC2B2AE3D; + return hash ^ (hash >>> 16); + } + @Override public T get(T key) { - T hot = hotEntries.getIfPresent(key); - if (hot != null) { - return hot; + final int hash = key.hashCode(); + final int hotIndex = improve(hash) & mask; + final var hotEntries = this.hotEntries; + var hotEntry = hotEntries[hotIndex]; + if (hotEntry != null && hotEntry.hash == hash && hotEntry.value.equals(key)) { + hotEntry.lastUsed = SecondsTicker.current(); + return hotEntry.value; } - T cold = coldEntries.get(new WeakReferenceWrap<>(key), k -> key); + + var cold = coldEntries.get(new WeakReferenceWrap<>(key, hash), k -> key); // after this, either we just put it in cold, or we got an old version back from // cold, so we are gonna put it back in the hot entries // note: the possible race between multiple puts is no problem, because // the coldEntries get will have made sure it will be of the same instance - hotEntries.put(cold, cold); + hotEntries[hotIndex] = new HotEntry<>(cold, hash); return cold; } From ffaa58f4fadd058ae21eedf950a2aa3c07e3cb05 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 20 Sep 2023 12:15:51 +0200 Subject: [PATCH 2/6] Replaced cold map with concurrent map --- .../util/WeakReferenceHashConsingMap.java | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java index d01badc1..3df6448c 100644 --- a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java +++ b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java @@ -12,15 +12,15 @@ */ package io.usethesource.vallang.util; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; -import java.util.HashMap; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; -import com.github.benmanes.caffeine.cache.Interner; -import com.github.benmanes.caffeine.cache.Scheduler; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -41,8 +41,8 @@ public class WeakReferenceHashConsingMap implements H private static class WeakReferenceWrap extends WeakReference { private final int hash; - public WeakReferenceWrap(T referent, int hash) { - super(referent); + public WeakReferenceWrap(T referent, int hash, ReferenceQueue cleared) { + super(referent, cleared); this.hash = hash; } @@ -53,6 +53,9 @@ public int hashCode() { @Override public boolean equals(@Nullable Object obj) { + if (obj == this) { + return true; + } if (obj instanceof WeakReferenceWrap) { WeakReferenceWrap wrappedObj = (WeakReferenceWrap) obj; if (wrappedObj.hash == hash) { @@ -99,7 +102,8 @@ private static class HotEntry { * All entries are also stored in a WeakReference, this helps with clearing memory * if entries are not referenced anymore */ - private final Cache, T> coldEntries; + private final ReferenceQueue queue = new ReferenceQueue(); + private final Map, WeakReferenceWrap> coldEntries; public WeakReferenceHashConsingMap() { @@ -116,12 +120,7 @@ public WeakReferenceHashConsingMap(int size, int demoteAfterSeconds) { this.mask = size - 1; this.expireAfter = demoteAfterSeconds; - coldEntries = Caffeine.newBuilder() - .weakValues() - .initialCapacity(size) - .executor(ForkJoinPool.commonPool()) - .scheduler(Scheduler.systemScheduler()) - .build(); + coldEntries = new ConcurrentHashMap<>(size); cleanup(); } @@ -137,6 +136,17 @@ private void cleanup() { hotEntries[i] = null; } } + List> toCleanup = new ArrayList<>(); + synchronized(queue) { + Reference cleared; + while ((cleared = queue.poll()) != null) { + if (cleared instanceof WeakReferenceWrap) { + toCleanup.add((WeakReferenceWrap) cleared); + } + } + } + System.err.println("Clearing up:" + toCleanup.size()); + toCleanup.forEach(coldEntries::remove); } finally { CompletableFuture .delayedExecutor(Math.max(1, this.expireAfter / 10), TimeUnit.SECONDS) @@ -164,13 +174,18 @@ public T get(T key) { return hotEntry.value; } - var cold = coldEntries.get(new WeakReferenceWrap<>(key, hash), k -> key); + T result = null; + while (result == null) { + var keyWrapped = new WeakReferenceWrap<>(key, hash, queue); + var winRace = coldEntries.putIfAbsent(keyWrapped, keyWrapped); + result = winRace == null ? key : winRace.get(); + } // after this, either we just put it in cold, or we got an old version back from // cold, so we are gonna put it back in the hot entries // note: the possible race between multiple puts is no problem, because // the coldEntries get will have made sure it will be of the same instance - hotEntries[hotIndex] = new HotEntry<>(cold, hash); - return cold; + hotEntries[hotIndex] = new HotEntry<>(result, hash); + return result; } } From 64934947657b156c64a3eb1e848fd32a67ad8c51 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 20 Sep 2023 12:36:08 +0200 Subject: [PATCH 3/6] Avoid allocating weak references --- .../util/WeakReferenceHashConsingMap.java | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java index 3df6448c..832cfe1c 100644 --- a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java +++ b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java @@ -82,6 +82,31 @@ private static class HotEntry { lastUsed = SecondsTicker.current(); } } + + private static class LookupKey { + private final T value; + private final int hash; + + LookupKey(T value, int hash) { + this.value = value; + this.hash = hash; + } + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(@Nullable Object obj) { + if (obj instanceof WeakReferenceWrap) { + var actual = (WeakReferenceWrap)obj; + return actual.hash == hash && value.equals(actual.get()); + } + return false; + } + + } + /** * We keep the most recently used entries in a simple open addressing map for quick access @@ -103,7 +128,7 @@ private static class HotEntry { * if entries are not referenced anymore */ private final ReferenceQueue queue = new ReferenceQueue(); - private final Map, WeakReferenceWrap> coldEntries; + private final Map> coldEntries; public WeakReferenceHashConsingMap() { @@ -145,7 +170,6 @@ private void cleanup() { } } } - System.err.println("Clearing up:" + toCleanup.size()); toCleanup.forEach(coldEntries::remove); } finally { CompletableFuture @@ -174,11 +198,18 @@ public T get(T key) { return hotEntry.value; } - T result = null; - while (result == null) { + // fast path, it' already in cold, + // we avoid making a new weak reference just for search + var fastGet = coldEntries.get(new LookupKey<>(key, hash)); + T result = fastGet == null ? null : fastGet.get(); + if (result == null) { + // we create a weak reference once + // and try to win putting it in var keyWrapped = new WeakReferenceWrap<>(key, hash, queue); - var winRace = coldEntries.putIfAbsent(keyWrapped, keyWrapped); - result = winRace == null ? key : winRace.get(); + while (result == null) { + var winRace = coldEntries.putIfAbsent(keyWrapped, keyWrapped); + result = winRace == null ? key : winRace.get(); + } } // after this, either we just put it in cold, or we got an old version back from // cold, so we are gonna put it back in the hot entries From 4fc4e45c5b6e5791721f23a3af583b15af2bb585 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 20 Sep 2023 12:52:57 +0200 Subject: [PATCH 4/6] Made cleanup not capture this --- .../vallang/util/WeakReferenceHashConsingMap.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java index 832cfe1c..c8c861e6 100644 --- a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java +++ b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java @@ -147,17 +147,17 @@ public WeakReferenceHashConsingMap(int size, int demoteAfterSeconds) { coldEntries = new ConcurrentHashMap<>(size); - cleanup(); + cleanup(demoteAfterSeconds, hotEntries, coldEntries, queue); } - private void cleanup() { + private static <@Nullable T extends Object> void cleanup(int demoteAfterSeconds, HotEntry[] hotEntries, Map> coldEntries, + ReferenceQueue queue) { try { final int now = SecondsTicker.current(); - final var hotEntries = this.hotEntries; for (int i = 0; i < hotEntries.length; i++) { var entry = hotEntries[i]; - if (entry != null && (now - entry.lastUsed >= this.expireAfter)) { + if (entry != null && (now - entry.lastUsed >= demoteAfterSeconds)) { hotEntries[i] = null; } } @@ -173,8 +173,8 @@ private void cleanup() { toCleanup.forEach(coldEntries::remove); } finally { CompletableFuture - .delayedExecutor(Math.max(1, this.expireAfter / 10), TimeUnit.SECONDS) - .execute(this::cleanup); + .delayedExecutor(Math.max(1, demoteAfterSeconds / 10), TimeUnit.SECONDS) + .execute(() -> cleanup(demoteAfterSeconds, hotEntries, coldEntries, queue)); } } From ecc8709cca66d3e49efd773181dd6a91e14dc6c5 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 20 Sep 2023 12:56:07 +0200 Subject: [PATCH 5/6] Proper attribution of mix function --- .../usethesource/vallang/util/WeakReferenceHashConsingMap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java index c8c861e6..ff8189de 100644 --- a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java +++ b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java @@ -179,7 +179,7 @@ public WeakReferenceHashConsingMap(int size, int demoteAfterSeconds) { } private static int improve(int hash) { - // xxhash avalanching phase + // base on XXH32_avalanche from xxHash (BSD2 license, Yann Collet) hash ^= hash >>> 15; hash *= 0x85EBCA77; hash ^= hash >>> 13; From 2c2fb5131f56466186277c31a7977760d6752248 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Wed, 20 Sep 2023 14:43:41 +0200 Subject: [PATCH 6/6] Improved null check annotations --- .../usethesource/vallang/util/WeakReferenceHashConsingMap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java index ff8189de..cb35a0ce 100644 --- a/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java +++ b/src/main/java/io/usethesource/vallang/util/WeakReferenceHashConsingMap.java @@ -151,7 +151,7 @@ public WeakReferenceHashConsingMap(int size, int demoteAfterSeconds) { } - private static <@Nullable T extends Object> void cleanup(int demoteAfterSeconds, HotEntry[] hotEntries, Map> coldEntries, + private static void cleanup(int demoteAfterSeconds, @Nullable HotEntry[] hotEntries, Map> coldEntries, ReferenceQueue queue) { try { final int now = SecondsTicker.current();