commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From robert burrell donkin <robertburrelldon...@blueyonder.co.uk>
Subject Re: [logging] DON_QUIXOTE branch
Date Tue, 19 Apr 2005 19:54:18 GMT
(a long email but i hope it'll be worth the read)

On Tue, 2005-04-19 at 23:03 +1200, Simon Kitching wrote:
> On Thu, 2005-03-31 at 21:56 +0100, robert burrell donkin wrote:
> > i've committed some code into a branch called DON_QUIXOTE. it's
> > illustrative code showing how it's possible to lift off a simple
> > superclass from LogFactory. i've believe for some time that this is the
> > most important step forward. moving to a simplified API would allow
> > static binding (whether compile time ala UGLI or byte code manipulation)
> > to be offered in addition to (improved) dynamic binding. 
> > 
> > one aspect that has been holding me back is the increased complexity
> > that this choice gives. however, i'm now convinced that it really isn't
> > any use running away from the complexity: we need to cover advanced use
> > cases with better documentation as well as giving some intermediary
> > heuristic recipes (to stop JCL blowing up so much).
> > 
> > this kind of design is (i think) one way forward for JCL. opinions
> > welcomed (but please forgive the implementation: it's only
> > illustrative). 
> > 
> 
> I've had a look at the DON_QUIXOTE code. Below is how I understand it.
> Corrections are welcome (I just hope *some* of my understanding is
> correct!).
> 
> I see two parts to the code: "classic" and "kernel".
> 
> Kernel defines a new class LogManager.
> 
> Classic contains the LogFactory class, and the getLog methods simply
> delegate to equivalent static methods of a LogManager class. Presumably
> this is just a mechanism to keep backward compatibility, and logically
> people could also call LogManager.getLog. For backwards compatibility,
> the LogFactory class retains the same API.

+1

> LogManager provides a new and cleaner API for library code to
> instantiate Log objects. 

+1

(IMHO the existing interface is too cluttered to allow effective use of
static binding)  

> These are all static methods which simply
> delegate to a singleton instance of LogManager whose actual type is the
> result of a "discovery" process that is performed exactly once: when the
> class is first loaded.
>  * use a system property to specify the concrete LogManager subclass;
>  * use class LogFactory.LogManager, ie delegate straight back to the
>    old LogFactory implementation (via the new LogFactory.Manager
>    inner class).
>  * use itself as a final fallback (which just provides a trivial
>    log implementation).
> This "discovery" is actually meta-discovery; it discovers what the
> discovery mechanism will be, with the existing LogFactory implementation
> as the default.

+1

the meta-discovery mechanism outlined is really only illustrative.

> Note: Calling LogFactory.getLog is completely equivalent to calling
> LogManager.getLog.

+1

(at least, that's the plan)

> ============
> Benefits:
> 0) Backwards compatibility is maintained, though at the price
> of a couple of extra layers of indirection (method calls) for
> each call to getLog. Actually, maybe not that bad - the methods
> involved are static ones, so the JIT compiler should be able to
> inline these calls, reducing the price to just a single extra method
> call.

+1

> 1) LogManager may be deployed all on its own. In this case, a trivial
> log implementation is available - assuming the calling code has been
> modified to use LogManager.getLog rather than LogFactory.getLog. In
> other words, the "kernel" code will function on its own, though without
> any backwards-compatibility.

+1

> 2) Using a system property, it is possible to provide a completely
> different implementation of LogManager, ie one that doesn't behave
> anything like the current LogFactory. LogFactory currently allows users
> to set property org.apache.commons.logging.LogFactory, but that doesn't
> override the way that LogFactory keeps a HashMap of
> previously-instantiated LogFactory objects keyed by classloader. The new
> system allows *total* takeover of logging implementation by an
> alternative class.

yep

IMHO it should be an easier job to use static binding effectively. when
i get some time (unless someone beats me to it) i'll add a UGLI bridge
that is a compile time static replacement for LogManager

> I would agree that LogManager is a better interface than the existing
> LogFactory; LogFactory has too many public members exposed. And it is
> nice to be able to override *all* of logging, rather than just the
> "second half".

IMHO the main fault with JCL is that LogFactory exposes too large a
public API. other faults could be fixed without breaking compatibility
but not this one.

>  Limitations:
> 
> 0) When this new code is deployed in a shared classloader, the override
> of the LogManager implementation can only be done on a server-wide
> level. So the admin could say "all webapps will use XXX variant of
> discovery" or "all webapps will use YYY variant of discovery", but
> individual webapps still have no control.

yep

i've watched good developers try and fail to create a discovery silver
bullet over the years. i don't think that it's possible to create a
without knowing something about the environment in which it will be
deployed. 

i would encourage those applications needing strong compile time control
(for example, the application uses libraries using JCL but has a
particular chose of logging system for the rest of application) to use
static binding.   

> 0a) The old LogFactory had an override ability for the discovery process
> too: setting property org.apache.commons.logging.LogFactory. The primary
> ability of the new approach over the old one is that with the old one,
> discovery was *always* performed once for each distinct
> contextClassLoader and the results cached in a map keyed by
> contextClassLoader. With LogManager, user code can take another approach
> if it wishes, as the hook is at an "earlier" stage. However I can't see
> why we would want to take another approach; it seems to me that
> performing discovery separately for each distinct context-classloader is
> fundamental to the problem we are trying to solve.

there are a number of reasons why one might wish to take a different
approach. 

the commons-discovery is superior in many ways (but bigger). LogManager
would allow a commons-discovery backed implementation.

some exotic environments do not support this context classloader
approach. this applies to secured environments (for example untrusted
applets) and J2ME-like environments. JCL discovery has a tendency to
blow up when faced with these environments. better to use something much
simpler.

> 1) It may be necessary to add back release and releaseAll; the problem
> with providing an API behind which sits different logging
> implementations is that the API really needs to provide the necessary
> hooks for all possible implementations. There's no point providing the
> ability to swap in new implementations transparently if the API doesn't
> provide the hooks they need to function correctly. Alternatively, a
> getLogManager method to return the manager is needed, so it can then be
> cast to the appropriate type and the manager-specific methods called on
> it. Yes it's ugly.

hmmmm

the way i'd thought of approaching this would be that really only
LogFactory needs to be able to release all loggers. but you're right:
there's a good chance that many as-yet-unborn discovery mechanisms may
also need such a mechanism.

on reflection, you're right: probably releaseAll needs to be put into
(and maybe release as well) LogManager.

> 2) As you say, providing the backwards compatibility makes the control
> flow a bit complex. It's not too bad though..

yep

> 3) The ability to override the JCL implementation is only really useful
> while the correct implementation of JCL discovery is still in doubt. It
> means that if we screw up JCL discovery badly then users can plug in
> alternative implementations. If we get it right, then this ability to
> swap in new implementations is moot :-)

i've bigger ambitions :)

i want to be able to use static binding (both compile time and byte
code) for those tricky use cases.

i want to be able to have a version that runs (compiles?) on JVM 1.1
(1.0?).

i want to be able to use JCL on J2ME 

IMHO all these need a simpler API and the ability to replace LogFactory
with something both different and simpler.

> 4) While it provides the ability to completely override the
> implementation of JCL (on a per-server basis), it doesn't actually
> *solve* any of the problems we face. The issues currently in the
> LogFactory class are just pushed down one level...

i'm confident that those issues which can be solved in theory (given the
basic architecture) can be fixed. (there is going to be a price to pay,
though. more on this later tonight, i hope.)

> 5) On a very minor note, I think LogManager should have separate
>     public static log getLog(Class) and getLog(String) 
>   rather than rolling them into a getLog(Object) method.

as i said, it's really just illustrative

i wanted to see what the ultimate thin logging interface looked like.
only time will tell whether it'll be hell or heaven to live with...

> As a summary of my initial thoughts, I would agree this is a step
> forward. I'm not sure, however, that it's enough of a step forward to be
> worth basing a release around; I think we still need to address the
> issue of discovery and memory management and once we agree on that we
> can look back at this and decide whether also including the improved
> LogManager API in JCL is appropriate (probably will be).

that's fair. it's really intended to be illustrative of a direction for
JCL2 which allows backwards compatibility whilst also allowing more
exotic stuff like static binding.

> All this is just a dump of my immediate interpretation of the code. 
> I'm open to persuasion/correction on all points above..

seems a pretty fair analysis

> Thanks Robert for creating this; it's certainly a lot better looking at
> code than discussing abstract ideas by email :=). 
> 
> I hope I haven't misunderstood too much of this new code.

long life the branch :)

code is so much easier... 

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message