[#GROOVY-2437] Groovyc ant task compiles closures twice

groovy

Groovyc ant task compiles closures twice

Created: 22/Dec/07 12:17 PM Updated: 15/Oct/08 10:00 AM

Component/s:

Ant integration

Affects Version/s:

1.5.1

Fix Version/s:

1.6-rc-1

Time Tracking:

Not Specified

File Attachments:	1. Test1'.java (20 kB) 2. test3.zip (1 kB)
Environment:	Java: 1.6.0_03 Ant: 1.6.5 WindowsXP SP2

Description

« Hide

Part 1:

If I build my software with the groovyc ant task I see that closures are compiled twice. I've attached a test project. You'll see that closure1 .. closure4 are generated, but there are only 2 closures in the groovy class.

If I remove the java class, the problem is gone.

I've also decompiled the constructor:

public Test1()
{
	super();
	class1 = Test1.class$0 == null ? ( Test1.class$0 = Test1.class$( "app.Test1" ) ) : Test1.class$0;
	class2 = Test1.class$groovy$lang$MetaClass == null ? ( Test1.class$groovy$lang$MetaClass = Test1.class$( "groovy.lang.MetaClass" ) ) : Test1.class$groovy$lang$MetaClass;
	this.index = new _closure1( this, this );
	this.search = new _closure2( this, this );
	this.metaClass = (MetaClass)ScriptBytecodeAdapter.castToType( 
		(MetaClass)ScriptBytecodeAdapter.castToType( 
			ScriptBytecodeAdapter.invokeStaticMethodN( 
				class1, 
				Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter == null ? 
					( Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter = Test1.class$( "org.codehaus.groovy.runtime.ScriptBytecodeAdapter" ) ) : 
					Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter, 
				"initMetaClass", 
				new Object[] { this } 
			), 
			class2 
		), 
		class2 
	);
	this.index = new _closure3( this, this );
	this.search = new _closure4( this, this );
	this.metaClass = (MetaClass)ScriptBytecodeAdapter.castToType( 
		(MetaClass)ScriptBytecodeAdapter.castToType( 
			ScriptBytecodeAdapter.invokeStaticMethodN( 
				class1, 
				Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter == null ? 
					( Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter = Test1.class$( "org.codehaus.groovy.runtime.ScriptBytecodeAdapter" ) ) : 
					Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter, 
				"initMetaClass", 
				new Object[] { this } 
			), 
			class2 
		), 
		class2 
	);
	return;
}

Here you see that the index, search and metaClass fields are set twice. 4 closures are instantiated.

Part 2:

Furthermore you see that the (MetaClass)ScriptBytecodeAdapter.castToType() method is called twice. The looks a bit overkill. This is done like this in a lot of other methods of the class too: getMetaClass, invokeMethod, getProperty, setProperty. This has negative influence on performance.

Part 3:

In the generated getters and setters you will also see that 2 classes are retrieved into 2 local variables although they are not used. This has negative influence on performance too.

I've attached the complete decompiled class.

Description

Part 1: If I build my software with the groovyc ant task I see that closures are compiled twice. I've attached a test project. You'll see that closure1 .. closure4 are generated, but there are only 2 closures in the groovy class. If I remove the java class, the problem is gone. I've also decompiled the constructor:

public Test1()
{
	super();
	class1 = Test1.class$0 == null ? ( Test1.class$0 = Test1.class$( "app.Test1" ) ) : Test1.class$0;
	class2 = Test1.class$groovy$lang$MetaClass == null ? ( Test1.class$groovy$lang$MetaClass = Test1.class$( "groovy.lang.MetaClass" ) ) : Test1.class$groovy$lang$MetaClass;
	this.index = new _closure1( this, this );
	this.search = new _closure2( this, this );
	this.metaClass = (MetaClass)ScriptBytecodeAdapter.castToType( 
		(MetaClass)ScriptBytecodeAdapter.castToType( 
			ScriptBytecodeAdapter.invokeStaticMethodN( 
				class1, 
				Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter == null ? 
					( Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter = Test1.class$( "org.codehaus.groovy.runtime.ScriptBytecodeAdapter" ) ) : 
					Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter, 
				"initMetaClass", 
				new Object[] { this } 
			), 
			class2 
		), 
		class2 
	);
	this.index = new _closure3( this, this );
	this.search = new _closure4( this, this );
	this.metaClass = (MetaClass)ScriptBytecodeAdapter.castToType( 
		(MetaClass)ScriptBytecodeAdapter.castToType( 
			ScriptBytecodeAdapter.invokeStaticMethodN( 
				class1, 
				Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter == null ? 
					( Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter = Test1.class$( "org.codehaus.groovy.runtime.ScriptBytecodeAdapter" ) ) : 
					Test1.class$org$codehaus$groovy$runtime$ScriptBytecodeAdapter, 
				"initMetaClass", 
				new Object[] { this } 
			), 
			class2 
		), 
		class2 
	);
	return;
}

Here you see that the index, search and metaClass fields are set twice. 4 closures are instantiated. Part 2: Furthermore you see that the (MetaClass)ScriptBytecodeAdapter.castToType() method is called twice. The looks a bit overkill. This is done like this in a lot of other methods of the class too: getMetaClass, invokeMethod, getProperty, setProperty. This has negative influence on performance. Part 3: In the generated getters and setters you will also see that 2 classes are retrieved into 2 local variables although they are not used. This has negative influence on performance too. I've attached the complete decompiled class.

Show »

All

Comments

Work Log

Change History

Sort Order:

[ Permalink | « Hide ]

René de Bloois added a comment - 22/Dec/07 12:20 PM

Forgot to attach the test project

[ Permalink | « Hide ]

Guillaume Laforge added a comment - 22/Dec/07 01:11 PM

I haven't looked at your test project so far, but are you able to retrieve four different Closure classes through reflection?
Decompiler have to generate some Java source code, but don't necessarily reflect 100% the reality.

[ Permalink | « Hide ]

René de Bloois added a comment - 22/Dec/07 06:29 PM

Looking at the time of day (or night

), I do not fully understand your question regarding the reflection (just came back from a birthday party).

In the attached java file I added the constructor twice, once without bytecode, once with the bytecodes. So you can look at the bytecodes too.

Furthermore, after compiling, I get 4 closure classes on the filesystem:

Test1$_closure1.class
Test1$_closure2.class
Test1$_closure3.class
Test1$_closure4.class

[ Permalink | « Hide ]

Guillaume Laforge added a comment - 23/Dec/07 12:09 PM

Jochen, can you double-check this?

[ Permalink | « Hide ]

Jochen Theodorou added a comment - 03/Jan/08 03:38 PM

should be fixed now

[ Permalink | « Hide ]

René de Bloois added a comment - 05/Jan/08 07:51 AM

I verified it and part 1 and 2 of this issue are solved now. Although minor, part 3 still exists:

Class class1 = Test1.class$0 == null ? ( Test1.class$0 = Test1.class$( "app.Test1" ) ) : Test1.class$0;
Class class2 = Test1.class$groovy$lang$MetaClass == null ? ( Test1.class$groovy$lang$MetaClass = Test1.class$( "groovy.lang.MetaClass" ) ) : Test1.class$groovy$lang$MetaClass;

But most of the time class2 is not used, and half of the time class1 is not used. In generated getters and setters they are never used.

For each call to a groovy method you get unused executions of:

getstatic app.Test1.class$0
ifnonnull
...
getstatic app.Test1.class$0
dup
astore_1
pop

('dup' and 'pop' are questionable by itself)

But, we can also wait till java 1.4 is dropped (when is that?).
Then we don't need this anymore.
Or a 'target' option could be added to the groovy compiler.

[ Permalink | « Hide ]

Paul King added a comment - 17/Feb/08 04:02 PM

given that parts 1 and 2 are solved, downgrading this to minor

[ Permalink | « Hide ]

Jochen Theodorou added a comment - 15/Oct/08 10:00 AM

This issue is partially fixed in beta1 for simple getter and setter. In general you still have the class prelude in many cases where you don't need need it. But that is not really a bug. Also removing that requires a reachability test in the compiler for bytecode variables... This improvement will come, but not soon.