GROOVY-10307: Improve documentation for thread safety design decisions

- Document best-effort invalidation strategy in invalidateSwitchPoints():
  concurrent threads may briefly reinstall optimized targets, but method
  handle guards (SAME_MC, SAME_CLASS) ensure correctness by forcing fallback
- Keep clearCache() volatile field access outside synchronized block for
  consistency with getAndPut() which also accesses it outside the lock;
  volatile provides sufficient visibility guarantees for this field
- Add comprehensive Javadoc for INDY_SWITCHPOINT_GUARD explaining
  performance implications of enabling the global switchpoint guard
- Document WeakReference cleanup behavior in ALL_CALL_SITES and
  registerCallSite() to clarify garbage collection semantics
- Simplify inline comment in setGuards() with detailed Javadoc on field

Author: jamesfredley

src/main/java/org/codehaus/groovy/vmplugin/v8/CacheableCallSite.java

@@ -128,17 +128,22 @@ public void setFallbackTarget(MethodHandle 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
+        // Clear the latest hit reference using volatile semantics
         latestHitMethodHandleWrapperSoftReference = null;
 
-        // Clear the LRU cache
+        // Clear the LRU cache under lock to synchronize with concurrent getAndPut operations
         synchronized (lruCache) {
             lruCache.clear();
         }
 
-        // Reset fallback count
+        // Reset fallback count (atomic operation, doesn't need to be in sync block)
         fallbackCount.set(0);
     }
 }

src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java

@@ -182,6 +182,13 @@ public static CallType fromCallSiteName(String callSiteName) {
     /**
      * 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);
 
@@ -191,6 +198,10 @@ public static CallType fromCallSiteName(String callSiteName) {
 
     /**
      * 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));
@@ -211,8 +222,12 @@ protected static void invalidateSwitchPoints() {
             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
+        // 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 -> {
             CacheableCallSite cs = ref.get();
             if (cs == null) {

src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java

@@ -126,9 +126,17 @@ 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.
+     * 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");
 
@@ -948,14 +956,7 @@ public void setGuards(Object receiver) {
                 }
             }
 
-            // 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
+            // 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");