From 41d9980985b7081e032f5e88eb704dd3f43e874f Mon Sep 17 00:00:00 2001 From: aclement Date: Mon, 9 Nov 2009 19:32:57 +0000 Subject: [PATCH] timers and improved signature pattern matching --- .../src/org/aspectj/weaver/World.java | 57 ++++++++--- .../patterns/AnnotationTypePattern.java | 4 + .../patterns/AnyAnnotationTypePattern.java | 60 +++++++---- .../weaver/patterns/SignaturePattern.java | 99 +++++++++++++++++-- 4 files changed, 176 insertions(+), 44 deletions(-) diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/World.java b/org.aspectj.matcher/src/org/aspectj/weaver/World.java index e8f6cd27e..4f85f4023 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/World.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/World.java @@ -137,8 +137,10 @@ public abstract class World implements Dump.INode { public boolean forDEBUG_structuralChangesCode = false; public boolean forDEBUG_bridgingCode = false; public boolean optimizedMatching = true; + protected long timersPerJoinpoint; + protected long timersPerType; - public boolean infoMessagesEnabled = false; + public int infoMessagesEnabled = 0; // 0=uninitialized, 1=no, 2=yes private static Trace trace = TraceFactory.getTraceFactory().getTrace(World.class); @@ -631,7 +633,6 @@ public abstract class World implements Dump.INode { } else { this.messageHandler = messageHandler; } - infoMessagesEnabled = !messageHandler.isIgnoring(IMessage.INFO); } /** @@ -800,7 +801,10 @@ public abstract class World implements Dump.INode { } public boolean areInfoMessagesEnabled() { - return infoMessagesEnabled; + if (infoMessagesEnabled == 0) { + infoMessagesEnabled = (messageHandler.isIgnoring(IMessage.INFO) ? 1 : 2); + } + return infoMessagesEnabled == 2; } /** @@ -841,6 +845,8 @@ public abstract class World implements Dump.INode { public final static String xsetFAST_PACK_METHODS = "fastPackMethods"; // default true public final static String xsetOVERWEAVING = "overWeaving"; public final static String xsetOPTIMIZED_MATCHING = "optimizedMatching"; + public final static String xsetTIMERS_PER_JOINPOINT = "timersPerJoinpoint"; + public final static String xsetTIMERS_PER_FASTMATCH_CALL = "timersPerFastMatchCall"; public boolean isInJava5Mode() { return behaveInJava5Way; @@ -1348,6 +1354,22 @@ public abstract class World implements Dump.INode { getMessageHandler().handleMessage(MessageUtil.info("[optimizedMatching=false] optimized matching turned off")); } + s = p.getProperty(xsetTIMERS_PER_JOINPOINT, "25000"); + try { + timersPerJoinpoint = Integer.parseInt(s); + } catch (Exception e) { + getMessageHandler().handleMessage(MessageUtil.error("unable to process timersPerJoinpoint value of " + s)); + timersPerJoinpoint = 25000; + } + + s = p.getProperty(xsetTIMERS_PER_FASTMATCH_CALL, "250"); + try { + timersPerType = Integer.parseInt(s); + } catch (Exception e) { + getMessageHandler().handleMessage(MessageUtil.error("unable to process timersPerType value of " + s)); + timersPerType = 250; + } + } checkedAdvancedConfiguration = true; } @@ -1495,6 +1517,7 @@ public abstract class World implements Dump.INode { */ public void record(Pointcut pointcut, long timetaken) { if (timeCollector == null) { + ensureAdvancedConfigurationProcessed(); timeCollector = new TimeCollector(this); } timeCollector.record(pointcut, timetaken); @@ -1506,6 +1529,7 @@ public abstract class World implements Dump.INode { */ public void recordFastMatch(Pointcut pointcut, long timetaken) { if (timeCollector == null) { + ensureAdvancedConfigurationProcessed(); timeCollector = new TimeCollector(this); } timeCollector.recordFastMatch(pointcut, timetaken); @@ -1516,17 +1540,15 @@ public abstract class World implements Dump.INode { long joinpointCount; long typeCount; long perJoinpointCount; + long perTypes; Map joinpointsPerPointcut = new HashMap(); Map timePerPointcut = new HashMap(); Map fastMatchTimesPerPointcut = new HashMap(); + Map fastMatchTypesPerPointcut = new HashMap(); TimeCollector(World world) { - try { - this.perJoinpointCount = Integer.parseInt(System.getProperty("org.aspectj.timing.perjoinpoints", "25000")); - } catch (Throwable t) { - System.err.println("Problem reading property 'org.aspectj.timing.perjoinpoints':" + t.toString()); - this.perJoinpointCount = 25000; - } + this.perJoinpointCount = world.timersPerJoinpoint; + this.perTypes = world.timersPerType; this.world = world; this.joinpointCount = 0; this.typeCount = 0; @@ -1558,7 +1580,7 @@ public abstract class World implements Dump.INode { totalTime += timePerPointcut.get(p); } world.getMessageHandler().handleMessage( - MessageUtil.info("Pointcut matching cost (total=" + (totalTime / 1000000) + "ms):")); + MessageUtil.info("Pointcut matching cost (total=" + (totalTime / 1000000) + "ms for "+joinpointCount+" joinpoint match calls):")); for (String p : joinpointsPerPointcut.keySet()) { StringBuffer sb = new StringBuffer(); sb.append("Time:" + (timePerPointcut.get(p) / 1000000) + "ms (jps:#" + joinpointsPerPointcut.get(p) @@ -1572,6 +1594,14 @@ public abstract class World implements Dump.INode { void recordFastMatch(Pointcut pointcut, long timetakenInNs) { typeCount++; String pointcutText = pointcut.toString(); + Long typecounter = fastMatchTypesPerPointcut.get(pointcutText); + if (typecounter == null) { + typecounter = 1L; + } else { + typecounter++; + } + fastMatchTypesPerPointcut.put(pointcutText, typecounter); + Long time = fastMatchTimesPerPointcut.get(pointcutText); if (time == null) { time = timetakenInNs; @@ -1579,16 +1609,17 @@ public abstract class World implements Dump.INode { time += timetakenInNs; } fastMatchTimesPerPointcut.put(pointcutText, time); - if ((typeCount % 250) == 0) { + if ((typeCount % perTypes) == 0) { long totalTime = 0L; for (String p : fastMatchTimesPerPointcut.keySet()) { totalTime += fastMatchTimesPerPointcut.get(p); } world.getMessageHandler().handleMessage( - MessageUtil.info("Pointcut fast matching cost (total=" + (totalTime / 1000000) + "ms):")); + MessageUtil.info("Pointcut fast matching cost (total=" + (totalTime / 1000000) + "ms for " + typeCount + + " fast match calls):")); for (String p : fastMatchTimesPerPointcut.keySet()) { StringBuffer sb = new StringBuffer(); - sb.append("Time:" + (fastMatchTimesPerPointcut.get(p) / 1000000) + "ms fast matching against " + p); + sb.append("Time:" + (fastMatchTimesPerPointcut.get(p) / 1000000) + "ms (types:#"+fastMatchTypesPerPointcut.get(p)+") fast matching against " + p); world.getMessageHandler().handleMessage(MessageUtil.info(sb.toString())); } world.getMessageHandler().handleMessage(MessageUtil.info("---")); diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnnotationTypePattern.java b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnnotationTypePattern.java index 4013f79ce..1d0a1cedf 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnnotationTypePattern.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnnotationTypePattern.java @@ -147,4 +147,8 @@ class EllipsisAnnotationTypePattern extends AnnotationTypePattern { return this; } + @Override + public void setForParameterAnnotationMatch() { + } + } \ No newline at end of file diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnyAnnotationTypePattern.java b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnyAnnotationTypePattern.java index 4a2f30af3..245807df0 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnyAnnotationTypePattern.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/AnyAnnotationTypePattern.java @@ -9,7 +9,7 @@ * Contributors * Andy Clement - extracted from AnnotationTypePattern * ******************************************************************/ - package org.aspectj.weaver.patterns; +package org.aspectj.weaver.patterns; import java.io.DataOutputStream; import java.io.IOException; @@ -17,39 +17,57 @@ import java.util.Map; import org.aspectj.util.FuzzyBoolean; import org.aspectj.weaver.AnnotatedElement; -import org.aspectj.weaver.World; import org.aspectj.weaver.ResolvedType; +import org.aspectj.weaver.World; public class AnyAnnotationTypePattern extends AnnotationTypePattern { - public FuzzyBoolean fastMatches(AnnotatedElement annotated) { - return FuzzyBoolean.YES; - } - + @Override + public FuzzyBoolean fastMatches(AnnotatedElement annotated) { + return FuzzyBoolean.YES; + } + + @Override public FuzzyBoolean matches(AnnotatedElement annotated) { return FuzzyBoolean.YES; } - - public FuzzyBoolean matches(AnnotatedElement annotated,ResolvedType[] parameterAnnotations) { + + @Override + public FuzzyBoolean matches(AnnotatedElement annotated, ResolvedType[] parameterAnnotations) { return FuzzyBoolean.YES; } + @Override public void write(DataOutputStream s) throws IOException { s.writeByte(AnnotationTypePattern.ANY_KEY); } - + + @Override public void resolve(World world) { } - - public String toString() { return "@ANY"; } - - public Object accept(PatternNodeVisitor visitor, Object data) { - return visitor.visit(this, data); - } - - public boolean isAny() { return true; } - - public AnnotationTypePattern parameterizeWith(Map arg0,World w) { - return this; - } + + @Override + public String toString() { + return "@ANY"; + } + + @Override + public Object accept(PatternNodeVisitor visitor, Object data) { + return visitor.visit(this, data); + } + + @Override + public boolean isAny() { + return true; + } + + @Override + public AnnotationTypePattern parameterizeWith(Map arg0, World w) { + return this; + } + + @Override + public void setForParameterAnnotationMatch() { + + } } \ No newline at end of file diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/SignaturePattern.java b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/SignaturePattern.java index 3ca7c2d2f..dba4dcbc0 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/SignaturePattern.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/SignaturePattern.java @@ -47,6 +47,10 @@ public class SignaturePattern extends PatternNode { private TypePattern declaringType; private NamePattern name; private TypePatternList parameterTypes; + private int bits = 0x0000; + private static final int PARAMETER_ANNOTATION_MATCHING = 0x0001; + private static final int CHECKED_FOR_PARAMETER_ANNOTATION_MATCHING = 0x0002; + private ThrowsPattern throwsPattern; private AnnotationTypePattern annotationPattern; private transient int hashcode = -1; @@ -456,24 +460,47 @@ public class SignaturePattern extends PatternNode { if (subjectMatch && !throwsPattern.matches(aMethod.getExceptions(), world)) { return FuzzyBoolean.NO; } - if (!declaringType.matchesStatically(aMethod.getDeclaringType().resolve(world))) { - return FuzzyBoolean.MAYBE; - } - if (!returnType.matchesStatically(aMethod.getReturnType().resolve(world))) { - // looking bad, but there might be parameterization to consider... - if (!returnType.matchesStatically(aMethod.getGenericReturnType().resolve(world))) { - // ok, it's bad. + + // '*' trivially matches everything, no need to check further + if (!declaringType.isStar()) { + if (!declaringType.matchesStatically(aMethod.getDeclaringType().resolve(world))) { return FuzzyBoolean.MAYBE; } } + + // '*' would match any return value + if (!returnType.isStar()) { + if (!returnType.matchesStatically(aMethod.getReturnType().resolve(world))) { + // looking bad, but there might be parameterization to consider... + if (!returnType.matchesStatically(aMethod.getGenericReturnType().resolve(world))) { + // ok, it's bad. + return FuzzyBoolean.MAYBE; + } + } + } + + // The most simple case: pattern is (..) will match anything + if (parameterTypes.size() == 1 && parameterTypes.get(0).isEllipsis()) { + return FuzzyBoolean.YES; + } + if (!parameterTypes.canMatchSignatureWithNParameters(aMethod.getParameterTypes().length)) { return FuzzyBoolean.NO; } + + // OPTIMIZE both resolution of these types and their annotations should be deferred - just pass down a world and do it lower + // down ResolvedType[] resolvedParameters = world.resolve(aMethod.getParameterTypes()); - ResolvedType[][] parameterAnnotationTypes = aMethod.getParameterAnnotationTypes(); - if (parameterAnnotationTypes == null || parameterAnnotationTypes.length == 0) { - parameterAnnotationTypes = null; + + // Only fetch the parameter annotations if the pointcut is going to be matching on them + ResolvedType[][] parameterAnnotationTypes = null; + if (isMatchingParameterAnnotations()) { + parameterAnnotationTypes = aMethod.getParameterAnnotationTypes(); + if (parameterAnnotationTypes != null && parameterAnnotationTypes.length == 0) { + parameterAnnotationTypes = null; + } } + if (!parameterTypes.matches(resolvedParameters, TypePattern.STATIC, parameterAnnotationTypes).alwaysTrue()) { // It could still be a match based on the generic sig parameter types of a parameterized type if (!parameterTypes.matches(world.resolve(aMethod.getGenericParameterTypes()), TypePattern.STATIC, @@ -493,6 +520,58 @@ public class SignaturePattern extends PatternNode { return FuzzyBoolean.YES; } + /** + * Determine if any pattern in the parameter type pattern list is attempting to match on parameter annotations. + * + * @return true if a parameter type pattern wants to match on a parameter annotation + */ + private boolean isMatchingParameterAnnotations() { + if ((bits & CHECKED_FOR_PARAMETER_ANNOTATION_MATCHING) == 0) { + bits |= CHECKED_FOR_PARAMETER_ANNOTATION_MATCHING; + for (int tp = 0, max = parameterTypes.size(); tp < max; tp++) { + TypePattern typePattern = parameterTypes.get(tp); + if (isParameterAnnotationMatching(typePattern)) { + bits |= PARAMETER_ANNOTATION_MATCHING; + } + } + } + return (bits & PARAMETER_ANNOTATION_MATCHING) != 0; + } + + /** + * Walk the simple structure of a type pattern and determine if any leaf node is involved in parameter annotation matching. + */ + private boolean isParameterAnnotationMatching(TypePattern tp) { + if (tp instanceof OrTypePattern) { + OrTypePattern orAtp = (OrTypePattern) tp; + return (isParameterAnnotationMatching(orAtp.getLeft()) || isParameterAnnotationMatching(orAtp.getRight())); + } else if (tp instanceof AndTypePattern) { + AndTypePattern andAtp = (AndTypePattern) tp; + return (isParameterAnnotationMatching(andAtp.getLeft()) || isParameterAnnotationMatching(andAtp.getRight())); + } else if (tp instanceof NotTypePattern) { + NotTypePattern notAtp = (NotTypePattern) tp; + return (isParameterAnnotationMatching(notAtp.getNegatedPattern())); + } else { + AnnotationTypePattern atp = tp.getAnnotationPattern(); + return isParameterAnnotationMatching(atp); + } + } + + private boolean isParameterAnnotationMatching(AnnotationTypePattern tp) { + if (tp instanceof OrAnnotationTypePattern) { + OrAnnotationTypePattern orAtp = (OrAnnotationTypePattern) tp; + return (isParameterAnnotationMatching(orAtp.getLeft()) || isParameterAnnotationMatching(orAtp.getRight())); + } else if (tp instanceof AndAnnotationTypePattern) { + AndAnnotationTypePattern andAtp = (AndAnnotationTypePattern) tp; + return (isParameterAnnotationMatching(andAtp.getLeft()) || isParameterAnnotationMatching(andAtp.getRight())); + } else if (tp instanceof NotAnnotationTypePattern) { + NotAnnotationTypePattern notAtp = (NotAnnotationTypePattern) tp; + return (isParameterAnnotationMatching(notAtp.negatedPattern)); + } else { + return tp.isForParameterAnnotationMatch(); + } + } + private ResolvedType[] getResolvedParameters(World world, UnresolvedType[] unresolvedParams) { return world.resolve(unresolvedParams); } -- 2.39.5