Bug 293450 - overweaving
Summary: overweaving
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.6.6   Edit
Hardware: PC Windows NT
: P3 enhancement (vote)
Target Milestone: 1.6.9   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-27 10:38 EDT by Andrew Clement CLA
Modified: 2010-06-30 10:57 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Clement CLA 2009-10-27 10:38:30 EDT

    
Comment 1 Andrew Clement CLA 2009-10-27 10:42:23 EDT
For some time now, AspectJ has allowed classes to be woven multiple times.  In order to preserve semantics and reduce the complexity of the implementation we use a reweaving approach where we revert to the original bytecode then reapply all the aspects previously applied plus any new ones.  In some situations this can be overkill if the new aspects were acting more like monitoring probes than doing anything particularly serious.

This enhancement covers overweaving.  This is like reweaving but we do *not* revert to the original bytecode we just weave on top of the existing bytecode.  The approach is:

- activated by -Xset:overWeaving=true
- causes us not to process any diff in the WeaverStateInfo attribute that would revert us to the original bytecode
- we don't check if the previously applied aspects are still around
- we *do* check we don't reapply any aspects that were previously applied

Issues uncovered so far include a problem where we start creating joinpoint fields (ajc$tjp_N) starting at N=0 for the new weave and these clash with those added previously.
Comment 2 Andrew Clement CLA 2009-10-27 11:36:40 EDT
fixed the counter for that field.

Fixed the problem of shadowmungers repeatedly applying.

Now looking at typemungers.

One interesting thing - the BcelClassWeaver attempts a fast match for shadow mungers and actually sorts them by shadow kind.  We don't use that sorted variant though - seems like weaving could be faster if we did so I've changed to using that for the moment.  Pretty fundamental change but can't see why it should be a problem.
Comment 3 Andrew Clement CLA 2009-10-27 12:02:55 EDT
if we are going to discount certain mungers based on their declaring type, we need all mungers to have their declaring type set.  This is currently not true for 'support mungers', for example the one representing entry to a control flow.  This is conjured up based on a real cflow but is not getting the aspect set properly.
Comment 4 Andrew Clement CLA 2010-05-12 18:01:14 EDT
working on this again.

fixing the case where a type is initially woven by aspect A then at load time aspects A and B are around.  We must not reapply A.  Fixed it for shadow mungers (advice) and for some type mungers (ITDs/declare parents).

Currently the shadow mungers are dimissed at quite a high level, before the class weaver is built to weave a particular type.  Type mungers are different, they are processed as normal and added to the target but right at the last minute they are not applied.  I think it may be necessary to have them around for other things to work properly, so I didn't remove them up front.
Comment 5 Andrew Clement CLA 2010-05-12 18:03:35 EDT
Looking at the 'call' pointcuts.  I imagined this would be a nightmare with overweaving, and it is.

On the secondary weave we trundle through the static initializer and see all sorts of candidate join points that were inserted in the primary weave - calls to factorys to build joinpoint objects.  unfortunately they cannot all be eliminated based on the type name because there is some class access via Class.forName() and some string manipulation (StringBuilder calls).  I'm thinking the route to address these would be to pull out the code that is inserted in the static initializer into another static method and call it from the static initializer.  If this new method has a name prefixed ajc$ it will be ignored for weaving purposes!
Comment 6 Andrew Clement CLA 2010-05-12 20:52:38 EDT
static initialization code now moved to ajc$preClinit - except when in an interface as we can't have additional static methods.

Extra code added to:
- BcelClassWeaver.match(LazyMethodGen) - ignore ajc$ prefixed methods, dont make them execution join points.  This avoids the new ajc$preClinit becoming a method initialization join point
- BcelClassWeaver.matchInvokeInstruction - when overweaving it will skip calls to CFlowCounter/CFlowStack (checking fully qualified type name) and calls to aspectOf.

That final change, to ignore calls to aspectOf() could cause us trouble down the line, but I imagine 99% of the calls to aspectOf in the world are not user initiated.  This only affects overweaving too, which is a work in progress.

Now to test a little around whether overweaving can run when the type has been built non reweavable.  Could be a nice reduction in class file size if we can do that.
Comment 7 Andrew Clement CLA 2010-05-13 12:13:42 EDT
fixed another case with the use of thisJoinPoint (as opposed to thisJoinPointStaticPart).
Comment 8 Andrew Clement CLA 2010-05-13 12:17:53 EDT
to distinguish with the user written aspectOf() and generated aspectOf(), we should create an ajc$aspectOf() (and hasAspect) in the aspect - this will mean the generated ones aren't accidentally picked up.
Comment 9 Andrew Clement CLA 2010-06-30 10:57:55 EDT
all work planned for 1.6.9 is complete - closing this bug to capture that.  Overweaving is actively being used in some configurations (eg. during the google keynote when a Roo app was insight instrumented).  Further work will be done in other bugs.