[Bug 31] Varargs for Logger methods

http://bugzilla.slf4j.org/show_bug.cgi?id=31 --- Comment #90 from Joern Huxhorn <joern@huxhorn.de> 2011-09-07 11:32:28 CEST --- (In reply to comment #89)
A new proposal (and code):
1. Release 2.0 targeting Java 5
Agreed, but it must be possible to use 1.6.x and 2.0 side-by-side.
Those stuck on 1.4 aren't looking for the latest and greatest features. SLF4J 1.6.2 should be sufficient.
2. Add varargs with Google Collections/Guava style methods
log*(String format, String arg1, ..., String arg6) log*(String format, String arg1, ..., String arg6, String... others)
There is a performance penalty for creating the implicit array required for varargs. A quick benchmark shows this to be about 1 nanosecond. Small, but why not eliminate it for nearly all use cases (6 or fewer args). As a bonus with this approach, the existing one and two arg methods will fit in naturally with this style API. The log*(format, Object[] arg) methods should be retained for compatibility.
This will require many new methods, but it is not as bad as it sounds. See below.
I can't help but feel dirty defining an interface like this... Even taking into account the PDF linked in comment #82, I think "optimizing" an interface like this is pretty much premature optimization. "Premature" since the next JVM (which is constantly enhanced and optimized) could suddenly implement optimizations that could result in nearly identical or even same/better performance. You can never get rid of those additional methods, ever, without having this very same discussion and having another compatibility break. I'm aware that this is a rather purist point of view but I also think that I have some very valid points...
4. Simplify the logger adapter contract
It seems that compatibility challenges may be largely due to the front end API (used by application writers) is tightly bound to the adapter API (used by logging platform developers).
The minimum set of methods required for an adapter is two (three if LocationAwareLogger is implemented):
protected abstract boolean isEnabled(Marker marker, Level level); protected abstract void log(Marker marker, Level level, String msg, Throwable t);
All maybeLog(...)-methods in NamedLoggerBase are calling the same private void log(Marker marker, Level level, String msg, Throwable t) method, already formatting the message and dropping any info about the argument array and message pattern that was used to call it. This is rather bad for several reasons: - not every logging framework/appender requires the rather expensive formatting to take place at this point or at all. Both my Logback SocketAppender and Encoder are using the raw message pattern and the arguments transformed into String instead of the formatted message. This reduces the amount of time required during the actual logging and moves the formatting into the code that is used for logging event consumption, resulting in increased performance of the logged application. - We filter on the unformatted message pattern regularly since it is much more straightforward than using the formatted message and creating a potentially very complex RegEx to catch (or rather: ignore) a specific logging call. - since losing those abilities is not an option, Logback can't use NamedLoggerBase and would therefore be required to reimplement all new methods on it's own. It also wouldn't be able to extend AbstractLogger for the same reason.
An AbstractLogger class would implement all application facing API methods. Recommending (or requiring) log adapters to extend AbstractLogger and implement the two methods above would effectively unbind the front end and adapter APIs. New Logger methods could be added. Formatting would no longer be handled by adapters. Adapter lines of code and risk of bugs would be greatly reduced.
The fact that (optional) formatting is happening in the adapters is not an issue but a feature. Message formatting is an implementation detail and different frameworks can handle it in different ways (e.g. postpone it as in my Logback example).
Loggers extending AbstractLogger would of course require the 2.0 API at runtime, with the long term benefit of simplified code and future compatibility.
I see what you are trying to achieve but it won't work, at least not in the way you've implemented it right now. Please check out comment #61, comment #69, comment #78 and comment #86 I'd really like to see the Logger extended with methods that can handle arbitrary Message implementations in addition to the current messagePattern+arguments style. Something like this has been frequently requested, i.e. being able to log with arbitrary attached objects that can still be accessed in the (Logback) appender. The way to achieve this at the moment is serializing them to a String in some way, putting them into the MDC with a name known by the appender and deserializing them in the appender by parsing in that String retrieved from the MDC. This, obviously, is less than ideal concerning performance. Cheers, Joern. -- Configure bugmail: http://bugzilla.slf4j.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug.
participants (1)
-
bugzilla-daemon@pixie.qos.ch