You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Martin Marinschek <ma...@gmail.com> on 2006/02/19 11:36:44 UTC

Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java

ok - so that means we'll need to drop static?

won't that cause performance problems?

regards,

Martin

On 2/19/06, Simon Kitching <sk...@apache.org> wrote:
> On Sun, 2006-02-19 at 00:46 +0000, mmarinschek@apache.org wrote:
> > Author: mmarinschek
> > Date: Sat Feb 18 16:46:18 2006
> > New Revision: 378805
> >
> > URL: http://svn.apache.org/viewcvs?rev=378805&view=rev
> > Log:
> > minor changes in application-factory, fixed "readOnly" referral in HtmlRendererUtils
> >
> > Modified:
> >     myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
> >
> > Modified: myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
> > URL: http://svn.apache.org/viewcvs/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java?rev=378805&r1=378804&r2=378805&view=diff
> > ==============================================================================
> > --- myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java (original)
> > +++ myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java Sat Feb 18 16:46:18 2006
> > @@ -29,12 +29,14 @@
> >  public class ApplicationFactoryImpl
> >      extends ApplicationFactory
> >  {
> > -    private static final Log log = LogFactory.getLog(ApplicationFactory.class);
> > +    private static final Log log = LogFactory.getLog(ApplicationFactoryImpl.class);
>
>
> Just a warning to all developers: when using commons-logging in a
> library, STATIC fields must **NOT** be used to store Log objects.
>
> The problem is that the class may be called with various thread-context
> classloaders (TCCL) set. However the class is only ever loaded once.
>
> If the Log object is static, then it is initialised using the TCCL of
> the very first webapp to use it (ie which forces the class to be loaded)
> - including picking up config info from that TCCL. All other webapps
> will then use the same Log object - which is not healthy.
>
> Yes this is a nuisance, but it's absolutely necessary to avoid storing
> Log instances in static fields for libraries like MyFaces.
>
> Regards,
>
> Simon
>
>


--

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces

Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java

Posted by Simon Kitching <sk...@apache.org>.
On Sun, 2006-02-19 at 11:36 +0100, Martin Marinschek wrote:
> ok - so that means we'll need to drop static?

Yes. In fact, because MyFaces libs are often placed in a "shared"
classpath, static should be avoided in almost all cases I expect.

> 
> won't that cause performance problems?

Calling LogFactory.getLog is *reasonably* efficient.

The impact is trivial for long-lived objects like
ApplicationFactoryImpl. I wouldn't worry about performance for things
like UIComponent objects either, which have a lifetime of a request.

If there are very short-lived objects which are frequently created, then
it might be worth considering either:
(a) only fetching a Log object if logging is actually needed, ie
  if (some condition) {
    Log log = LogFactory.getLog(Foo.class);
    log.warn(....);
  }
(b) using the Log object stored on some other component.
  longLivedObject.getXYZLog().debug(....)
  Of course this works only when there is some suitable
  longer-lived object that is easily accessable from the
  short-lived object that wants to do logging.

Cheers,

Simon

> 
> regards,
> 
> Martin
> 
> On 2/19/06, Simon Kitching <sk...@apache.org> wrote:
> > On Sun, 2006-02-19 at 00:46 +0000, mmarinschek@apache.org wrote:
> > > Author: mmarinschek
> > > Date: Sat Feb 18 16:46:18 2006
> > > New Revision: 378805
> > >
> > > URL: http://svn.apache.org/viewcvs?rev=378805&view=rev
> > > Log:
> > > minor changes in application-factory, fixed "readOnly" referral in HtmlRendererUtils
> > >
> > > Modified:
> > >     myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
> > >
> > > Modified: myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
> > > URL: http://svn.apache.org/viewcvs/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java?rev=378805&r1=378804&r2=378805&view=diff
> > > ==============================================================================
> > > --- myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java (original)
> > > +++ myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java Sat Feb 18 16:46:18 2006
> > > @@ -29,12 +29,14 @@
> > >  public class ApplicationFactoryImpl
> > >      extends ApplicationFactory
> > >  {
> > > -    private static final Log log = LogFactory.getLog(ApplicationFactory.class);
> > > +    private static final Log log = LogFactory.getLog(ApplicationFactoryImpl.class);
> >
> >
> > Just a warning to all developers: when using commons-logging in a
> > library, STATIC fields must **NOT** be used to store Log objects.
> >
> > The problem is that the class may be called with various thread-context
> > classloaders (TCCL) set. However the class is only ever loaded once.
> >
> > If the Log object is static, then it is initialised using the TCCL of
> > the very first webapp to use it (ie which forces the class to be loaded)
> > - including picking up config info from that TCCL. All other webapps
> > will then use the same Log object - which is not healthy.
> >
> > Yes this is a nuisance, but it's absolutely necessary to avoid storing
> > Log instances in static fields for libraries like MyFaces.
> >
> > Regards,
> >
> > Simon
> >
> >
> 
> 
> --
> 
> http://www.irian.at
> 
> Your JSF powerhouse -
> JSF Consulting, Development and
> Courses in English and German
> 
> Professional Support for Apache MyFaces