Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
RE: [aspectj-dev] change in runtime execution order

Gregor,
 
Thanks again for your good explanation on the whys behind what was choosen.  Given all the discussions and the fact that I relied upon the default ordering to do the right thing for me, I have submitted Bug 111667 to cover and an enhacement request to report this as a warning because as you mention, some of us have relied upon this to occur without ever realizing it was occuring.
 
And in my case, it caused a severe production error that was difficult at best to pin down.  Had I had a warning, I may have corrected this long ago.
 
Thanks again!
 
Ron

	-----Original Message----- 
	From: aspectj-dev-bounces@xxxxxxxxxxx on behalf of Gregor Kiczales 
	Sent: Wed 10/5/2005 2:12 PM 
	To: 'AspectJ developer discussions'; 'Andy Clement' 
	Cc: 
	Subject: RE: [aspectj-dev] change in runtime execution order
	
	
	Ron said: 
	 
	Now, I am going to ensure that my advice is order the way I want it to, but declaring precedence. :-)
	 
	That really is the key point here.
	 
	In the AspectJ language, there are some cases of advice precedence where the program just doesn't say. Two pieces of before advice, in totally unrelated aspects, just don't have a well-defined order. When designing the language we had to make a decision on what to do about such cases. There were several avenues:
	 
	 - Just design the language so that such cases can't come up. A simple single inheritance OO language for example doesn't have this kind of problem. A well-designed multiple-inheritance OO language can avoid it as well. But for AOP, designing the language so that this situation can't come up doesn't seem to work. Should we say you just can't have more than one advice on a join point (shadow)?
	 
	 - Pick a well-defined, but arbitrary rule to resolve ambiguity, such as alphabetical order of enclosing aspect name. The problem with that is that is that it encourages programmers to use aspect names as a way of declaring precedence, which seems awful.
	 
	 - Make it undefined, and encourage programmers who depend on precedence to use one of the well-defined mechanisms for controlling it.
	 
	We decided to do the third. But even when you do the third, there's some open design space. For example, you could make the implementation (compiler, loader, vm, whatever) pick a random order for advice precedence that isn't declared. That's a way of constantly waking people up to the fact that they are depending on something that they haven't declared. Unfortunately its also a way to give people a headache when they are debugging, reading logs, etc.
	 
	So the original implementation had an order, but the language specification made it clear you couldn't count on it. The problem of course, in the absence of a warning message like the one Ron is proposing, is that people came to count on it without knowing they were counting on it.
	 
	I still think the specification has to leave this undefined. But maybe a warning is good thing that the individual implementations could do.
	 
	 
	 


  _____  

		From: aspectj-dev-bounces@xxxxxxxxxxx [mailto:aspectj-dev-bounces@xxxxxxxxxxx] On Behalf Of Ron DiFrango
		Sent: Wednesday, October 05, 2005 10:33 AM
		To: Andy Clement; AspectJ developer discussions
		Subject: RE: [aspectj-dev] change in runtime execution order
		
		
		Thanks Andy for that great explanation of the how & why it is the way it is.  
		 
		Any thoughts on spitting out a warning when the compiler has to make this determiniation?  In my opinion and my recent experience, it would be.  Also, it would be nice if it spit out the ordering that it choose.  Please :-)
		 
		Now, I am going to ensure that my advice is order the way I want it to, but declaring precedence. :-)

			-----Original Message----- 
			From: Andy Clement [mailto:andrew.clement@xxxxxxxxx] 
			Sent: Wed 10/5/2005 12:42 PM 
			To: AspectJ developer discussions 
			Cc: 
			Subject: Re: [aspectj-dev] change in runtime execution order
			
			

			I presume you don't want me just to answer that "Its undefined - so
			you can't rely on it and I can change it whenever I like".
			
			As you have asked on aspectj-dev, I will give you more to go on.
			Heres todays story, but be aware I could change it all tomorrow:
			
			Here is the highly scientific advice sorting algorithm from M2:
			
			Collections.sort(
			        shadowMungerList,
			        new Comparator() {
			                public int compare(Object o1, Object o2) {
			                        return o1.toString().compareTo(o2.toString());
			                }
			        });
			
			Here is an example program:
			
			class X {
			  public static void main(String[]argv) {
			  }
			}
			
			aspect A {
			  before(): execution(* main(..)) && within(*) { }
			}
			aspect B {
			  before(): execution(* main(..)) && within(*){ }
			}
			
			Here is the other of the shadow mungers after the sort (these are
			examples of the toString() of a munger):
			
			(before: ((within(*) && execution(* main(..))) &&
			persingleton(A))->void A.ajc$before$A$1$ce209d4e())
			(before: ((within(*) && execution(* main(..))) &&
			persingleton(B))->void B.ajc$before$B$1$ce209d4e())
			
			
			Here is the advice sorting algorithm for M3:
			
			Collections.sort(
			        shadowMungerList,
			        new Comparator() {
			          public int compare(Object o1, Object o2) {
			            ShadowMunger sm1 = (ShadowMunger)o1;
			            ShadowMunger sm2 = (ShadowMunger)o2;
			            if (sm1.getSourceLocation()==null) return
			(sm2.getSourceLocation()==null?0:1);
			            if (sm2.getSourceLocation()==null) return -1;                      
			            return (sm2.getSourceLocation().getOffset()-sm1.getSourceLocation().getOffset());
			        }
			});
			
			On the same program, the order is now:
			
			(before: ((within(*) && execution(* main(..))) &&
			persingleton(B))->void B.ajc$before$B$1$ce209d4e())
			(before: ((within(*) && execution(* main(..))) &&
			persingleton(A))->void A.ajc$before$A$1$ce209d4e())
			
			The reason it was changed was because of @AJ syntax.  Consider the
			case where two pieces of advice *come from the same type* and hit the
			same join point.  The first point where they will differ in their
			toString() is in the name of the advice method generated
			(ajc$blahblahblah) - now in code style the name is generated by the
			compiler and is prefix "ajc$" advicekind "before$" sourcetype "A$"
			advice number within file "1$" hashcode of pointcut string "ce209d4e".
			 And so a sort on toString effectively sorts on the advice number
			within the file, if you have two source files, it will happen to sort
			by file then advice number within the file (since the type name
			happens to appear in the perclause when the munger is toString()'d)
			
			Now, in @AJ style, the *user* controls the name of the advice, so
			sorting by toString() is a little more unpredictable for multiple
			pieces of advice from the same file  (Seeing as how two pieces of
			advice could have the same name and just differ in terms of
			parameters)
			So to get back to some kind of at least predictable order (for the
			sake of writing testcases that expect certain output...), we switched
			to an algorithm that sorts by source location (actually offset within
			the file).  I could change it again to sort by source file name, then
			by source location which might get us back to the old ordering...  But
			if we just worked because you'd called your aspects A and B and they'd
			fail if they were renamed in the opposite alphabetical order then that
			wouldn't be ideal...
			
			It seems that todays choice of algorithm has flushed out
			(unfortunately for Ron) some issues to do with precedence being more
			important than anticipated.
			
			Look out for the M4 version which sorts advice based on how many times
			you've used the letter 'p' in your advice body.
			
			;)
			_______________________________________________
			aspectj-dev mailing list
			aspectj-dev@xxxxxxxxxxx
			https://dev.eclipse.org/mailman/listinfo/aspectj-dev
			


Back to the top