Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [aspectj-users] LTW requirements for annotation-style @Aspect s compiled with javac

> It feels like advice named in aop.xml should be auto-woven if required (built with javac) rather than repeating it in <weaver>.

Yep, I agree - there should be no need to repeat yourself there.
Please raise an issue
https://bugs.eclipse.org/bugs/enter_bug.cgi?product=AspectJ

cheers,
Andy

On 15 November 2010 18:12, javabrett <javabrett@xxxxxxxxx> wrote:
>
>
> On Tue, Nov 16, 2010 at 4:39 AM, Andy Clement [via AspectJ] <[hidden email]>
> wrote:
>>
>> Hi Brett,
>>
>> On 14 November 2010 12:41, javabrett <[hidden email]> wrote:
>> >
>> > This exercise sparked the following questions for me:
>> >
>> > 1) Is the no-search and explicit weaver-need for the annotation-based
>> > advice
>> > behaviour change in 1.6.8 mentioned anywhere in the changes docs?  I
>> > couldn't spot it in the 1.6.8 README [2].  Might be a good one to point
>> > out
>> > somewhere if not already noted, perhaps a FAQ entry for those using
>> > annotation-style javac-compiled advice.
>> > 2) To me it seems like the bulk of aop.xml examples are either pre this
>> > change, or expect non-annotation-based advice or use of the ajc compiler
>> > (which does the advice-weaving).  I reckon most if not all examples of
>> > aop.xml I've encountered do not include weaving directives for the
>> > advice
>> > itself, only for the classes to be woven/advised at load-tim.  It might
>> > be
>> > worth having such an example with a commented <weaver> inclusion for an
>> > annotation-style javac-compiled advice.
>> You are exactly right that previously (prior to 1.6.7 actually) it
>> used to more actively search for annotation based aspects because they
>> needed 'finishing off'.  A change to this wasn't really made due to
>> more people using ajc rather than javac, but because of the sheer cost
>> of searching for annotation based aspects.  Usually there are far
>> fewer aspects than regular classes in a normal system, and the only
>> way to discover annotation based aspects was by actually loading the
>> bytes, it was found to drastically improve performance if the change
>> was made requiring the user to actively list the aspects in the
>> aop.xml file.
>>
>> I had thought some text to this effect had made it into either the
>> 1.6.7 or 1.6.8 readmes.  (1.6.7 covered the bulk of changes in this
>> area, 1.6.8 was an emergency release a few days later to address some
>> serious issues that were found with the big changes in 1.6.7).
>> However, I double checked and it isn't  - so I raised
>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=330269 as a placeholder
>> bug to remind me to document it (and a FAQ entry would be nice, yes).
>>
>> Normally I would always say that all aspects should be listed in an
>> aop.xml - without them listed reweaving won't work very well.
>> Recently I had to address a situation where Spring Roo (which heavily
>> uses aspects) was not listing all aspects and that caused reweaving to
>> fail at application server startup time (due to Spring Insight, which
>> uses LTW, attempting to reweave the Roo application).  This was
>> nothing to do with code style vs annotation style.
>>
>> My gut reaction is that you only need to list them in the aspects
>> section so we know what they are - are you finding you also need to
>> list them in the weaver section?  (I would need to double check my
>> expectations here.)
>>
>> cheers
>> Andy
>
> Thanks for the note Andy.
> This was my development sequence/experience:
> * Target code to advise and JVM are 1.6/Sun 1.6.
> * I created a new Eclipse Helios AspectJ project with defaults (although I
> always intended to write the advice in Java).
> * Added a new/single aspect class, written in plain-old .java and using
> @Aspect annotation-style.
> * Single/inline execution pointcut (with args and returning bindings) in an
> @AfterReturning annotation.
> * Added META-INF/aop.xml as part of Eclipse-exported JAR.  Contains single
> <weaver> include (advised package) and a single <aspects><aspect> (the
> advice class).
> * Exported JAR with Eclipse - at this point AJDT is active.  Configured the
> LTW agent and the advice is correctly weaved.
> * Built the advice project with plain-old javac (via Maven actually, but not
> using ajc or special Maven plugins - plain Java JAR project).  Advice is no
> longer weaved with this JAR.
> * Converted the Eclipse project to a plain Java project, so AJDT becomes JDT
> compiler.  Advice is also no longer weaved, so nothing wrong with Maven
> options.
> * Read your mailing list post.
> * Added a new entry to <weaver>, <include>ing the Java package name of the
> advice Java class.
> * Advice is again woven by the LTW.
> Conclusion is that the advice class, as well as the advised class must be
> explicitly exposed to the weaver when the plain javac compiler is used, and
> that this is a behavioural change in 1.6.7/8 caused by removing the
> annotation-hunt.  Listing it as <advice> seems to not be sufficient (this is
> probably a violation of DRY principle - i.e. I've named the advice, this
> should be sufficient to have it woven?).  This is not required if the ajc
> compiler is used, as it performs some enhancement/weaving of the advice
> class before deployment and exposure to the LTW (the additional bytecode is
> visible when comparing the java output with the ajc output).
> Let me know if you feel that there is a DRY principle violation here and I
> can raise a bug.  It feels like advice named in aop.xml should be auto-woven
> if required (built with javac) rather than repeating it in <weaver>.
> Cheers
> Brett
> ________________________________
> View this message in context: Re: LTW requirements for annotation-style
> @Aspect s compiled with javac
> Sent from the AspectJ - users mailing list archive at Nabble.com.
>
> _______________________________________________
> aspectj-users mailing list
> aspectj-users@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/aspectj-users
>
>


Back to the top