[
https://issues.apache.org/jira/browse/GROOVY-7705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15141785#comment-15141785
]
Jason Winnebeck commented on GROOVY-7705:
-----------------------------------------
[~shils], awesome work on this, I have used 2.4.6-SNAPSHOT today from JFrog
repo and confirmed via bytecode analysis through Fernflower that the fix does
work to create direct accessors. For my code there is a substantial performance
improvement: a performance test suite goes from 44s to 40s. Within that test
suite the average page load time for the most directly affected page drops from
419ms avg to 321ms avg. Note this is with the "make all fields public"
workaround applied for the most used classes in our framework. If I make the
fields private again, the time goes from 2 seconds+ average page load times
(total test suite on order of minutes) down to the 321ms, so for those who
haven't applied this workaround and are impacted this could be a major win.
Even for us who have applied the workaround, the patch catches the less used
classes for a 10% overall, or 25% improvement for the page load times.
One thing I had issue with in 2.4.6 is that static compiler gave a handful of
errors I had to fix, I filed GROOVY-7753. In other cases the static compiler
assumes more specific types than it used to (for example method where I
returned Map<String, Object> complains because Groovy now thinks it returns
Map<String, Comparable>).
> CompileStatic closure accessing "thisObject" private field: setProperty
> instead of synthetic method
> ---------------------------------------------------------------------------------------------------
>
> Key: GROOVY-7705
> URL: https://issues.apache.org/jira/browse/GROOVY-7705
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 2.4.5
> Reporter: Jason Winnebeck
> Assignee: Shil Sinha
> Fix For: 2.4.6
>
>
> When a closure sets the private field of a class, it does it via a
> ScriptBytecodeAdapter.setGroovyObjectProperty on the closure object itself,
> whereas when getting a private field of the class it uses a synthetic
> "pfaccess$0" method.
> Source:
> {code:title=Source}
> @CompileStatic
> class SetFromClosure {
> private int x
> void doIt() {
> def closure = {
> x = 5
> println x
> }
> closure()
> }
> }
> {code}
> Decompiled result of closure class (via Fernflower in IntelliJ):
> {code}
> class SetFromClosure$_doIt_closure1 extends Closure implements
> GeneratedClosure {
> public SetFromClosure$_doIt_closure1(Object _outerInstance, Object
> _thisObject) {
> super(_outerInstance, _thisObject);
> }
> public Object doCall(Object it) {
> byte var2 = 5;
> ScriptBytecodeAdapter.setGroovyObjectProperty(Integer.valueOf(var2),
> SetFromClosure$_doIt_closure1.class, this, (String)"x");
> DefaultGroovyMethods.println((SetFromClosure)this.getThisObject(),
> Integer.valueOf(SetFromClosure.pfaccess$0((SetFromClosure)this.getThisObject())));
> return null;
> }
> public Object call(Object args) {
> return this.doCall(args);
> }
> public Object call() {
> return this.doCall((Object)null);
> }
> public Object doCall() {
> return this.doCall((Object)null);
> }
> }
> {code}
> Workaround is to remove "private" keyword on the variable declaration, which
> causes Groovy to generate public getter and setter. In that mode, the static
> compiler does generate direct calls to both the getter and the setter. In one
> example within my project the performance speedup was 10x with this
> workaround. The only thing I notice in the workaround that is odd is that
> instead of normal Java casts used, ScriptBytecodeAdapter.castToType is used
> on "getThisObject" before calling the getters or setters.
> This issue could possibly related to GROOVY-6825, but in that case it's an
> inner class, and the issue description (currently) is not clear enough to see
> if it's also generating a setProperty call.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
