jamesfredley commented on code in PR #2374:
URL: https://github.com/apache/groovy/pull/2374#discussion_r2750297976
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java:
##########
@@ -168,24 +171,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();
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));
+ }
/**
- * 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 -> {
Review Comment:
I tested two versions of the parallel stream change with the two test
application and saw a reduction in performance.
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);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
