Bug 221427 - Builds are too frequently full builds for minor changes
Summary: Builds are too frequently full builds for minor changes
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.5.4   Edit
Hardware: PC All
: P2 major (vote)
Target Milestone: 1.6.0 RC1   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-04 20:16 EST by Andrew Clement CLA
Modified: 2008-04-23 16:03 EDT (History)
2 users (show)

See Also:


Attachments
Eclipse 3.2.2 configuration details where the test case is executed (40.10 KB, text/plain)
2008-03-05 11:48 EST, Sebastien Tardif CLA
no flags Details
The full workspace metadata with the code Part 1 - 3 (684.59 KB, application/octet-stream)
2008-03-05 11:55 EST, Sebastien Tardif CLA
no flags Details
The full workspace metadata with the code Part 2 - 3 (1.16 MB, application/x-zip-compressed)
2008-03-05 11:57 EST, Sebastien Tardif CLA
no flags Details
The full workspace metadata with the code Part 3 - 3 (1.16 MB, application/octet-stream)
2008-03-05 11:57 EST, Sebastien Tardif CLA
no flags Details
files to update in an AJDT/AspectJ1.5.4 ajde.jar (20.81 KB, application/octet-stream)
2008-03-06 21:27 EST, Andrew Clement CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Clement CLA 2008-03-04 20:16:50 EST
Reported on the list by Sebastien Tardif.  He has manually upgraded AJDT 1.4.2 (for RAD on Eclipse 3.2) to AspectJ1.5.4 in order to pick up a fix for bug 166580.  He has multiple projects in a dependency graph, some AspectJ, some Java.

With the latest changes he has made to AJDT, he now reports:
====
After more understanding it seems there is nothing more to apply to the
source code from what is already included in 1.5.4 final. So I'm still
stock with the problem.

How do we get debugIncrementalStates to be true to debug the problem?

How do we trace AJDT and components?

Only space and modification of method body avoid full build after this
message: Preparing for build: not going to be incremental because path
change detected (one of classpath/aspectpath/inpath/injars)

Addition or removal of constant or method is triggering batch build.
====
Comment 1 Andrew Clement CLA 2008-03-04 20:37:00 EST
> How do we get debugIncrementalStates to be true to debug the problem?

I don't believe that will help right now

> How do we trace AJDT and components?

Have you turned on the options for max diagnostics in the AJDT Event Trace view?

> Only space and modification of method body avoid full build after this
> message: Preparing for build: not going to be incremental because path
> change detected (one of classpath/aspectpath/inpath/injars)

I don't quite understand what you mean here.  Do you mean it is incremental or it isn't? Your first sentence implies it is incremental (as it avoids full build) but then the attached message says it was not?

> Addition or removal of constant or method is triggering batch build.

I'm not sure which way round you have the project dependencies for your test now - are you modifying a java project and watching an AJDT project dependent on it? Or are you modifying an AJDT project and watching another one further downstream?

Incremental compilation is really complex.  What I would need to progress this is the most simple scenario that you believe should work.  Ideally just two projects, you change one and don't expect the other to full build.  From that kind of scenario I can evolve a test case entirely outside of AJDT and can then work on it - it is just too complicated to debug anything more than a trivial case.

I'm currently playing with a Java project that has an AJDT project dependent on it - but as I mentioned on the list, with the state object for the incremental state of the java project being a black box to me, I may not be able to do anything in the short term. let me know what your test scenario is.
Comment 2 Sebastien Tardif CLA 2008-03-04 21:34:26 EST
When I use maximum output settings in AJDT Event Trace using the options available in the UI I see more output but nothing that seem relevant to this problem.

Space or modification of method body work fine, incremental build is used for the project and dependent projects.

However, if I add or remove constant or method to a class of a project, the project will build incrementally but all its children project will not build incrementally. The parent and direct children are AJDT projects.

We have the problem even when all the projects compatible with AJDT Capability are using the AJDT builder. We use only one output path without the checkbox "Allow output folders for source folders" enabled.
Comment 3 Sebastien Tardif CLA 2008-03-05 11:48:04 EST
Created attachment 91666 [details]
Eclipse 3.2.2 configuration details where the test case is executed

The AJDT plug-ins used in the test case is a merge of AJDT for Eclipse 3.2 version 1.4.2.200705221209 with latest AspectJ and AJDE from 1.5.4 release. So replacing AspectJ 1.5.4.200705211336 that was shipped with latest AJDT release version 1.4.2.200705221209.
Comment 4 Sebastien Tardif CLA 2008-03-05 11:55:55 EST
Created attachment 91668 [details]
The full workspace metadata with the code Part 1 - 3
Comment 5 Sebastien Tardif CLA 2008-03-05 11:57:13 EST
Created attachment 91669 [details]
The full workspace metadata with the code Part 2 - 3 

This is part 2 of 3 of a zip splip done by WinZip
Comment 6 Sebastien Tardif CLA 2008-03-05 11:57:58 EST
Created attachment 91671 [details]
The full workspace metadata with the code Part 3 - 3
Comment 7 Sebastien Tardif CLA 2008-03-05 12:08:59 EST
I have attached the full workspace with the test code and the configuration of Eclipse 3.2.2 I'm using output from Eclipse "About Box". This Eclipse was downloaded from: http://download.eclipse.org/eclipse/downloads/

The only plug-ins added to Eclipse is a merge of AJDT for Eclipse 3.2 version 1.4.2.200705221209 with latest AspectJ and AJDE from 1.5.4 release. So replacing AspectJ 1.5.4.200705211336 that was shipped with latest AJDT release version 1.4.2.200705221209.

The test case cannot be simpler. 

We have only 2 projects both Aspect enabled. No aspect in any projects. Both projects has just one Java class each. If modify the name of a private class variable on the project having the dependent I got what is below. Please note the marker of the bug: 11:59:18 AM Preparing for build: not going to be incremental because path change detected (one of classpath/aspectpath/inpath/injars)

11:59:18 AM ===========================================================================================
11:59:18 AM Build kind = AUTOBUILD
11:59:18 AM Project=ChildTopParent, kind of build requested=Incremental AspectJ compilation
11:59:18 AM build: Examined delta - source file changes in required project ChildTopParent
11:59:18 AM Classpath=C:\workspaceEclipse3.2.2\ChildTopParent;C:/jdk1.5.0_06/jre/lib/rt.jar;C:/jdk1.5.0_06/jre/lib/jsse.jar;C:/jdk1.5.0_06/jre/lib/jce.jar;C:/jdk1.5.0_06/jre/lib/charsets.jar;C:/jdk1.5.0_06/jre/lib/ext/dnsns.jar;C:/jdk1.5.0_06/jre/lib/ext/localedata.jar;C:/jdk1.5.0_06/jre/lib/ext/sunjce_provider.jar;C:/jdk1.5.0_06/jre/lib/ext/sunpkcs11.jar;C:/EclipseSDK-3.2.2/eclipse/plugins/org.aspectj.runtime_1.5.4.200705211336/aspectjrt.jar;
11:59:18 AM Preparing for build: planning to be an incremental build
11:59:18 AM Starting incremental compilation loop 1 of possibly 5
11:59:18 AM AJC: compiling source files
11:59:18 AM Timer event: 328ms: Time to first compiled message
11:59:18 AM AJC: compiled: C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest.java
11:59:18 AM AJC: processing reweavable state
11:59:18 AM AJC: adding type mungers
11:59:18 AM Timer event: 328ms: Time to first woven message
11:59:18 AM AJC: woven class ChildTest (from C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest.java)
11:59:18 AM addSourcelineTask message=The field ChildTest.test12 is never read locally file=C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest.java line=3
11:59:18 AM Examining whether any other files now need compilation based just compiling: '{C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest.java}'
11:59:18 AM AspectJ reports build successful, build was: INCREMENTAL
11:59:18 AM AJDE Callback: finish. Was full build: false
11:59:18 AM Timer event: 328ms: Total time spent in AJDE
11:59:18 AM Timer event: 0ms: Create element map (0 rels in project: ChildTopParent)
11:59:18 AM Types affected during build = 1
11:59:18 AM Timer event: 0ms: Add markers (0 markers)
11:59:18 AM Timer event: 437ms: Total time spent in AJBuilder.build()
11:59:18 AM ===========================================================================================
11:59:18 AM Build kind = AUTOBUILD
11:59:18 AM Project=TopParent, kind of build requested=Incremental AspectJ compilation
11:59:18 AM Classpath=C:\workspaceEclipse3.2.2\TopParent;C:/jdk1.5.0_06/jre/lib/rt.jar;C:/jdk1.5.0_06/jre/lib/jsse.jar;C:/jdk1.5.0_06/jre/lib/jce.jar;C:/jdk1.5.0_06/jre/lib/charsets.jar;C:/jdk1.5.0_06/jre/lib/ext/dnsns.jar;C:/jdk1.5.0_06/jre/lib/ext/localedata.jar;C:/jdk1.5.0_06/jre/lib/ext/sunjce_provider.jar;C:/jdk1.5.0_06/jre/lib/ext/sunpkcs11.jar;C:/EclipseSDK-3.2.2/eclipse/plugins/org.aspectj.runtime_1.5.4.200705211336/aspectjrt.jar;C:\workspaceEclipse3.2.2\ChildTopParent;
11:59:18 AM Preparing for build: not going to be incremental because path change detected (one of classpath/aspectpath/inpath/injars)
11:59:18 AM Falling back to batch compilation
11:59:18 AM Preparing for build: not going to be incremental because no successful previous full build
11:59:19 AM AJC: compiling source files
11:59:19 AM Timer event: 266ms: Time to first compiled message
11:59:19 AM AJC: compiled: C:\workspaceEclipse3.2.2\TopParent\ParentTest.java
11:59:19 AM AJC: processing reweavable state
11:59:19 AM AJC: adding type mungers
11:59:19 AM Timer event: 266ms: Time to first woven message
11:59:19 AM AJC: woven class ParentTest (from C:\workspaceEclipse3.2.2\TopParent\ParentTest.java)
11:59:19 AM AspectJ reports build successful, build was: FULL
11:59:19 AM AJDE Callback: finish. Was full build: true
11:59:19 AM Timer event: 266ms: Total time spent in AJDE
11:59:19 AM Timer event: 0ms: Create element map (0 rels in project: TopParent)
11:59:19 AM Types affected during build = 1
11:59:19 AM Timer event: 0ms: Add markers (0 markers)
11:59:19 AM Timer event: 328ms: Total time spent in AJBuilder.build()
Comment 8 Andrew Clement CLA 2008-03-06 11:57:08 EST
I will take a look today at this.  My initial thought is that this is an enhancement request rather than a bug though.  The no dependent builds on whitespace change is what is advertised as working - if that doesnt work, it is a bug.  Changing the structure of a type or the signature of a type has never been a case that has been considered.  Anyway, taking a look now...  I really dont like that misleading message about:

not going to be incremental because path change detected (one of classpath/aspectpath/inpath/injars)

i've no idea why it is saying that.

I will be fixing it on the latest AspectJ, but will try retrofit to AJDT1.4 for you .
Comment 9 Andrew Clement CLA 2008-03-06 12:36:00 EST
I was confused by your comment about a private class variable (I read it too quickly) - i see (now I've extracted your zip) that you only meant a private field, not a private inner class.  Renaming a private inner class actually seems to work OK.  The private field rename is triggering a dependent rebuild, currently I'm checking why.

However, it might be worth capturing other things you think should not trigger a dependent rebuild - feel free to list them in this bug.  I am not currently sure whether there is going to be a global way to capture them all in one go, or if each will need addressing separately.
Comment 10 Sebastien Tardif CLA 2008-03-06 13:46:28 EST
Every change Eclipse compiler support for incremental build should be supported.

A look to Eclipse code may communicate what is supported and how it's done.

Below is a list to verify:
- create/update/delete/renaming/changeType private/protected/public/package static/instance final/notFinal method/variable declaration
- class creation/deletion/renaming/repackage/signature(interface/base class)

What already work are:
- space inside or outside method 
- method body modifications
Comment 11 Andrew Clement CLA 2008-03-06 14:04:59 EST
> Every change Eclipse compiler support for incremental build should be supported.

I agree but i want to write testcases for them all to verify so it is useful to spell them out.

> Below is a list to verify:
> - create/update/delete/renaming/changeType private/protected/public/package
> static/instance final/notFinal method/variable declaration
> - class creation/deletion/renaming/repackage/signature(interface/base class)

Whether some of these make a difference can depend on whether the dependent has errors or not.  If the dependent has an error because a type is missing and the 'class creation' in the project being depended on fixes that problem, a full build of the dependent is necessary.

And as I mentioned, changing the type in a significant way should sometimes trigger a build of the dependent.  Deleting a type, renaming a type, changing its package - all these should trigger a build of the dependent projects - the dependents may not do anything if they did not previously depend on those changes, but they must be told that something has changed and be given the chance.

The initial work here will only be for AJDT projects depending on other AJDT projects.
Comment 12 Andrew Clement CLA 2008-03-06 14:45:09 EST
The path change detected message occurs because the classpath of the project which is being depended upon is on the classpath of the 'subproject' and the .class file on that classpath has changed - we have not correctly realised we have a better representation of the state associated with that path and so can determine that it is a non-interesting change.
Comment 13 Andrew Clement CLA 2008-03-06 15:55:57 EST
As with the JDT code, AspectJ considers something like changing the name of a field (even a private one) as a structural change to the class.  Non structural changes are whitespace/method body type changes.  Here is the JDT code for looking at fields, from ClassFileReader:

private boolean hasStructuralFieldChanges(FieldInfo currentFieldInfo, FieldInfo otherFieldInfo) {
	// generic signature
	if (!CharOperation.equals(currentFieldInfo.getGenericSignature(), otherFieldInfo.getGenericSignature()))
		return true;
	if (currentFieldInfo.getModifiers() != otherFieldInfo.getModifiers())
		return true;
	if ((currentFieldInfo.getTagBits() & TagBits.AnnotationDeprecated) != (otherFieldInfo.getTagBits() & TagBits.AnnotationDeprecated))
		return true;
	if (!CharOperation.equals(currentFieldInfo.getName(), otherFieldInfo.getName()))
		return true;
	if (!CharOperation.equals(currentFieldInfo.getTypeName(), otherFieldInfo.getTypeName()))
		return true;
	if (currentFieldInfo.hasConstant() != otherFieldInfo.hasConstant())
		return true;
...

So a structural change is recorded when this occurs for a JDT or an AJDT project.  The different with JDT is that there is a large surrounding infrastructure of JavaDelta objects that describe the change that occurred and the builders are able to access them and work with them.  We don't duplicate the whole of JDT for AJDT and so have nothing to do with deltas right now.
Comment 14 Sebastien Tardif CLA 2008-03-06 16:13:32 EST
To clarify, what is the consequence of structural change? Is this triggering a full build or still do incremental build but more granular like at class level instead of some section of it?

What can be accomplish easily right now to get more changes not be categorized has structural change? Like private fields and methods?
Comment 15 Andrew Clement CLA 2008-03-06 17:24:32 EST
I think we are categorizing the right set of things as structural change - but there is a subset that could be considered unimportant structural changes, these would include:

- renaming/adding private fields/methods - that kind of thing

The delta handling in the JDT compiler understands whether something had a structural dependency on something in another type (eg. it extended another type or called a method within that type).  This delta handling enables it to say: well I know type X in a project I depend on has a structural change in it, but I don't care about that type.  And so the dependent project doesn't rebuild.

What can be accomplished easily right now are *probably* the private field/method/class interactions.  I am having a go at prototyping that now.
Comment 16 Andrew Clement CLA 2008-03-06 21:25:10 EST
I was thinking about this during my run on the treadmill today.  I think there
are three things we can do, each increasing in implementation effort but each
relatively straightforward, to help the situation here:

1. differentiate between a structural change and an 'interesting' structural
change.  The latter of which may actually affect a dependent.  This involves
changing the AjState code to recognize some cases that are not interesting,
like add/delete/rename of a private field (is this definetly true in every
case...).  My first prototype of this is running fine. I will attach a patched
class file shortly.

2. enhance ajstate with some knowledge from the classes constant pools so that
it knows which types it really depends on.  So that when there *is* a
structural change, check if it occurred to a type this project actually cares
about.  Currently a change to any type in the project you depend on causes you
to be rebuilt - regardless of whether that type was referenced in your code.

3. if the AspectJ IncrementalStateManager encounters a directory not being
managed (ie. it is not the output folder of an AspectJ project), then create a
lightweight State object to monitor changes on it and check for real structural
changes.

--- 

(1) will allow increasing flexibility in what you can change in an AJDT project
that will cause full builds of any dependent AJDT projects.

(2) will take that even further, allowing structural changes in some types to
not cause downstream builds because those project are not concerned with that
type.

(3) will enable AJDT projects to depend on Java projects and it will do the
right thing and not build unless necessary.  It won't address java projects
depending on AJDT projects though.

How soon those could be implemented, hmmmmm... and we'd have to be very careful
about memory usage with these new data structures.
Comment 17 Andrew Clement CLA 2008-03-06 21:27:29 EST
Created attachment 91826 [details]
files to update in an AJDT/AspectJ1.5.4 ajde.jar

This includes my changes to AjState that allow it to cope with changes to private fields in a better way - in case you wanted to try it.

In your AJDT with an up to date AspectJ1.5.4.jar, go to the org.aspectj.ajde plugin and

jar -xvf ajstate_patch.zip
jar -uvf ajde.jar org
Comment 18 Sebastien Tardif CLA 2008-03-11 19:04:33 EDT
I get the following stack in AJDT Event Trace after first start of Eclipse 3.2 with the patch:

java.lang.UnsupportedClassVersionError: Bad version number in .class file
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:620)
	at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.defineClass(DefaultClassLoader.java:161)
	at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.defineClass(ClasspathManager.java:501)
	at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findClassImpl(ClasspathManager.java:471)
	at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClassImpl(ClasspathManager.java:430)
	at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClass(ClasspathManager.java:413)
	at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.findLocalClass(DefaultClassLoader.java:189)
	at org.eclipse.osgi.framework.internal.core.BundleLoader.findLocalClass(BundleLoader.java:334)
	at org.eclipse.osgi.framework.internal.core.SingleSourcePackage.loadClass(SingleSourcePackage.java:37)
	at org.eclipse.osgi.framework.internal.core.BundleLoader.findClass(BundleLoader.java:383)
	at org.eclipse.osgi.framework.internal.core.BundleLoader.findClass(BundleLoader.java:347)
	at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:83)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:251)
	at java.lang.ClassLoader.loadClassInternal(ClassLoader.java:319)
	at org.eclipse.ajdt.core.builder.AJBuilder.addStateListener(AJBuilder.java:970)
	at org.eclipse.ajdt.internal.ui.tracing.DebugTracing.setDebug(DebugTracing.java:99)
	at org.eclipse.ajdt.internal.ui.tracing.EventTraceView.createPartControl(EventTraceView.java:77)
	at org.eclipse.ui.internal.ViewReference.createPartHelper(ViewReference.java:332)
	at org.eclipse.ui.internal.ViewReference.createPart(ViewReference.java:197)
	at org.eclipse.ui.internal.WorkbenchPartReference.getPart(WorkbenchPartReference.java:566)
	at org.eclipse.ui.internal.PartPane.setVisible(PartPane.java:290)
	at org.eclipse.ui.internal.ViewPane.setVisible(ViewPane.java:525)
	at org.eclipse.ui.internal.presentations.PresentablePart.setVisible(PresentablePart.java:140)
	at org.eclipse.ui.internal.presentations.util.PresentablePartFolder.select(PresentablePartFolder.java:268)
	at org.eclipse.ui.internal.presentations.util.LeftToRightTabOrder.select(LeftToRightTabOrder.java:65)
	at org.eclipse.ui.internal.presentations.util.TabbedStackPresentation.selectPart(TabbedStackPresentation.java:394)
	at org.eclipse.ui.internal.PartStack.refreshPresentationSelection(PartStack.java:1144)
	at org.eclipse.ui.internal.PartStack.createControl(PartStack.java:620)
	at org.eclipse.ui.internal.PartStack.createControl(PartStack.java:532)
	at org.eclipse.ui.internal.PartSashContainer.createControl(PartSashContainer.java:562)
	at org.eclipse.ui.internal.PerspectiveHelper.activate(PerspectiveHelper.java:244)
	at org.eclipse.ui.internal.Perspective.onActivate(Perspective.java:815)
	at org.eclipse.ui.internal.WorkbenchPage.onActivate(WorkbenchPage.java:2436)
	at org.eclipse.ui.internal.WorkbenchWindow$6.run(WorkbenchWindow.java:2616)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
	at org.eclipse.ui.internal.WorkbenchWindow.setActivePage(WorkbenchWindow.java:2597)
	at org.eclipse.ui.internal.WorkbenchWindow.restoreState(WorkbenchWindow.java:1982)
	at org.eclipse.ui.internal.Workbench.doRestoreState(Workbench.java:2873)
	at org.eclipse.ui.internal.Workbench.access$14(Workbench.java:2821)
	at org.eclipse.ui.internal.Workbench$20.run(Workbench.java:1697)
	at org.eclipse.ui.internal.Workbench.runStartupWithProgress(Workbench.java:1437)
	at org.eclipse.ui.internal.Workbench.restoreState(Workbench.java:1695)
	at org.eclipse.ui.internal.Workbench.access$12(Workbench.java:1666)
	at org.eclipse.ui.internal.Workbench$18.run(Workbench.java:1545)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
	at org.eclipse.ui.internal.Workbench.restoreState(Workbench.java:1489)
	at org.eclipse.ui.internal.WorkbenchConfigurer.restoreState(WorkbenchConfigurer.java:183)
	at org.eclipse.ui.application.WorkbenchAdvisor.openWindows(WorkbenchAdvisor.java:702)
	at org.eclipse.ui.internal.Workbench.init(Workbench.java:1101)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:1863)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:422)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.IDEApplication.run(IDEApplication.java:95)
	at org.eclipse.core.internal.runtime.PlatformActivator$1.run(PlatformActivator.java:78)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:92)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:68)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:400)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:177)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:585)
	at org.eclipse.core.launcher.Main.invokeFramework(Main.java:336)
	at org.eclipse.core.launcher.Main.basicRun(Main.java:280)
	at org.eclipse.core.launcher.Main.run(Main.java:977)
	at org.eclipse.core.launcher.Main.main(Main.java:952)
Comment 19 Andrew Clement CLA 2008-03-11 19:27:01 EDT
Damn, that was built with Java6 (a version 50 class file) - sorry to waste your time, stupid eclipse, it should ideally have been built with 1.4.
Comment 20 Sebastien Tardif CLA 2008-03-12 16:23:00 EDT
I had to disable AJDT in our project because right now multi-project setup doesn't normally support incremental build. Granted, space and method body modification work but in real scenario, update from source control always involve something else. 

We had flashback to the productivity we had 5 years ago with JBuilder ;-( and how was compiling in C++.

When do you think a prototype will be available for item 2:

>2 - will take that even further, allowing structural changes in some types to
>not cause downstream builds because those project are not concerned with that
>type.

If child projects are called to build but still do incremental build the performance will probably be good enough.
Comment 21 Andrew Clement CLA 2008-03-13 17:40:15 EDT
not quite sure when a prototype of 2 will be available.  Immediate priority is shipping a 1.6.0 final release after doing some work on memory usage (another problem you eluded to in a previous exchange we had).

I have up'd the priority to P2 and targetted 1.6.0rc1 just so this doesn't drop off my radar but I'm not sure whether it will make it (and of course there'd be extra work to backport the changes to an AJDT suitable for RAD users)
Comment 22 Andrew Clement CLA 2008-03-21 23:26:10 EDT
...I have a prototype of option (2), currently doing some testing of it - will let you know when there is something to try.
Comment 23 Andrew Clement CLA 2008-03-24 14:41:55 EDT
My current changes work as follows:

On detecting a change on the classpath, we look at whether any type in this project depends on that type.  If no-one does, we don't care and a fast incremental build will ensue.  If someone does, we make a note of who and just incrementally compile that source file.  If more than 10 (arbitrarily chosen...) source files depend upon changes seen on the classpath then we drop back to a full build.

It does not matter if the project depended upon is AJDT or JDT, we treat them the same which means we can do incremental builds correctly for AspectJ projects depending upon Java projects.

All the new state to make this happen is stored via SoftReferences and so heap will be utilised when available but this stuff will be collected up when memory is short.

I have done some basic testing of this but it needs more.  It is non trivial to plug into an old AJDT like 1.4 but I could do it if you will try it out?
Comment 24 Andrew Clement CLA 2008-03-24 15:02:18 EDT
Here is some typical output I now see when using an AJDT 1.5 enabled with this update, this comes out in the event trace view when turned up to fully verbose.  I make a change to an internal type in an ajdt project:

12:0:22 ===========================================================================================
12:0:22 Build kind = AUTOBUILD
12:0:22 Project=org.eclipse.ajdt.core, kind of build requested=Incremental AspectJ compilation
12:0:22 build: Examined delta - source file changes in required project org.eclipse.ajdt.core
12:0:22 Classpath=D:\Workspaces\ajdt15\org.eclipse.ajdt.core\bin;etc...
12:0:22 Starting incremental compilation loop 1 of possibly 5
12:0:22 AJC: compiling source files
12:0:23 Timer event: 648ms: Time to first compiled message
12:0:23 AJC: compiled: D:\Workspaces\ajdt15\org.eclipse.ajdt.core\src\org\eclipse\ajdt\internal\core\ajde\CoreBuildMessageHandler.java
12:0:23 AJC: processing reweavable state
12:0:23 AJC: adding type mungers
12:0:23 Timer event: 697ms: Time to first woven message
12:0:23 AJC: woven class org.eclipse.ajdt.internal.core.ajde.CoreBuildMessageHandler (from D:\Workspaces\ajdt15\org.eclipse.ajdt.core\src\org\eclipse\ajdt\internal\core\ajde\CoreBuildMessageHandler.java)
12:0:23 AspectJ reports build successful, build was: INCREMENTAL
12:0:23 AJDE Callback: finish. Was full build: false
12:0:23 Timer event: 721ms: Total time spent in AJDE
12:0:23 Timer event: 15ms: Create element map (402 rels in project: org.eclipse.ajdt.core)
12:0:23 Types affected during build = 1
12:0:23 Timer event: 37ms: Add markers (198 markers)
12:0:23 Timer event: 930ms: Total time spent in AJBuilder.build()
12:0:23 ===========================================================================================
12:0:23 Build kind = AUTOBUILD
12:0:23 Project=org.eclipse.ajdt.core.tests, kind of build requested=Incremental AspectJ compilation
12:0:23 Classpath=D:\Workspaces\ajdt15\org.eclipse.ajdt.core.tests\bin;etc...
12:0:23 isTypeWeReferTo(): Cache hit, looking up classname for : D:\Workspaces\ajdt15\org.eclipse.ajdt.core\bin\org\eclipse\ajdt\internal\core\ajde\CoreBuildMessageHandler.class
12:0:23 AjState(org.eclipse.ajdt.core.tests): type org/eclipse/ajdt/internal/core/ajde/CoreBuildMessageHandler is not depended upon by this state
12:0:23 Preparing for build: planning to be an incremental build
12:0:23 AspectJ reports build successful, build was: INCREMENTAL
12:0:23 AJDE Callback: finish. Was full build: false
12:0:23 Timer event: 100ms: Total time spent in AJDE
12:0:23 Timer event: 8ms: Create element map (0 rels in project: org.eclipse.ajdt.core.tests)
12:0:23 Timer event: 0ms: Add markers (0 markers)
12:0:23 Timer event: 245ms: Total time spent in AJBuilder.build()
12:0:23 ===========================================================================================

You can see in the build of the dependent project the line 'type org/eclipse/ajdt/internal/core/ajde/CoreBuildMessageHandler is not depended upon by this state' which means a fast incremental build of the sub project.

Comment 25 Andrew Clement CLA 2008-03-24 15:50:07 EDT
If running a recent dev build version of AJDT 1.5 for Eclipse 3.3.2 then the modified ajde.jar can be found here:

http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/ajde_221427.jar

Take this ajde_221427.jar and replace the version in your eclipse org.aspectj.ajde plugin with it (backup the ajde.jar in that plugin before replacing it).

Once all the output is turned on in the AJDT event trace view, the log will be quite verbose and is likely to even slow down the response times, however it is invaluable for checking that the incremental compiler is doing the right thing.  To see if the UI is more responsive I recommend turning off the AJDT event trace view messages (not just closing the view, turn them off in the event trace view configure menu).  

If you see unexpected behaviour (it is repeatedly compiling more than you think) then attach the verbose output here (sanitized if you don't want me to see your type names) and let me know what you would have expected to see?

The only case I can't influence is where a Java project depends upon an AJDT project - since I can't make the JDT make better decisions about changes it sees on the classpath of a Java project.

Let me know if it:
a) doesnt behave for you
b) makes no difference at all!
c) makes a difference
Comment 26 Andrew Clement CLA 2008-03-24 16:05:43 EDT
hmm, I may have just seen a problem where an error got attached to a type because not even had been rebuilt - but proving tricky to isolate the cause.
Comment 27 Andrew Clement CLA 2008-03-24 16:07:31 EDT
my mistake, user error ;)  patch was working OK
Comment 28 Sebastien Tardif CLA 2008-03-24 22:35:54 EDT
(In reply to comment #23)
>If more than 10 (arbitrarily
> chosen...) source files depend upon changes seen on the classpath then we drop
> back to a full build.

If 10 is total of changes of all dependents projects recursively than that's too small. I'm not sure we really need a limit. If incremental build can work should it be always faster?

> It does not matter if the project depended upon is AJDT or JDT, we treat them
> the same which means we can do incremental builds correctly for AspectJ
> projects depending upon Java projects.

That's great news.

> All the new state to make this happen is stored via SoftReferences and so heap
> will be utilised when available but this stuff will be collected up when memory is short.

Sound a good idea but I'm not sure in practice that will work because AJDT/AspectJ use so much memory that Eclipse are often reaching the max of memory usage so do often full gc. 

> I have done some basic testing of this but it needs more.  It is non trivial to
> plug into an old AJDT like 1.4 but I could do it if you will try it out?


Yes please try to backport to AJDT for Eclipse 3.2. I will certainly test it and use it if that work because right now I didn't find a better solution than plug-in ANT script,s with dependencies detection, to Eclipse project builder, to call AspectJ compiler.
Comment 29 Andrew Clement CLA 2008-03-24 23:20:03 EDT
> If 10 is total of changes of all dependents projects recursively than that's
> too small. I'm not sure we really need a limit. If incremental build can work
> should it be always faster?

It is 10 per project.  You should have a limit because there is a cost associated with determining what depends on what - I chose a number similar to what JDT uses internally.  It is the cut off point where you realise 'well so much has changed, we'd be better doing a full build if the analysis is going to continue like this'.

> Sound a good idea but I'm not sure in practice that will work because
> AJDT/AspectJ use so much memory that Eclipse are often reaching the max of
> memory usage so do often full gc. 

That's something to find out with testing of this on large projects - if there is no room for further references, it will just not cache the information and recalculate each time.


> Yes please try to backport to AJDT for Eclipse 3.2. I will certainly test it
> and use it if that work because right now I didn't find a better solution than
> plug-in ANT script,s with dependencies detection, to Eclipse project builder,
> to call AspectJ compiler.

I will get to this as soon as I can.
Comment 30 Sebastien Tardif CLA 2008-03-25 09:20:28 EDT
(In reply to comment #29)
> It is the cut off point where you realise 'well so
> much has changed, we'd be better doing a full build if the analysis is going to
> continue like this'.

If project Xdoes a full build but the full build end-up to just modify 9 classes, the dependent projects of X should still do incremental build based on the classes that was really modified if the dependent projects really care about those specific classes.
Comment 31 Andrew Clement CLA 2008-03-25 13:01:52 EDT
Going to miss 1.6.0 - can be put into 1.6.1 if it gets more testing.
Comment 32 Neale Upstone CLA 2008-03-25 15:00:24 EDT
Hi Andy,

I can see the sense in postponing, and agree that it sounds like more testing and feedback is needed.

Would it be possible to include this functionality as a Preferences option, or does it hit too many areas?  If you could, then it would encourage many more people to try it out when they move to Ganymede.

Cheers,

Neale
Comment 33 Andrew Clement CLA 2008-03-26 02:23:06 EDT
Hi Neale,

delaying it to 1.6.1 will just mean it is in a dev build just after 1.6.0 - so there won't be a long delay.  I'm trying to move away from long gaps between point releases.  Further testing today revealed to me it has an awkward bug in it that I hadn't initially spotted that I have now fixed.  I do plan to make it switchable initially - possibly defaulting to on though so it only gets switched off if it misbehaves - and hopefully whoever switches it off raises a bug for me :)
Comment 34 Andrew Clement CLA 2008-04-01 17:34:06 EDT
after further testing, I've committed these changes for 1.6.0rc1.  I also included some simple optimizations to the state that will save a bit of memory per project.  still not in a 1.5.4 driver though as I had to modify the compiler itself which makes it trickier.
Comment 35 Andrew Clement CLA 2008-04-03 19:23:29 EDT
changes are available in latest AJDT 1.5.2 dev builds for Eclipse 3.3.
Comment 36 Sebastien Tardif CLA 2008-04-03 20:09:29 EDT
Thanks Andy for your hard work. 

I have tested latest AJDT 1.5.2 dev builds for Eclipse 3.3 and it's behaving well in all scenarios I tried.

Do you think it's possible to get something for Eclipse 3.2.x ?
Comment 37 Andrew Clement CLA 2008-04-03 20:22:51 EDT
i'm trying, i'm trying (right now in fact).
Comment 38 Andrew Clement CLA 2008-04-04 00:41:49 EDT
For Eclipse 3.2 and AJDT 1.4, I've built a new jar for the org.aspectj.ajde plugin:

http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/ajde.jar.20080403

and a new jar for the org.aspectj.weaver plugin:

http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/aspectjweaver.jar.20080403

replace the ones in your current build of AJDT1.4 and try it.

I tried to do a dev build of 1.4.3 but the build system is playing up and refuses to run the tests so I can't do it that way at the moment.

Those jars may take a little while to replicate around for download as I've only just uploaded them.
Comment 39 Andrew Clement CLA 2008-04-08 11:50:23 EDT
Did you get a chance to try out the new jars?  I'd like to tell the mailing list users about them being available for Eclipse 3.2 AJDT, if they work.
Comment 40 Sebastien Tardif CLA 2008-04-08 14:45:07 EDT
I have tested simple scenarios under Eclipse 3.2.2 using ajdt_1.4.2_for_eclipse_3.2.zip as the base but using the two jars files you have provided.

All scenarios tested are working. 

However, some messages are duplicated like:

2:41:29 PM Found state instance managing output location : C:\workspaceEclipse3.2.2\ChildTopParent
2:41:29 PM Found state instance managing output location : C:\workspaceEclipse3.2.2\ChildTopParent

and

2:42:24 PM Found state instance managing output location : C:\workspaceEclipse3.2.2\ChildTopParent
2:42:24 PM Change detected in C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest2$Child2Test.class but it is not structural
2:42:24 PM Structural change detected in : C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest2.class
2:42:24 PM AjState(TopParent): type ChildTest2 is depended upon by 'C:\workspaceEclipse3.2.2\TopParent\ParentTest.java'
2:42:24 PM Found state instance managing output location : C:\workspaceEclipse3.2.2\ChildTopParent
2:42:24 PM Change detected in C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest2$Child2Test.class but it is not structural
2:42:24 PM Structural change detected in : C:\workspaceEclipse3.2.2\ChildTopParent\ChildTest2.class
2:42:24 PM AjState(TopParent): type ChildTest2 is depended upon by 'C:\workspaceEclipse3.2.2\TopParent\ParentTest.java'

Hopefully, duplicate message doesn't mean duplicate work.

I will be testing on a real complex workspace having dozens of projects shortly.
Comment 41 Andrew Clement CLA 2008-04-08 17:06:03 EDT
same message twice initially suggests to me duplicate entries on the classpath and will cause duplicate work.
Comment 42 Sebastien Tardif CLA 2008-04-08 17:21:17 EDT
We do not have duplicate entries in the classpath. Also no entries are exported. The scenario is using the extra simple workspace provided previously in this bug, see attachment. The only difference is that we have made one class of the total of 2 classes extends the other. Each class are in different projects.
Comment 43 Andrew Clement CLA 2008-04-09 19:45:49 EDT
duplication of analysis fixed in AspectJ.  Was due to duplicate entries in the classpath (at least it was in my test scenario).  Not necessarily entries created directly by the user, but in the classpath constructed by AJDT for the build.

I am also persuing build complexities in bug 160868.
Comment 44 Andrew Clement CLA 2008-04-23 16:03:37 EDT
i am closing this to mark the work done for AspectJ1.6.0 - if we are going to do more for 1.6.1 we should do it in a new bug referring back to this one.