[ 
https://issues.apache.org/jira/browse/GROOVY-12191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18099443#comment-18099443
 ] 

ASF GitHub Bot commented on GROOVY-12191:
-----------------------------------------

daniellansun commented on code in PR #2736:
URL: https://github.com/apache/groovy/pull/2736#discussion_r3657316553


##########
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java:
##########
@@ -215,29 +216,86 @@ public int getOrderNumber() {
     }
 
     /**
-     * Shared switch point invalidated when metaclass state changes.
+     * Legacy process-wide SwitchPoint retained for binary compatibility only.
+     * <p>
+     * <strong>Behavioral change in 6.0 (GROOVY-12191):</strong> MetaClass 
changes
+     * are scoped per {@link org.codehaus.groovy.reflection.ClassInfo}. This 
field
+     * is <em>not</em> the MOP guard on linked call sites and is <em>not</em>
+     * rotated on per-class MetaClass changes. It is rotated only when
+     * {@link #invalidateSwitchPoints()} runs — i.e. category enter/leave and
+     * {@code VMPlugin.invalidateCallSites()} — so external observers of this
+     * field still see those bulk events. Guarding a site on this field alone
+     * will <strong>silently miss</strong> type-scoped MetaClass invalidations;
+     * migrate to {@link IndyInvalidation#guardWithMopSwitchPoints}.
+     *
+     * @see IndyInvalidation
+     * @deprecated since 6.0.0 — use {@link 
IndyInvalidation#guardWithMopSwitchPoints};
+     *             this field is not the call-site MOP guard and is not 
rotated on
+     *             per-class MetaClass changes.
      */
-    protected static SwitchPoint switchPoint = new SwitchPoint();
+    @Deprecated(since = "6.0.0", forRemoval = false)
+    protected static volatile SwitchPoint switchPoint = new SwitchPoint();

Review Comment:
   Agreed — thank you.
   
   `org.codehaus.groovy.vmplugin.*` is internal-by-intent. Keeping a dead 
process-wide `SwitchPoint` only for binary compatibility was weaker than a 
clean removal: after scoping, the field was no longer the MOP guard and was 
rotated only on bulk paths, which was easy to misread.
   
   **Action taken**
   
   - Removed `protected static SwitchPoint switchPoint` entirely.
   - `invalidateSwitchPoints()` now only bulk-retires per-class domains via 
`IndyInvalidation.invalidateCategory()` (no legacy field rotation, no 
`volatile`, no extra lock).
   - Docs/`package-info` updated; tests no longer assert on the removed field.
   
   The earlier `volatile` was only there to make concurrent bulk rotation of 
that legacy field observable; with the field gone, that concern disappears.
   





> Scope indy SwitchPoint invalidation
> -----------------------------------
>
>                 Key: GROOVY-12191
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12191
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Priority: Major
>             Fix For: 6.0.0-beta-1
>
>
> h3. Problem
> With invokedynamic enabled (default since Groovy 4), linked MOP call sites 
> were guarded by a *single process-wide* {{SwitchPoint}} 
> ({{{}IndyInterface.switchPoint{}}}).
> Any MetaClass registry change or category enter/leave invalidated that switch 
> point, so *every* linked site fell back and re-linked — including sites whose 
> receiver type was unrelated.
> That global invalidation is expensive when MetaClass churn is common (e.g. 
> ExpandoMetaClass / mixins on startup or per-request paths, Grails-like 
> patterns). Unrelated hot monomorphic sites pay re-link and JIT deopt cost 
> they should not.
> h3. Goal
> Keep linked call sites optimized unless the *relevant* MetaClass state for 
> that site actually changed.
> h3. Approach
> One SwitchPoint domain {*}per class{*}, stored on {{{}ClassInfo{}}}:
>  * MetaClass change for type {{T}} retires {{{}T{}}}'s SwitchPoint *and* 
> those of loaded subtypes / implementors (hierarchy fan-out).
>  * Unrelated types keep their SwitchPoints; their call sites stay optimized.
>  * Category enter/leave (and {{{}VMPlugin.invalidateCallSites(){}}}) 
> bulk-retire *all* loaded class SwitchPoints so sites re-link under the new 
> category state. There is *no* second category SwitchPoint on the hot path.
>  * Linked handles always install a *single* class-domain guard via 
> {{IndyInvalidation.guardWithMopSwitchPoints(...)}} — same monomorphic guard 
> shape as before, without global deopt on unrelated MetaClass churn.
> Final classes short-circuit hierarchy fan-out (no full {{ClassInfo}} scan). 
> Non-final types batch retirements with {{{}SwitchPoint.invalidateAll{}}}.
> h3. Invalidation map
> ||Event||What is retired||
> |MetaClass change for type {{T}} (registry / 
> {{{}ClassInfo.incVersion{}}})|{{T}} + loaded subtypes / implementors|
> |Category enter/leave, {{invalidateCallSites()}}|All loaded class 
> SwitchPoints (bulk)|
> |Unattributed MetaClass registry event|All loaded class SwitchPoints (bulk)|
> |First MetaClass *install* on a class|Version bump only (no linked sites 
> yet); replacement / clear retires that class's SwitchPoint|
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to