Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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,21 @@ 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.
*/
public void clearCache() {
// Clear the latest hit reference
latestHitMethodHandleWrapperSoftReference = null;

// Clear the LRU cache
synchronized (lruCache) {
Comment thread
jamesfredley marked this conversation as resolved.
Outdated
lruCache.clear();
}

// Reset fallback count
fallbackCount.set(0);
}
}
48 changes: 46 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,55 @@ 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.
*/
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.
*/
static void registerCallSite(CacheableCallSite callSite) {
ALL_CALL_SITES.add(new WeakReference<>(callSite));
Comment thread
jamesfredley marked this conversation as resolved.
}

/**
* 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
ALL_CALL_SITES.removeIf(ref -> {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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();
Comment thread
jamesfredley marked this conversation as resolved.
return false;
});
}

/**
Expand Down Expand Up @@ -230,6 +271,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
23 changes: 20 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,13 @@ public abstract class Selector {
*/
private static final CallType[] CALL_TYPE_VALUES = CallType.values();

/**
* Whether to use global SwitchPoint guard for metaclass changes.
* Default is false for better performance in frameworks with frequent metaclass changes.
* Set groovy.indy.switchpoint.guard=true to enable strict metaclass change detection.
Comment thread
jamesfredley marked this conversation as resolved.
Outdated
*/
private static final boolean INDY_SWITCHPOINT_GUARD = SystemUtil.getBooleanSafe("groovy.indy.switchpoint.guard");

/**
* Returns the Selector
*/
Expand Down Expand Up @@ -940,9 +948,18 @@ 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");
// Skip the global switchpoint guard by default.
// The switchpoint causes ALL call sites to fail when ANY metaclass changes.
// In Grails and similar frameworks with frequent metaclass changes, this causes
// massive guard failures and performance degradation.
// The other guards (metaclass identity, class receiver, category) should be
// sufficient, combined with cache invalidation on metaclass changes.
//
// If you need strict metaclass change detection, set groovy.indy.switchpoint.guard=true
Comment thread
jamesfredley marked this conversation as resolved.
Outdated
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