This is an automated email from the ASF dual-hosted git repository. blackdrag pushed a commit to branch feature/Cache_changes in repository https://gitbox.apache.org/repos/asf/groovy.git
commit cb74b15efd1209417fe4300ab35abdb4822db18a Author: Jochen Theodorou <[email protected]> AuthorDate: Thu May 21 09:27:31 2026 +0200 GROOVY-12023: further fixes for multi threading and fixing some minor issues --- .../groovy/vmplugin/v8/CacheableCallSite.java | 135 ++++++++++++--------- .../codehaus/groovy/vmplugin/v8/IndyInterface.java | 35 ++---- .../groovy/vmplugin/v8/MethodHandleWrapper.java | 23 ++-- .../v8/IndyInterfaceCallSiteTargetTest.groovy | 5 +- 4 files changed, 111 insertions(+), 87 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/CacheableCallSite.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/CacheableCallSite.java index 57d726e34f..191ec1ca71 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/CacheableCallSite.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/CacheableCallSite.java @@ -23,12 +23,12 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.invoke.MutableCallSite; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; import java.util.Arrays; import java.util.LinkedHashMap; import java.util.Map; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.atomic.AtomicLong; import java.util.function.UnaryOperator; import java.util.logging.Level; @@ -66,6 +66,7 @@ import org.codehaus.groovy.runtime.memoize.MemoizeCache; * @since 3.0.0 */ public class CacheableCallSite extends MutableCallSite { + private static final Logger LOGGER = Logger.getLogger(CacheableCallSite.class.getName()); private static final int CACHE_SIZE = SystemUtil.getIntegerSafe("groovy.indy.callsite.cache.size", 8); private static final float LOAD_FACTOR = 0.75f; private static final int INITIAL_CAPACITY = (int) Math.ceil(CACHE_SIZE / LOAD_FACTOR) + 1; @@ -87,6 +88,7 @@ public class CacheableCallSite extends MutableCallSite { * <b>Concurrency:</b> {@code volatile} ensures that updates to the PIC chain are immediately * visible to all threads during high-speed dispatch. */ + @SuppressWarnings("java:S3077") private volatile MethodHandle picChain; /** @@ -104,7 +106,7 @@ public class CacheableCallSite extends MutableCallSite { */ private volatile int picCount; private final Map<Object, SoftReference<MethodHandleWrapper>> lruCache = - new LinkedHashMap<Object, SoftReference<MethodHandleWrapper>>(INITIAL_CAPACITY, LOAD_FACTOR, true) { + new LinkedHashMap<>(INITIAL_CAPACITY, LOAD_FACTOR, true) { @Serial private static final long serialVersionUID = 7785958879964294463L; /** @@ -123,7 +125,7 @@ public class CacheableCallSite extends MutableCallSite { * Creates a cacheable call site for the supplied type and lookup context. * * @param type the call-site type - * @param lookup the lookup used to unreflect targets + * @param lookup the lookup used to un-reflect targets */ public CacheableCallSite(MethodType type, MethodHandles.Lookup lookup) { super(type); @@ -137,7 +139,7 @@ public class CacheableCallSite extends MutableCallSite { * @param key the receiver cache key * @return the cached wrapper, or {@code null} if not found or not MRU */ - public MethodHandleWrapper get(Object key) { + MethodHandleWrapper get(Object key) { MRUEntry entry = mruEntry; if (entry != null && entry.key == key) { return entry.wrapper; @@ -156,19 +158,21 @@ public class CacheableCallSite extends MutableCallSite { * @param sender the caller class * @return the cached or newly created wrapper */ - public MethodHandleWrapper getAndPut(Object key, MemoizeCache.ValueProvider<? super Object, ? extends MethodHandleWrapper> valueProvider, Class<?> sender) { + MethodHandleWrapper getAndPut(Object key, MemoizeCache.ValueProvider<? super Object, ? extends MethodHandleWrapper> valueProvider, Class<?> sender) { MethodHandleWrapper result = null; SoftReference<MethodHandleWrapper> resultSoftReference; synchronized (lruCache) { resultSoftReference = lruCache.get(key); if (null != resultSoftReference) { result = resultSoftReference.get(); - if (null == result) removeAllStaleEntriesOfLruCache(); + if (null == result) { + lruCache.remove(key); + } } if (null == result) { result = valueProvider.provide(key); - resultSoftReference = new SoftReference<>(result); + resultSoftReference = new SoftReferenceWithKey(key, result, REFERENCE_QUEUE, lruCache); lruCache.put(key, resultSoftReference); } } @@ -211,13 +215,15 @@ public class CacheableCallSite extends MutableCallSite { * @param mhw the wrapper to cache * @return the previously cached wrapper, or {@code null} if none existed */ - public MethodHandleWrapper put(Object key, MethodHandleWrapper mhw) { + MethodHandleWrapper put(Object key, MethodHandleWrapper mhw) { synchronized (lruCache) { - final SoftReference<MethodHandleWrapper> methodHandleWrapperSoftReference; - methodHandleWrapperSoftReference = lruCache.put(key, new SoftReference<>(mhw)); + final SoftReference<MethodHandleWrapper> methodHandleWrapperSoftReference = + lruCache.put(key, new SoftReferenceWithKey(key, mhw, REFERENCE_QUEUE, lruCache)); if (null == methodHandleWrapperSoftReference) return null; final MethodHandleWrapper methodHandleWrapper = methodHandleWrapperSoftReference.get(); - if (null == methodHandleWrapper) removeAllStaleEntriesOfLruCache(); + if (null == methodHandleWrapper) { + lruCache.remove(key); + } return methodHandleWrapper; } } @@ -248,55 +254,32 @@ public class CacheableCallSite extends MutableCallSite { } } - /** - * Resets the PIC and targets to the default state if not already reset. - * <p> - * <b>Concurrency:</b> Synchronizes on {@code this} to safely reset the - * target and PIC metadata when the site becomes too megamorphic. - */ - public void resetPicAndTarget() { - synchronized (this) { - if (getTarget() != defaultTarget) { - setTarget(defaultTarget); - resetFallbackCount(); - } - } - } - - public synchronized void recordInPic(Object key) { - if (picCount < picKeys.length) { - picKeys[picCount++] = key; - } - } - /** * Checks if a receiver shape is already present in the PIC. * <p> - * <b>Concurrency:</b> Marked as {@code synchronized} to prevent reading inconsistent state - * while the PIC is being updated. + * <b>Concurrency:</b> Lock-free read of the PIC metadata. The volatile read of {@link #picCount} + * ensures visibility of prior writes to {@link #picKeys} by the same or another thread. * * @param key the receiver cache key * @return {@code true} if the key is in the PIC */ - public synchronized boolean picIncludes(Object key) { - for (int i = 0; i < picCount; i++) { - if (picKeys[i] == key) return true; + public boolean picInsertIfMissing(Object key) { + int count = picCount; + for (int i = 0; i < count; i++) { + if (picKeys[i] == key) return false; } - return false; + return true; } public MethodHandle getPicChain() { return picChain; } - public void setPicChain(MethodHandle picChain) { - this.picChain = picChain; - } - - public synchronized int getPicCount() { + public int getPicCount() { return picCount; } + @javax.annotation.concurrent.Immutable private static final class MRUEntry { final Object key; final MethodHandleWrapper wrapper; @@ -306,12 +289,50 @@ public class CacheableCallSite extends MutableCallSite { } } - private void removeAllStaleEntriesOfLruCache() { - CACHE_CLEANER_QUEUE.offer(() -> { - synchronized (lruCache) { - lruCache.values().removeIf(v -> null == v.get()); + private static class SoftReferenceWithKey extends SoftReference<MethodHandleWrapper> { + private final Object key; + private final Map<Object, SoftReference<MethodHandleWrapper>> cache; + + SoftReferenceWithKey(Object key, MethodHandleWrapper referent, ReferenceQueue<MethodHandleWrapper> q, Map<Object, SoftReference<MethodHandleWrapper>> cache) { + super(referent, q); + this.key = key; + this.cache = cache; + } + + void clean() { + synchronized (cache) { + if (cache.get(key) == this) { + cache.remove(key); + } + } + } + } + + /** + * Atomically resets the call site to the default target when the fallback count + * exceeds the given threshold. This provides a concurrency-safe, double-checked + * locking reset for mega-morphic call sites. + * <p> + * <b>Concurrency:</b> Synchronizes on {@code this} to atomically reset + * the target and associated PIC metadata, preventing redundant resets + * when multiple threads detect a high fallback count simultaneously. + * + * @param defaultTarget the default target handle to restore + * @param threshold the fallback count threshold that triggers a reset + * @param fallbackCount the current fallback count + * @return {@code true} if the target was reset to the default + */ + public boolean tryResetToDefaultTarget(MethodHandle defaultTarget, long threshold, long fallbackCount) { + if (fallbackCount > threshold && getTarget() != defaultTarget) { + synchronized (this) { + if (getTarget() != defaultTarget) { + setTarget(defaultTarget); + resetFallbackCount(); + return true; + } } - }); + } + return false; } /** @@ -391,16 +412,22 @@ public class CacheableCallSite extends MutableCallSite { return lookup; } - private static final BlockingQueue<Runnable> CACHE_CLEANER_QUEUE = new LinkedBlockingQueue<>(); + private static final ReferenceQueue<MethodHandleWrapper> REFERENCE_QUEUE = new ReferenceQueue<>(); static { Thread cacheCleaner = new Thread(() -> { while (true) { try { - CACHE_CLEANER_QUEUE.take().run(); - } catch (Throwable ignore) { + Reference<? extends MethodHandleWrapper> ref = REFERENCE_QUEUE.remove(); + if (ref instanceof SoftReferenceWithKey sRef) { + sRef.clean(); + } + } catch (@SuppressWarnings("java:S1181") Throwable throwable) { + if (throwable instanceof InterruptedException) { + Thread.currentThread().interrupt(); + } Logger logger = Logger.getLogger(MethodHandles.lookup().lookupClass().getName()); - if (logger.isLoggable(Level.FINEST)) { - logger.finest(DefaultGroovyMethods.asString(ignore)); + if (LOGGER.isLoggable(Level.FINEST)) { + logger.finest(DefaultGroovyMethods.asString(throwable)); } } } diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java index 982b088e80..bb7f182383 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java @@ -18,6 +18,7 @@ */ package org.codehaus.groovy.vmplugin.v8; +import edu.umd.cs.findbugs.annotations.NonNull; import groovy.lang.GroovySystem; import org.apache.groovy.util.SystemUtil; import org.codehaus.groovy.GroovyBugError; @@ -205,6 +206,7 @@ public class IndyInterface { * <b>Concurrency:</b> {@code volatile} ensures that global invalidations are immediately * visible to all threads across the JVM, causing them to fall back from JIT-optimized handles. */ + @SuppressWarnings("java:S3077") protected static volatile SwitchPoint switchPoint = new SwitchPoint(); static { @@ -371,9 +373,9 @@ public class IndyInterface { } private static final Object NULL_KEY = new Object(); - private static final ClassValue<Object> STATIC_KEYS = new ClassValue<Object>() { + private static final ClassValue<Object> STATIC_KEYS = new ClassValue<>() { @Override - protected Object computeValue(Class<?> type) { + protected Object computeValue(@NonNull Class<?> type) { return new Object(); } }; @@ -388,16 +390,14 @@ public class IndyInterface { MethodHandleWrapper mhw = callSite.get(receiverKey); if (mhw != null) { mhw.incrementLatestHitCount(); - if (mhw.isCanSetTarget() && (callSite.getTarget() != mhw.getTargetMethodHandle())) { - if (mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD) { - optimizeCallSite(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, arguments, mhw); - } + if (mhw.isCanSetTarget() && (callSite.getTarget() != mhw.getTargetMethodHandle()) && mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD && callSite.picInsertIfMissing(receiverKey)) { + optimizeCallSite(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, arguments, receiverKey, mhw); } return mhw.getCachedMethodHandle(); } FallbackSupplier fallbackSupplier = new FallbackSupplier(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments); - mhw = callSite.getAndPut(receiverKey, (theKey) -> { + mhw = callSite.getAndPut(receiverKey, theKey -> { MethodHandleWrapper fallback = fallbackSupplier.get(); if (fallback.isCanSetTarget()) return fallback; return NULL_METHOD_HANDLE_WRAPPER; @@ -418,21 +418,20 @@ public class IndyInterface { } } - if (mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD) { - optimizeCallSite(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, arguments, mhw); + if (mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD && callSite.picInsertIfMissing(receiverKey)) { + optimizeCallSite(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, arguments, receiverKey, mhw); } } return mhw.getCachedMethodHandle(); } - private static void optimizeCallSite(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[] arguments, MethodHandleWrapper mhw) { + private static void optimizeCallSite(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[] arguments, Object receiverKey, MethodHandleWrapper mhw) { if (callSite.getFallbackRound().get() > INDY_FALLBACK_CUTOFF) { if (callSite.getTarget() != callSite.getDefaultTarget()) { callSite.setTarget(callSite.getDefaultTarget()); } } else { - Object receiverKey = receiverCacheKey(arguments[0]); callSite.maybeUpdatePic(receiverKey, picChain -> { Selector selector = Selector.getSelector(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, arguments); selector.skipSwitchPoint = true; @@ -466,18 +465,8 @@ public class IndyInterface { MethodHandle defaultTarget = callSite.getDefaultTarget(); long fallbackCount = callSite.incrementFallbackCount(); - if ((fallbackCount > INDY_FALLBACK_THRESHOLD) && (callSite.getTarget() != defaultTarget)) { - /** - * <b>Concurrency:</b> Synchronizes on the {@code callSite} to safely reset the - * target and PIC metadata when the site becomes too megamorphic. - */ - synchronized (callSite) { - if (callSite.getTarget() != defaultTarget) { - callSite.setTarget(defaultTarget); - if (LOG_ENABLED) LOG.info("call site target reset to default, preparing outside invocation"); - callSite.resetFallbackCount(); - } - } + if (callSite.tryResetToDefaultTarget(defaultTarget, INDY_FALLBACK_THRESHOLD, fallbackCount)) { + if (LOG_ENABLED) LOG.info("call site target reset to default, preparing outside invocation"); } if (callSite.getTarget() == defaultTarget) { diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java index 71c3cb5dbb..34b59ac061 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java @@ -21,7 +21,7 @@ package org.codehaus.groovy.vmplugin.v8; import groovy.lang.MetaMethod; import java.lang.invoke.MethodHandle; -import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.LongAdder; /** * Wrap method handles @@ -33,7 +33,7 @@ class MethodHandleWrapper { private final MethodHandle targetMethodHandle; private final MetaMethod method; private final boolean canSetTarget; - private final AtomicLong latestHitCount = new AtomicLong(0); + private final LongAdder latestHitCount = new LongAdder(); /** * Creates a wrapper for the cached and relink targets of a meta method. @@ -88,18 +88,25 @@ class MethodHandleWrapper { /** * Increments the hit count for the latest inline-cache hit. - * - * @return the updated hit count */ - public long incrementLatestHitCount() { - return latestHitCount.incrementAndGet(); + public void incrementLatestHitCount() { + latestHitCount.increment(); } /** * Resets the latest-hit counter. */ public void resetLatestHitCount() { - latestHitCount.set(0); + latestHitCount.reset(); + } + + /** + * Adds the specified value to the latest-hit counter. + * + * @param value the value to add + */ + public void addLatestHitCount(long value) { + latestHitCount.add(value); } /** @@ -108,7 +115,7 @@ class MethodHandleWrapper { * @return the current latest-hit counter */ public long getLatestHitCount() { - return latestHitCount.get(); + return latestHitCount.sum(); } /** diff --git a/src/test/groovy/org/codehaus/groovy/vmplugin/v8/IndyInterfaceCallSiteTargetTest.groovy b/src/test/groovy/org/codehaus/groovy/vmplugin/v8/IndyInterfaceCallSiteTargetTest.groovy index 3655958308..b2f5f80e4b 100644 --- a/src/test/groovy/org/codehaus/groovy/vmplugin/v8/IndyInterfaceCallSiteTargetTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/vmplugin/v8/IndyInterfaceCallSiteTargetTest.groovy @@ -424,7 +424,8 @@ final class IndyInterfaceCallSiteTargetTest { private static void primeLatestHitCount(CacheableCallSite callSite, Object receiver, MethodHandleWrapper wrapper, long value) { assertSame(wrapper, callSite.getAndPut(IndyInterface.receiverCacheKey(receiver), { wrapper }, IndyInterfaceCallSiteTargetTest)) - [email protected](value) + wrapper.resetLatestHitCount() + wrapper.addLatestHitCount(value) } private static void assertFallbackCutoffLeavesDefaultTarget(boolean startAwayFromDefaultTarget) { @@ -452,7 +453,7 @@ final class IndyInterfaceCallSiteTargetTest { assertSame(wrapper.cachedMethodHandle, methodHandle) assertSame(callSite.defaultTarget, callSite.target) - assertEquals(0L, wrapper.latestHitCount) + assertEquals(0L, wrapper.getLatestHitCount()) } private static MethodHandleWrapper requireCachedWrapper(CacheableCallSite callSite, Object receiver) {
