From 56a48272e61ecc506741fe8bbbcafeb0d199dda9 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 17 Apr 2009 17:28:31 +0000 Subject: [PATCH] avoiding synchronization as far as possible (SPR-5668) --- .../aspectj/AspectJExpressionPointcut.java | 63 ++++++++----------- ...ngletonAspectInstanceFactoryDecorator.java | 27 ++++---- .../aop/framework/AdvisedSupport.java | 6 +- .../aop/support/IntroductionInfoSupport.java | 16 ++--- 4 files changed, 48 insertions(+), 64 deletions(-) diff --git a/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java b/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java index 36ee8502e19..4d11d0a9764 100644 --- a/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java +++ b/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,10 +17,10 @@ package org.springframework.aop.aspectj; import java.lang.reflect.Method; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; @@ -28,6 +28,7 @@ import org.apache.commons.logging.LogFactory; import org.aspectj.weaver.BCException; import org.aspectj.weaver.patterns.NamePattern; import org.aspectj.weaver.reflect.ReflectionWorld; +import org.aspectj.weaver.reflect.ShadowMatchImpl; import org.aspectj.weaver.tools.ContextBasedMatcher; import org.aspectj.weaver.tools.FuzzyBoolean; import org.aspectj.weaver.tools.JoinPointMatch; @@ -92,7 +93,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut private static final Log logger = LogFactory.getLog(AspectJExpressionPointcut.class); - private final Map shadowMatchCache = new HashMap(); + private final Map shadowMatchCache = new ConcurrentHashMap(32); private PointcutParser pointcutParser; @@ -243,15 +244,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut public boolean matches(Method method, Class targetClass, boolean beanHasIntroductions) { checkReadyToMatch(); Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass); - ShadowMatch shadowMatch = null; - try { - shadowMatch = getShadowMatch(targetMethod, method); - } - catch (ReflectionWorld.ReflectionWorldException ex) { - // Could neither introspect the target class nor the proxy class -> - // let's simply consider this method as non-matching. - return false; - } + ShadowMatch shadowMatch = getShadowMatch(targetMethod, method); // Special handling for this, target, @this, @target, @annotation // in Spring - we can optimize since we know we have exactly this class, @@ -279,17 +272,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut public boolean matches(Method method, Class targetClass, Object[] args) { checkReadyToMatch(); - ShadowMatch shadowMatch = null; - ShadowMatch originalShadowMatch = null; - try { - shadowMatch = getShadowMatch(AopUtils.getMostSpecificMethod(method, targetClass), method); - originalShadowMatch = getShadowMatch(method, method); - } - catch (ReflectionWorld.ReflectionWorldException ex) { - // Could neither introspect the target class nor the proxy class -> - // let's simply consider this method as non-matching. - return false; - } + ShadowMatch shadowMatch = getShadowMatch(AopUtils.getMostSpecificMethod(method, targetClass), method); + ShadowMatch originalShadowMatch = getShadowMatch(method, method); // Bind Spring AOP proxy to AspectJ "this" and Spring AOP target to AspectJ target, // consistent with return of MethodInvocationProceedingJoinPoint @@ -364,24 +348,31 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) { - synchronized (this.shadowMatchCache) { - ShadowMatch shadowMatch = this.shadowMatchCache.get(targetMethod); - if (shadowMatch == null) { - try { - shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod); + ShadowMatch shadowMatch = this.shadowMatchCache.get(targetMethod); + if (shadowMatch == null) { + try { + shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod); + } + catch (ReflectionWorld.ReflectionWorldException ex) { + // Failed to introspect target method, probably because it has been loaded + // in a special ClassLoader. Let's try the original method instead... + if (targetMethod == originalMethod) { + shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); } - catch (ReflectionWorld.ReflectionWorldException ex) { - // Failed to introspect target method, probably because it has been loaded - // in a special ClassLoader. Let's try the original method instead... - if (targetMethod == originalMethod) { - throw ex; + else { + try { + shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod); + } + catch (ReflectionWorld.ReflectionWorldException ex2) { + // Could neither introspect the target class nor the proxy class -> + // let's simply consider this method as non-matching. + shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); } - shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod); } - this.shadowMatchCache.put(targetMethod, shadowMatch); } - return shadowMatch; + this.shadowMatchCache.put(targetMethod, shadowMatch); } + return shadowMatch; } diff --git a/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java b/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java index 34bc6cc181a..39d28fff7ed 100644 --- a/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java +++ b/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/annotation/LazySingletonAspectInstanceFactoryDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2007 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,11 +16,10 @@ package org.springframework.aop.aspectj.annotation; -import org.springframework.core.Ordered; import org.springframework.util.Assert; /** - * Decorator to cause a MetadataAwareAspectInstanceFactory to instantiate only once. + * Decorator to cause a {@link MetadataAwareAspectInstanceFactory} to instantiate only once. * * @author Rod Johnson * @author Juergen Hoeller @@ -30,7 +29,7 @@ public class LazySingletonAspectInstanceFactoryDecorator implements MetadataAwar private final MetadataAwareAspectInstanceFactory maaif; - private Object materialized; + private volatile Object materialized; /** @@ -42,30 +41,32 @@ public class LazySingletonAspectInstanceFactoryDecorator implements MetadataAwar this.maaif = maaif; } + public synchronized Object getAspectInstance() { if (this.materialized == null) { - this.materialized = this.maaif.getAspectInstance(); + synchronized (this) { + if (this.materialized == null) { + this.materialized = this.maaif.getAspectInstance(); + } + } } return this.materialized; } - public ClassLoader getAspectClassLoader() { - return this.maaif.getAspectClassLoader(); - } - public boolean isMaterialized() { return (this.materialized != null); } + public ClassLoader getAspectClassLoader() { + return this.maaif.getAspectClassLoader(); + } + public AspectMetadata getAspectMetadata() { return this.maaif.getAspectMetadata(); } public int getOrder() { - if (this.maaif instanceof Ordered) { - return ((Ordered) this.maaif).getOrder(); - } - return Ordered.LOWEST_PRECEDENCE; + return this.maaif.getOrder(); } diff --git a/org.springframework.aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java b/org.springframework.aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java index f5015a2c816..2835ed4be93 100644 --- a/org.springframework.aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java +++ b/org.springframework.aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -489,9 +489,7 @@ public class AdvisedSupport extends ProxyConfig implements Advised { * Invoked when advice has changed. */ protected void adviceChanged() { - synchronized (this.methodCache) { - this.methodCache.clear(); - } + this.methodCache.clear(); } /** diff --git a/org.springframework.aop/src/main/java/org/springframework/aop/support/IntroductionInfoSupport.java b/org.springframework.aop/src/main/java/org/springframework/aop/support/IntroductionInfoSupport.java index 979e5c8819a..c04c049163e 100644 --- a/org.springframework.aop/src/main/java/org/springframework/aop/support/IntroductionInfoSupport.java +++ b/org.springframework.aop/src/main/java/org/springframework/aop/support/IntroductionInfoSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,13 +21,11 @@ import java.io.ObjectInputStream; import java.io.Serializable; import java.lang.reflect.Method; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.aopalliance.intercept.MethodInvocation; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.aop.IntroductionInfo; import org.springframework.util.ClassUtils; @@ -44,11 +42,9 @@ import org.springframework.util.ClassUtils; */ public class IntroductionInfoSupport implements IntroductionInfo, Serializable { - protected transient Log logger = LogFactory.getLog(getClass()); + protected final Set publishedInterfaces = new HashSet(); - protected Set publishedInterfaces = new HashSet(); - - private transient Map rememberedMethods = new IdentityHashMap(32); + private transient Map rememberedMethods = new ConcurrentHashMap(32); /** @@ -119,10 +115,8 @@ public class IntroductionInfoSupport implements IntroductionInfo, Serializable { private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException { // Rely on default serialization; just initialize state after deserialization. ois.defaultReadObject(); - // Initialize transient fields. - this.logger = LogFactory.getLog(getClass()); - this.rememberedMethods = new IdentityHashMap(32); + this.rememberedMethods = new ConcurrentHashMap(32); } }