Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,26 @@ public MethodHandle getFallbackTarget() {
public void setFallbackTarget(MethodHandle fallbackTarget) {
this.fallbackTarget = fallbackTarget;
}

/**
* Clear the cache entirely. Called when metaclass changes to ensure
* stale method handles are discarded.
* <p>
* The volatile field {@code latestHitMethodHandleWrapperSoftReference} is cleared
* outside the synchronized block to be consistent with {@link #getAndPut}, which
* also accesses this field outside the lock. Volatile provides sufficient visibility
* guarantees for this field.
*/
public void clearCache() {
// Clear the latest hit reference using volatile semantics
latestHitMethodHandleWrapperSoftReference = null;

// Clear the LRU cache under lock to synchronize with concurrent getAndPut operations
synchronized (lruCache) {
lruCache.clear();
}

// Reset fallback count (atomic operation, doesn't need to be in sync block)
fallbackCount.set(0);
}
}
63 changes: 61 additions & 2 deletions src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
import java.lang.invoke.MethodType;
import java.lang.invoke.MutableCallSite;
import java.lang.invoke.SwitchPoint;
import java.lang.ref.WeakReference;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.logging.Level;
Expand All @@ -50,6 +53,13 @@
public class IndyInterface {
private static final long INDY_OPTIMIZE_THRESHOLD = SystemUtil.getLongSafe("groovy.indy.optimize.threshold", 10_000L);
private static final long INDY_FALLBACK_THRESHOLD = SystemUtil.getLongSafe("groovy.indy.fallback.threshold", 10_000L);
/**
* Initial capacity for the call site registry used to track all active call sites
* for cache invalidation when metaclass changes occur. The default of 1024 balances
* memory usage against resize overhead. Tune via {@code groovy.indy.callsite.initial.capacity}
* system property for larger applications.
*/
private static final int INDY_CALLSITE_INITIAL_CAPACITY = SystemUtil.getIntegerSafe("groovy.indy.callsite.initial.capacity", 1024);

/**
* flags for method and property calls
Expand Down Expand Up @@ -168,24 +178,70 @@ public static CallType fromCallSiteName(String callSiteName) {
}

protected static SwitchPoint switchPoint = new SwitchPoint();

/**
* Weak set of all CacheableCallSites. Used to invalidate caches when metaclass changes.
* Uses WeakReferences so call sites can be garbage collected when no longer referenced.
* <p>
* Note: Stale (garbage-collected) WeakReferences are cleaned up during each call to
* {@link #invalidateSwitchPoints()}. In applications with infrequent metaclass changes,
* the set may accumulate some dead references between invalidations. This is acceptable
* because: (1) the memory overhead per dead reference is minimal (~16 bytes), and
* (2) frameworks like Grails that benefit most from this optimization have frequent
* metaclass changes that trigger regular cleanup.
*/
private static final Set<WeakReference<CacheableCallSite>> ALL_CALL_SITES = ConcurrentHashMap.newKeySet(INDY_CALLSITE_INITIAL_CAPACITY);

static {
GroovySystem.getMetaClassRegistry().addMetaClassRegistryChangeEventListener(cmcu -> invalidateSwitchPoints());
}

/**
* Register a call site for cache invalidation when metaclass changes.
* <p>
* Registered call sites are held via WeakReferences and will be automatically
* removed when garbage collected. Cleanup of stale references occurs during
* {@link #invalidateSwitchPoints()}.
*/
static void registerCallSite(CacheableCallSite callSite) {
ALL_CALL_SITES.add(new WeakReference<>(callSite));
}

/**
* Callback for constant metaclass update change
* Callback for constant metaclass update change.
* Invalidates all call site caches to ensure metaclass changes are visible.
*/
protected static void invalidateSwitchPoints() {
if (LOG_ENABLED) {
LOG.info("invalidating switch point");
LOG.info("invalidating switch point and call site caches");
}

synchronized (IndyInterface.class) {
SwitchPoint old = switchPoint;
switchPoint = new SwitchPoint();
SwitchPoint.invalidateAll(new SwitchPoint[]{old});
}

// Invalidate all call site caches and reset targets to default (cache lookup).
// This ensures metaclass changes are visible without using expensive switchpoint guards.
// Note: This is best-effort invalidation. A concurrent thread in fromCache() may briefly
// reinstall an optimized target after we reset it. This is acceptable because the method
// handle guards (SAME_MC, SAME_CLASS) will fail on the next call if the metaclass changed,
// forcing a fallback to selectMethod() which will resolve the correct method.
ALL_CALL_SITES.removeIf(ref -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size of ALL_CALL_SITES could be very big, so it's better to use parallel stream for better performance.

Copy link
Contributor

@daniellansun daniellansun Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When metaclass changes, all registered call sites have their caches cleared

Current implementation will clear all cache no matter whether the changed metaclasses are releated.
Groovy 5 has lookup instance in the CacheableCallSite, it can help us determine which class is using the cache site, maybe we can backport it to Groovy 4:

private final MethodHandles.Lookup lookup;

Copy link
Author

@jamesfredley jamesfredley Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested two versions of the parallel stream change with the two test application and saw a reduction in performance.

Metric Baseline Approach 1 Approach 2
Grails App (steady state) ~141 ms ~167 ms (18% SLOWER) ~158 ms (12% SLOWER)

Approach 1: Parallel Collect + Atomic Swap - 18% SLOWER

// Invalidate all call site caches and reset targets to default (cache lookup)
// This ensures metaclass changes are visible without using expensive switchpoint guards
Set<WeakReference<CacheableCallSite>> liveReferences = ALL_CALL_SITES.parallelStream()
    .filter(ref -> {
        CacheableCallSite cs = ref.get();
        if (cs == null) {
            return false; // Don't keep garbage collected references
        }
        // Reset target to default (fromCache) so next call goes through cache lookup
        MethodHandle defaultTarget = cs.getDefaultTarget();
        if (defaultTarget != null && cs.getTarget() != defaultTarget) {
            cs.setTarget(defaultTarget);
        }
        // Clear the cache so stale method handles are discarded
        cs.clearCache();
        return true; // Keep live references
    })
    .collect(Collectors.toSet());
// Atomic swap - clear and replace with live references only
ALL_CALL_SITES.clear();
ALL_CALL_SITES.addAll(liveReferences);

Approach 2: Parallel ForEach + Sequential RemoveIf - 12% SLOWER
// Invalidate all call site caches in parallel

ALL_CALL_SITES.parallelStream().forEach(ref -> {
    CacheableCallSite cs = ref.get();
    if (cs != null) {
        // Reset target to default (fromCache) so next call goes through cache lookup
        MethodHandle defaultTarget = cs.getDefaultTarget();
        if (defaultTarget != null && cs.getTarget() != defaultTarget) {
            cs.setTarget(defaultTarget);
        }
        // Clear the cache so stale method handles are discarded
        cs.clearCache();
    }
});
// Remove garbage collected references (must be sequential for ConcurrentHashMap.KeySetView)
ALL_CALL_SITES.removeIf(ref -> ref.get() == null);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing with the Groovy 5 caller-specific Lookup pattern is generally positive on the micro benchmarks but 11% slower in the grails test application:

More details: jamesfredley#1

CacheableCallSite cs = ref.get();
if (cs == null) {
return true; // Remove garbage collected references
}
// Reset target to default (fromCache) so next call goes through cache lookup
MethodHandle defaultTarget = cs.getDefaultTarget();
if (defaultTarget != null && cs.getTarget() != defaultTarget) {
cs.setTarget(defaultTarget);
}
// Clear the cache so stale method handles are discarded
cs.clearCache();
return false;
});
}

/**
Expand Down Expand Up @@ -230,6 +286,9 @@ private static CallSite realBootstrap(Lookup caller, String name, int callID, Me
mc.setTarget(mh);
mc.setDefaultTarget(mh);
mc.setFallbackTarget(makeFallBack(mc, sender, name, callID, type, safe, thisCall, spreadCall));

// Register for cache invalidation on metaclass changes
registerCallSite(mc);

return mc;
}
Expand Down
24 changes: 21 additions & 3 deletions src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import groovy.lang.MissingMethodException;
import groovy.transform.Internal;
import org.apache.groovy.runtime.ObjectUtil;
import org.apache.groovy.util.SystemUtil;
import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.reflection.CachedField;
import org.codehaus.groovy.reflection.CachedMethod;
Expand Down Expand Up @@ -124,6 +125,21 @@ public abstract class Selector {
*/
private static final CallType[] CALL_TYPE_VALUES = CallType.values();

/**
* Whether to use a global SwitchPoint guard for metaclass changes.
* <p>
* <strong>Default is {@code false}</strong> to avoid the performance cost of globally
* invalidating all indy call sites whenever <em>any</em> metaclass changes.
* Enabling this will significantly degrade performance in frameworks or applications
* with frequent metaclass changes (for example, Grails), because every such change
* forces all guarded call sites to be re-linked.
* <p>
* Set {@code groovy.indy.switchpoint.guard=true} <strong>only</strong> for specific
* debugging or backward-compatibility scenarios where strict metaclass change
* detection is required, and not for general production use.
*/
private static final boolean INDY_SWITCHPOINT_GUARD = SystemUtil.getBooleanSafe("groovy.indy.switchpoint.guard");

/**
* Returns the Selector
*/
Expand Down Expand Up @@ -940,9 +956,11 @@ public void setGuards(Object receiver) {
}
}

// handle constant metaclass and category changes
handle = switchPoint.guardWithTest(handle, fallback);
if (LOG_ENABLED) LOG.info("added switch point guard");
// Apply global switchpoint guard if enabled (disabled by default for performance)
if (INDY_SWITCHPOINT_GUARD) {
handle = switchPoint.guardWithTest(handle, fallback);
if (LOG_ENABLED) LOG.info("added switch point guard");
}

java.util.function.Predicate<Class<?>> nonFinalOrNullUnsafe = (t) -> {
return !Modifier.isFinal(t.getModifiers())
Expand Down
Loading