+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
>
>

Reply via email to