Copilot commented on code in PR #2233:
URL: https://github.com/apache/groovy/pull/2233#discussion_r2105761391
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java:
##########
@@ -327,8 +328,15 @@ public static Object fromCache(CacheableCallSite callSite,
Class<?> sender, Stri
}
if (mhw.isCanSetTarget() && (callSite.getTarget() !=
mhw.getTargetMethodHandle()) && (mhw.getLatestHitCount() >
INDY_OPTIMIZE_THRESHOLD)) {
- callSite.setTarget(mhw.getTargetMethodHandle());
- if (LOG_ENABLED) LOG.info("call site target set, preparing outside
invocation");
+ if (callSite.getFallbackRound().get() > INDY_FALLBACK_CUTOFF) {
+ if (callSite.getTarget() != callSite.getDefaultTarget()) {
+ // reset the call site target to default forever to avoid
JIT deoptimization storm further
+ callSite.setTarget(callSite.getDefaultTarget());
+ }
+ } else {
+ callSite.setTarget(mhw.getTargetMethodHandle());
+ if (LOG_ENABLED) LOG.info("call site target set, preparing
outside invocation");
Review Comment:
[nitpick] Consider refactoring or adding inline comments to clarify the
nested conditional logic for better readability and maintainability.
```suggestion
if (shouldResetCallSiteTarget(callSite)) {
resetCallSiteTarget(callSite);
} else {
updateCallSiteTarget(callSite, mhw);
```
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/CacheableCallSite.java:
##########
@@ -119,6 +120,11 @@ public long incrementFallbackCount() {
public void resetFallbackCount() {
fallbackCount.set(0);
+ fallbackRound.incrementAndGet();
Review Comment:
Consider adding JavaDoc or inline comments to explain the role of
fallbackRound in the fallback reset process to enhance code clarity.
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java:
##########
@@ -48,8 +48,9 @@
* methods and classes.
*/
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);
+ private static final long INDY_OPTIMIZE_THRESHOLD =
SystemUtil.getLongSafe("groovy.indy.optimize.threshold", 1_000L);
+ private static final long INDY_FALLBACK_THRESHOLD =
SystemUtil.getLongSafe("groovy.indy.fallback.threshold", 1_000L);
+ private static final long INDY_FALLBACK_CUTOFF =
SystemUtil.getLongSafe("groovy.indy.fallback.cutoff", 100L);
Review Comment:
Consider adding a comment that explains the purpose of INDY_FALLBACK_CUTOFF
and how it impacts the fallback mechanism to aid future maintainers.
--
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: notifications-unsubscr...@groovy.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
