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.
LogManager provides a new and cleaner API for library code to
instantiate Log objects. 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.
Note: Calling LogFactory.getLog is completely equivalent to calling
LogManager.getLog.
============
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) 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.
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.
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".
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.
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.
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.
2) As you say, providing the backwards compatibility makes the control
flow a bit complex. It's not too bad though..
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 :-)
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...
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 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).
All this is just a dump of my immediate interpretation of the code.
I'm open to persuasion/correction on all points above..
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.
Cheers,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
