+1 for slf4j On Tue, May 3, 2011 at 4:23 AM, Rene Gielen <gie...@it-neering.net> wrote:
> There is another possible solution / improvement we might want to think > of: Switch our logging system to slf4j facade logging, which addresses - > besides a lot of other cool things - the parameter evaluation issue with > varags and late concatenation. > > See: > http://www.slf4j.org/ > http://www.slf4j.org/faq.html > > On 03.05.11 09:07, Rene Gielen wrote: > > Lukasz, > > > > why were you removing the logging gates, aka the if blocks around log > > statements? I agree that for simple String parameters (without > > concatenation) they might be left out, but for everything else they > > really help a lot for performance - that's why I use them in general. > > > > We should even consider reviewing S2 code if there are more unguarded > > log statements, to squeeze out the last bit of performance :) > > > > See also http://logging.apache.org/log4j/1.2/faq.html#a2.3 > > > > - René > > > > On 01.05.11 16:29, lukaszlen...@apache.org wrote: > >> Author: lukaszlenart > >> Date: Sun May 1 14:29:02 2011 > >> New Revision: 1098322 > >> > >> URL: http://svn.apache.org/viewvc?rev=1098322&view=rev > >> Log: > >> Removes unnecessary checking for log level and uses ConcurrentHashMap > instead of HashMap > >> > >> Modified: > >> > struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java > >> > >> Modified: > struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java > >> URL: > http://svn.apache.org/viewvc/struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java?rev=1098322&r1=1098321&r2=1098322&view=diff > >> > ============================================================================== > >> --- > struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java > (original) > >> +++ > struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java > Sun May 1 14:29:02 2011 > >> @@ -23,14 +23,14 @@ import com.opensymphony.xwork2.ObjectFac > >> import com.opensymphony.xwork2.util.logging.Logger; > >> import com.opensymphony.xwork2.util.logging.LoggerFactory; > >> > >> +import javax.enterprise.context.spi.CreationalContext; > >> import javax.enterprise.inject.spi.BeanManager; > >> import javax.enterprise.inject.spi.InjectionTarget; > >> -import javax.enterprise.context.spi.CreationalContext; > >> import javax.naming.Context; > >> import javax.naming.InitialContext; > >> import javax.naming.NamingException; > >> import java.util.Map; > >> -import java.util.HashMap; > >> +import java.util.concurrent.ConcurrentHashMap; > >> > >> /** > >> * CdiObjectFactory allows Struts 2 managed objects, like Actions, > Interceptors or Results, to be injected by a Contexts > >> @@ -52,7 +52,7 @@ public class CdiObjectFactory extends Ob > >> protected BeanManager beanManager; > >> protected CreationalContext ctx; > >> > >> - Map<Class<?>, InjectionTarget<?>> injectionTargetCache = new > HashMap<Class<?>, InjectionTarget<?>>(); > >> + Map<Class<?>, InjectionTarget<?>> injectionTargetCache = new > ConcurrentHashMap<Class<?>, InjectionTarget<?>>(); > >> > >> public CdiObjectFactory() { > >> super(); > >> @@ -76,25 +76,17 @@ public class CdiObjectFactory extends Ob > >> BeanManager bm; > >> try { > >> Context initialContext = new InitialContext(); > >> - if (LOG.isInfoEnabled()) { > >> - LOG.info("[findBeanManager]: Checking for BeanManager > under JNDI key " + CDI_JNDIKEY_BEANMANAGER_COMP); > >> - } > >> + LOG.info("[findBeanManager]: Checking for BeanManager under > JNDI key " + CDI_JNDIKEY_BEANMANAGER_COMP); > >> try { > >> bm = (BeanManager) > initialContext.lookup(CdiObjectFactory.CDI_JNDIKEY_BEANMANAGER_COMP); > >> - } catch ( NamingException e ) { > >> - if (LOG.isWarnEnabled()) { > >> - LOG.warn("[findBeanManager]: Lookup failed."); > >> - } > >> - if (LOG.isInfoEnabled()) { > >> - LOG.info("[findBeanManager]: Checking for > BeanManager under JNDI key " + CDI_JNDIKEY_BEANMANAGER_APP); > >> - } > >> + } catch (NamingException e) { > >> + LOG.warn("[findBeanManager]: Lookup failed.", e); > >> + LOG.info("[findBeanManager]: Checking for BeanManager > under JNDI key " + CDI_JNDIKEY_BEANMANAGER_APP); > >> bm = (BeanManager) > initialContext.lookup(CdiObjectFactory.CDI_JNDIKEY_BEANMANAGER_APP); > >> } > >> - if (LOG.isInfoEnabled()) { > >> - LOG.info("[findBeanManager]: BeanManager found."); > >> - } > >> + LOG.info("[findBeanManager]: BeanManager found."); > >> return bm; > >> - } catch ( NamingException e ) { > >> + } catch (NamingException e) { > >> LOG.error("Could not get BeanManager from JNDI context", > e); > >> } > >> return null; > >> @@ -102,7 +94,7 @@ public class CdiObjectFactory extends Ob > >> > >> @Override > >> @SuppressWarnings("unchecked") > >> - public Object buildBean( String className, Map<String, Object> > extraContext, boolean injectInternal ) > >> + public Object buildBean(String className, Map<String, Object> > extraContext, boolean injectInternal) > >> throws Exception { > >> > >> Class<?> clazz = getClassInstance(className); > >> @@ -124,19 +116,16 @@ public class CdiObjectFactory extends Ob > >> * will be created. > >> * > >> * @param clazz The class to get a InjectionTarget instance for. > >> - * > >> * @return if found in cache, an existing instance. A new instance > otherwise. > >> */ > >> - protected InjectionTarget<?> getInjectionTarget( Class<?> clazz ) { > >> + protected InjectionTarget<?> getInjectionTarget(Class<?> clazz) { > >> InjectionTarget<?> result; > >> - synchronized ( this ) { > >> - result = injectionTargetCache.get(clazz); > >> - if (result == null) { > >> - result = > beanManager.createInjectionTarget(beanManager.createAnnotatedType(clazz)); > >> - injectionTargetCache.put(clazz, result); > >> - } > >> - > >> + result = injectionTargetCache.get(clazz); > >> + if (result == null) { > >> + result = > beanManager.createInjectionTarget(beanManager.createAnnotatedType(clazz)); > >> + injectionTargetCache.put(clazz, result); > >> } > >> + > >> return result; > >> } > >> > >> @@ -144,11 +133,10 @@ public class CdiObjectFactory extends Ob > >> * Simple wrapper for CreationalContext creation. > >> * > >> * @param beanManager the BeanManager to use for creating the > context. > >> - * > >> * @return the context to use, if given BeanManager was not > <tt>null</tt>. <tt>null</tt> otherwise. > >> */ > >> @SuppressWarnings("unchecked") > >> - protected CreationalContext buildNonContextualCreationalContext( > BeanManager beanManager ) { > >> + protected CreationalContext > buildNonContextualCreationalContext(BeanManager beanManager) { > >> return beanManager != null ? > beanManager.createCreationalContext(null) : null; > >> } > >> } > >> > >> > > > > -- > René Gielen > IT-Neering.net > Saarstrasse 100, 52062 Aachen, Germany > Tel: +49-(0)241-4010770 > Fax: +49-(0)241-4010771 > Cel: +49-(0)163-2844164 > http://twitter.com/rgielen > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org > For additional commands, e-mail: dev-h...@struts.apache.org > >
