[
https://issues.apache.org/jira/browse/GROOVY-11609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17943795#comment-17943795
]
ASF GitHub Bot commented on GROOVY-11609:
-----------------------------------------
Copilot commented on code in PR #2186:
URL: https://github.com/apache/groovy/pull/2186#discussion_r2040634434
##########
src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java:
##########
@@ -760,4 +780,66 @@ public void visitVariableExpression(final
VariableExpression expression) {
checkVariableContextAccess(variable, expression);
}
}
+
+ private static class VariableWrapper {
+ private final Variable variable;
+ private int accessedCount;
+
+ VariableWrapper(final Variable variable) {
+ this.variable = variable;
+ }
+ }
+
+ private static class ClassMemberCacheKey {
+ private static final int DEFAULT_HASH = 0;
+ private final String name;
+ private final ClassNode node;
+ private int hash = DEFAULT_HASH;
+
+ ClassMemberCacheKey(final String name, final ClassNode node) {
+ this.name = name;
+ this.node = node;
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj) return true;
+ if (!(obj instanceof ClassMemberCacheKey)) return false;
+ ClassMemberCacheKey that = (ClassMemberCacheKey) obj;
+ return name.equals(that.name) && node.equals(that.node);
+ }
+
+ @Override
+ public int hashCode() {
+ return DEFAULT_HASH != hash ? hash : (hash = Objects.hash(name,
node));
+ }
+ }
+
+ private static class VariableCacheKey {
+ private static final int DEFAULT_HASH = 0;
+ private final String name;
+ private final VariableScope scope;
+ private final boolean inSpecialConstructorCall;
+ private int hash = DEFAULT_HASH;
+
+ VariableCacheKey(final String name, final VariableScope scope, boolean
inSpecialConstructorCall) {
+ this.name = name;
+ this.scope = scope;
+ this.inSpecialConstructorCall = inSpecialConstructorCall;
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj) return true;
+ if (!(obj instanceof VariableCacheKey)) return false;
+ VariableCacheKey that = (VariableCacheKey) obj;
+ return name.equals(that.name) && scope.equals(that.scope) &&
inSpecialConstructorCall == that.inSpecialConstructorCall;
+ }
+
+ @Override
+ public int hashCode() {
+ return DEFAULT_HASH != hash ? hash : (hash = Objects.hash(name,
scope, inSpecialConstructorCall));
+ }
Review Comment:
If the computed hash value is 0, this hash code might be recalculated each
time it's called, which could degrade performance. Consider revising the lazy
hash caching approach, for example by using a non-zero default value.
```suggestion
return hash != DEFAULT_HASH ? hash : (hash = Objects.hash(name,
scope, inSpecialConstructorCall));
```
##########
src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java:
##########
@@ -760,4 +780,66 @@ public void visitVariableExpression(final
VariableExpression expression) {
checkVariableContextAccess(variable, expression);
}
}
+
+ private static class VariableWrapper {
+ private final Variable variable;
+ private int accessedCount;
+
+ VariableWrapper(final Variable variable) {
+ this.variable = variable;
+ }
+ }
+
+ private static class ClassMemberCacheKey {
+ private static final int DEFAULT_HASH = 0;
+ private final String name;
+ private final ClassNode node;
+ private int hash = DEFAULT_HASH;
+
+ ClassMemberCacheKey(final String name, final ClassNode node) {
+ this.name = name;
+ this.node = node;
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj) return true;
+ if (!(obj instanceof ClassMemberCacheKey)) return false;
+ ClassMemberCacheKey that = (ClassMemberCacheKey) obj;
+ return name.equals(that.name) && node.equals(that.node);
+ }
+
+ @Override
+ public int hashCode() {
+ return DEFAULT_HASH != hash ? hash : (hash = Objects.hash(name,
node));
+ }
+ }
+
+ private static class VariableCacheKey {
+ private static final int DEFAULT_HASH = 0;
Review Comment:
If the computed hash value equals 0, the hash code may be recalculated on
every call, potentially impacting performance. Consider ensuring a non-zero
default or a different caching strategy to avoid repeated computation.
```suggestion
return hash != DEFAULT_HASH ? hash : (hash = Objects.hash(name,
node));
}
}
private static class VariableCacheKey {
private static final int DEFAULT_HASH = -1;
```
> Avoid finding same variable/class member repeatedly
> ---------------------------------------------------
>
> Key: GROOVY-11609
> URL: https://issues.apache.org/jira/browse/GROOVY-11609
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> When I debugged GROOVY-4721, I found {{VariableScopeVisitor}} will try to
> find same variable/class member again and again, the finding logic is quite
> complex. Unfortunately, {{VariableScopeVisitor}} is widely used, e.g.
> {{ResolveVisitor}}, {{JavaStubCompilationUnit}} and
> {{TraitASTTransformation}}. So it's better to avoid finding same
> variable/class member repeatedly to gain better performance.
> For example, in the following code, {{a}}, {{i}}, {{j}} will be tried to find
> many times:
> {code}
> class BubbleSort {
> public static void bubbleSort(int[] a) {
> for (int i = 0, n = a.length; i < n - 1; i++) {
> for (int j = 0; j < n - i - 1; j++) {
> if (a[j] > a[j + 1]) {
> int temp = a[j]
> a[j] = a[j + 1]
> a[j + 1] = temp
> }
> }
> }
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
