[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-529890504 @paulk-asert In theory, it is not a breaking change and its build is green. But we can not test all frameworks based on Groovy, so how about merge the PR for the time being and ask framework maintainers to give it a try? I think it conforms to the meaning of beta releases. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-527198828 @melix Hi Cédric, I am trying to reduce the cost of resolving[1], could you review the PR too? Thanks in advance :-) [1] http://groovy.329449.n5.nabble.com/Performance-of-the-compiler-tt5750989.html [2] https://issues.apache.org/jira/browse/GROOVY-9236 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-527198828 @melix Hi Cédric, I am trying to reduce the cost of resolving[1], could you review the PR too? Thanks in advance :-) Here is the related JIRA issue: GROOVY-9236[2] [1] http://groovy.329449.n5.nabble.com/Performance-of-the-compiler-tt5750989.html [2] https://issues.apache.org/jira/browse/GROOVY-9236 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary
resolving
URL: https://github.com/apache/groovy/pull/998#issuecomment-526739881
I tried to compile the following code to verify whether the times of
guessing class names are reduced. Then I got the result:
https://github.com/apache/groovy/pull/998#issuecomment-526408884
```groovy
import groovy.transform.CompileStatic
import java.util.stream.Collectors
import java.util.stream.Stream
@CompileStatic
void p() {
assert [2, 3, 4] == [1, 2, 3].stream().map(e -> e.plus
1).collect(Collectors.toList())
}
p()
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary
resolving
URL: https://github.com/apache/groovy/pull/998#issuecomment-526739881
I tried to compile the following code to verify whether the times of
guessing class names are reduced.
```groovy
import groovy.transform.CompileStatic
import java.util.stream.Collectors
import java.util.stream.Stream
@CompileStatic
void p() {
assert [2, 3, 4] == [1, 2, 3].stream().map(e -> e.plus
1).collect(Collectors.toList())
}
p()
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-526538718 As `resolveFromStaticInnerClasses` only resolves classes whose name contains `.` and `resolveToOuter` only resolves classes whose name does not contains `.`, no overlap in their target classes to resolve, thus no breaking changes if we change the precedence of resolving IMO. (See the comment in the following code) https://github.com/apache/groovy/blob/master/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L552 https://github.com/apache/groovy/blob/master/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L845 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-526538718 As `resolveFromStaticInnerClasses` only resolves classes whose name contains `.` and `resolveToOuter` only resolves classes whose name does not contains `.`, no overlap in their target classes to resolve, thus no breaking changes in the PR IMO. (See the comment in the following code) https://github.com/apache/groovy/blob/master/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L845 https://github.com/apache/groovy/blob/master/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L552 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-526538718 As `resolveFromStaticInnerClasses` only resolves classes whose name contains `.` and `resolveToOuter` only resolves classes whose name does not contains `.`, no overlap in their target classes to resolve, thus no breaking changes in the PR IMO. (See the comment in the following code) https://github.com/apache/groovy/blob/79dfd8586b33e3c80add6b2ffb26c07ff7a6e8b3/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L552-L558 https://github.com/apache/groovy/blob/79dfd8586b33e3c80add6b2ffb26c07ff7a6e8b3/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L845-L856 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-526538718 As `resolveToOuter` only resolves classes whose name does not contains `.` and `resolveFromStaticInnerClasses` only resolves classes whose name contains `.`, no overlap in their target classes to resolve, thus no breaking changes in the PR IMO. https://github.com/apache/groovy/blob/79dfd8586b33e3c80add6b2ffb26c07ff7a6e8b3/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L552-L558 https://github.com/apache/groovy/blob/79dfd8586b33e3c80add6b2ffb26c07ff7a6e8b3/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L845-L856 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-526408884 The following guessed class lists show the difference caused by the PR. As you can see, lots of classes will be avoided to guess and try to load. * After changes: ``` groovy.lang.GroovyObject$java$util$stream$Collectors groovy.lang.GroovyObject$groovy$transform$CompileStatic groovy.lang.GroovyObject$java$util$stream$Stream groovy.lang.GroovyObject$CompileStatic groovy.lang.GroovyObject$Collectors Script1BeanInfo Script1Customizer Script1$_p_lambda1BeanInfo Script1$_p_lambda1Customizer ``` * Before changes: ``` Script1$_p_lambda1BeanInfo Script1$_p_lambda1Customizer Script1BeanInfo Script1Customizer groovy$transform$CompileStatic groovy.lang.GroovyObject$Collectors groovy.lang.GroovyObject$CompileStatic groovy.lang.GroovyObject$groovy$transform$CompileStatic groovy.lang.GroovyObject$java$util$stream$Collectors groovy.lang.GroovyObject$java$util$stream$Stream groovy.lang.groovy$transform$CompileStatic groovy.lang.java$util$stream$Collectors groovy.lang.java$util$stream$Stream groovy.transform$CompileStatic groovy.util.groovy$transform$CompileStatic groovy.util.java$util$stream$Collectors groovy.util.java$util$stream$Stream java$util$stream$Collectors java$util$stream$Stream java.io.groovy$transform$CompileStatic java.io.java$util$stream$Collectors java.io.java$util$stream$Stream java.lang.groovy$transform$CompileStatic java.lang.java$util$stream$Collectors java.lang.java$util$stream$Stream java.net.groovy$transform$CompileStatic java.net.java$util$stream$Collectors java.net.java$util$stream$Stream java.util$stream$Collectors java.util$stream$Stream java.util.groovy$transform$CompileStatic java.util.java$util$stream$Collectors java.util.java$util$stream$Stream java.util.stream$Collectors java.util.stream$Stream ``` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [groovy] danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving
danielsun1106 edited a comment on issue #998: GROOVY-9236: Avoid unnecessary resolving URL: https://github.com/apache/groovy/pull/998#issuecomment-526408884 The following guessed class lists show the difference caused by the PR. * After changes: ``` groovy.lang.GroovyObject$java$util$stream$Collectors groovy.lang.GroovyObject$groovy$transform$CompileStatic groovy.lang.GroovyObject$java$util$stream$Stream groovy.lang.GroovyObject$CompileStatic groovy.lang.GroovyObject$Collectors Script1BeanInfo Script1Customizer Script1$_p_lambda1BeanInfo Script1$_p_lambda1Customizer ``` * Before changes: ``` Script1$_p_lambda1BeanInfo Script1$_p_lambda1Customizer Script1BeanInfo Script1Customizer groovy$transform$CompileStatic groovy.lang.GroovyObject$Collectors groovy.lang.GroovyObject$CompileStatic groovy.lang.GroovyObject$groovy$transform$CompileStatic groovy.lang.GroovyObject$java$util$stream$Collectors groovy.lang.GroovyObject$java$util$stream$Stream groovy.lang.groovy$transform$CompileStatic groovy.lang.java$util$stream$Collectors groovy.lang.java$util$stream$Stream groovy.transform$CompileStatic groovy.util.groovy$transform$CompileStatic groovy.util.java$util$stream$Collectors groovy.util.java$util$stream$Stream java$util$stream$Collectors java$util$stream$Stream java.io.groovy$transform$CompileStatic java.io.java$util$stream$Collectors java.io.java$util$stream$Stream java.lang.groovy$transform$CompileStatic java.lang.java$util$stream$Collectors java.lang.java$util$stream$Stream java.net.groovy$transform$CompileStatic java.net.java$util$stream$Collectors java.net.java$util$stream$Stream java.util$stream$Collectors java.util$stream$Stream java.util.groovy$transform$CompileStatic java.util.java$util$stream$Collectors java.util.java$util$stream$Stream java.util.stream$Collectors java.util.stream$Stream ``` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
