On Nov 9, 2005, at 09:31, Jeremias Maerki wrote:
Jeremias / Finn,
I agree that we should keep an eye on logging. What you propose is
certainly an option. Some random thoughts:
- I wonder how it would affect the source code (readbility). You'd
have
to write a debug() method in each class that logs to a separate
logger.
Not really. You could, for example give FONode the following method:
final void debug(Logger log, String msg) {
if(DEBUG) {
log.debug(msg);
}
}
So all its subclasses can use this one method, and pass in their own
log instance to send the output to. Qua readability it becomes: the
current 'log.debug(msg)' vs. 'debug(log, msg)'. Not such a big deal,
but not really an improvement either...
Finn's response would lead me to conclude that, unless the body of
the method is reasonably large, this may not be worth the trouble. A
bit of googling around seems to indicate that JIT already performs
rather aggressive inlining (even of non-final methods, if they are
never overridden --the most important example being private methods)
Although in the end, the benefit remains: because the debug() method
is inlined, this means that when DEBUG=true, the 'invokevirtual' call
in the bytecode will be replaced by the body of the method being
called. Since with DEBUG=false, that body is empty, no code gets
executed (there is no penalty compared to not making the call to the
inlined method)
In any case, it's more the thought that counts here than the precise
way to go about it. I remember similar issues being raised in the
past (unfortunately the threads remained rather short; probably
because there really isn't *that* much to say about it)
- Right now I don't know of any bigger component/project that does
this.
Most just rely on the configurability of the logging backend. If
you use
log.is*Enabled() you lose only very little compared to the conditional
compilation.
Right, and in combination with JIT, is*Enabled() should indeed be
more than sufficient.
<snip />
- The parts that will move to Commons should eventually get freed
of the
dependency on Commons Logging as agreed.
I'm curious: will those parts simply not be logged --all logging-
related code removed--, or would eventual output end up on System.out
or System.err, or would these parts be so perfect that logging simply
won't be necessary? Stupid question! The last option, of course :-)
To your questions:
1) As indicated above I don't think the penalty of the current
approach
is big if is*Enabled() is used where Strings are concatenated. And I
don't think the logging statements add too much to the class sizes.
Better make FOP more modular for people wanting to scale down: SVG/
Batik as
optional extension, Renderer pluggability as I implemented it and will
commit later today. For the latter we could change the build to make
certain renderers optional or exclude them from the build process
(configurable).
Agreed.
2a) pretty much shows why this approach may not be flexible enough. I
like having the ability to enable the debug level just for certain
element
lists (that's why each category is logged to a separate logger) and
the
layout managers leaving everything else to "INFO".
See above: the concern for 'inflexibility' is rather easily worked
around, and you wouldn't lose the ability to set the loglevel of
different classes differently, as the loglevel is an attribute of the
log instance that is passed in. Expand it to have three parameters to
be able to feed it the severity of the message:
final void debug(Logger log, String msg, int level) {
if (DEBUG) {
switch (level) {
case LEVEL_DEBUG: log.debug(msg); break;
case LEVEL_TRACE: log.trace(msg); break;
...
2b) Really just see to it that we clean out unhelpful logging
statements
and that is*Enabled() is used where necessary.
Yup! But that one takes care of not performing the String
concatenation if it's not necessary to do so (which I erroneously
presumed to be the case for the final void debug()), so it operates
at the point where the msg String gets built. (= outside the scope of
the described debug method, but that could be changed... I mean: if
the debug() method would receive an Object parameter instead of a
String, and based on the state of that Object the message is built
inside the body of the debug() method, it would be optimized away)
Bottom-line (FTM): the proposal seems to be more trouble than it's
worth, especially if the need arises for activating debugging at run-
time. (although it still remains interesting from a theoretical
perspective, for those who want to expand their knowledge of Java
compilation/bytecode interpretation)
Thanks to the both of you for your input (and to Finn for correcting
my error in interpreting the effects on String.intern() --would
better have checked that before sending, but anyway...)
Cheers,
Andreas
