Community
Participate
Working Groups
This is something it might now be time to address. I encountered it in bug 267794 and it was rather annoying. With all the improvements to incremental it is annoying to just throw our hands in the air and do a full build if we hit an aspect on the defined classpath/aspectpath for a project. I'm not looking into inpath here, if that benefits from some of these changes - that's great - but it is not something I'm trying to fix here.
I have a minimal project configuration A - defines abstract base tracing aspect (abstract pointcut) B - depends upon A (via classpath) and supplies: 1) a concrete aspect extending the tracing aspect 2) a type affected by that tracing aspect C - this project depends upon B Here is a typical problematic scenario: In project B the affected class is changed (a STRUCTURAL change is made, a new method to be affected by the trace aspect). It is recompiled - due to the type dependencies stored in the state, we see that the concrete subaspect is related to it and so rebuild that too - this is OK. Project C then rebuilds because it depends on project B. It sees that the affected class has been recompiled, but decides it doesn't care as it has nothing in it that depends upon it. Then it sees the rebuilt aspect class file - and goes OH NO, MUST REBUILD FROM SCRATCH. Rebuilding this project from scratch then causes nightmare builds for everything else downstream. The problematic code is the statement in the path analysis logic: if (state.isAspect(classFile)) { return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD; } Two questions need answering before I change it (kind of related to each other). - Can I just remove the check and depend on the regular type dependency information maintained to know what to do? Or do we need special logic to handle aspects? - Is it true that anything that changes within an aspect will cause the .class file for the aspect to exhibit a structural change?
> - Is it true that anything that changes within an aspect will cause the .class > file for the aspect to exhibit a structural change? In a simple case, if a pointcut changes, the hashcode from the pointcut text changes and the hashcode is used to create the advice name. However in the case of an abstract aspect and concrete subaspect, the advice names will be basically 'fixed' in the super-aspect based on the pointcut specified in the aspect. Right now (I've tried it) - changing the expression in the subaspect pointcut does NOT manifest as a structural change to the .class file for the subaspect. So an aspect can be changed in such away that its affect should affect more or less places and just seeing if we need to do something based on structural changes of .class files will not identify what has happened. So the timestamp of the .class file will change but not the structure. What does that mean for downstream projects? If a downstream project, like my project C, had only a classpath dependency (and not aspectpath) upon that aspect and that aspect was concrete, then we are fine to avoid a full build. The effect that aspect had on other types in project B will cause us to rebuild the appropriate parts of project C. If A defined an abstract aspect, then B extended it via another abstract aspect, and then C concretized the aspect... then regardless of whether it has a structural change on we need to do the isTypeWeReferTo check - and if that is true, we know we have a local concrete aspect. So it sounds like I'm morphing this test from: if (state.isAspect(classFile)) { return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD; } to if (state.isAspect(classFile)) { if (state.hasStructuralChangedSince(classFile, lastSuccessfulBuildTime) || isTypeWeReferTo(classFile)) { // further improvements possible return CLASS_FILE_CHANGED_THAT_NEEDS_FULL_BUILD; } else { // it is an aspect but we don't refer to it: // - for CLASSPATH I think this is OK, we can continue and try an // incremental build // - for ASPECTPATH we don't know what else might be touched in this project // and must rebuild } } is that right...?
The test appears ok but glitches galore are falling out now: - in incremental building it checks all types from projects upon which it references to see which it has a direct dependency on. Once one of them triggered it to say YES, I depend directly on that type, then it considered that it directly depended upon everything checked after that. This would lead to incremental builds that build more than they need to. - now I have the problem that in multiple projects I named the sub-aspect the same thing (Tracer). This means when we observe the aspect Tracer on the classpath for our project, we think we depend upon it because we have a type called Tracer - I think that match needs to be ignored because ours would override what was on the classpath anyway. Fixed the first one, working on the second one.
I'm deferring the second problem I listed in the previous comment - it is unlikely in a real world setup I *think* and I want to chew on it some more. But here are the numbers so far. For my large scale configuration where all the aspectj modules are aspectj projects and 3 of them extend an abstract aspect defined in another project - the build time (for adding a new method to util/FileUtil.java) is reduced. Without the change in the test: 3 runs of an incremental structural change: 51352ms, 46705ms, 50881ms With the change: 3 runs of an incremental structural change: 7843ms, 11112ms, 7680ms err, bit of a saving... the big benefit here will be to anyone downstream of a project using aspectpath where they themselves are not using aspectpath.
spun off that seconday problem as bug 270046. Running the AJ tests against the changes I've just made for aspectpath projects.
fixes in - now going back to 267794
