[logback-dev] [Bug 134] New: Suggestion: Make LoggingEvent dumber, Logger more intelligent

[bugzilla-daemon at pixie.qos.ch]

http://bugzilla.qos.ch/show_bug.cgi?id=134

           Summary: Suggestion: Make LoggingEvent dumber, Logger more
                    intelligent
           Product: logback-classic
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P1
         Component: Other
        AssignedTo: logback-dev at qos.ch
        ReportedBy: joern at huxhorn.de


This change would introduce incompatibilities with previous versions but would
increase general logging performance and decrease size of serialized events.

After I have turned you off sufficiently ;), let me explain my idea:

At the moment, LoggingEvent is handling it's own initialization. It even
contains measures against manipulation after initialization in that a call of a
setter results in an IllegalStateException if the attribute has already been
initialized before.

Instead, I'd suggest that LoggingEvent is changed into a dumb data container
with getter and setters for all it's attributes.

If it's really necessary to have a semi-const LoggingEvent I'd suggest to
define LoggingEvent as an interface containing only the getters of the
attributes and an implementation LoggingEventImpl that implements LoggingEvent
including getters and setters.

This communicates the *intent* that LoggingEvents aren't supposed to be
modified but nothing more. It would still be possible to change e.g. the
content of an array like argumentArray. Even if the getter would return a copy
of the contained array, generating heat in the process, it would *still* be
possible to brutally manipulate the LoggingEvent using java.lang.reflection
setAccessible(true) so it's just not possible to guarantee constness.

Instead of having a pseudo-const LoggingEvent let's dump the constness
altogether and make it a dumb data container that contains no logic at all.
This would instead be moved to the Logger class.

I'd also suggest to remove formattedMessage and change the argumentArray to
String[].

This is possible because logback-classic/slf4j promises formatted message, not
LoggingEvent.

It would increase performance because a Logger would simply create a
LoggingEvent without performing a possibly unnecessary formatting of the
message. Creation of LoggingEvents should be as fast and simple as possible,
obviously.

The formatting of the message should/would instead be performed in the appender
(or, more generally, user of the event) if required.

A serializing appender, an xml appender (e.g. using java.beans.XMLEncoder) or a
socket appender would *not* require a formatted message, executing
significantly faster than before.

The argumentArray should be String[] to securely prevent
java.io.NotSerializableException in the serializer and .
java.lang.ClassNotFoundException in the deserializer.

LoggerRemoteView and LoggerContextRemoteView should be changed the same way as
LoggingEvent, i.e. just data container, logic moved to Logger, for similar
reasons.

I have to admit, though, that I currently don't understand the reason the
LoggingEvent needs to know about LoggerContext beside resolving of the Logger
name. I couldn't find code that uses LoggerContextRemoteView.getPropertyMap(),
beside loading and saving it.
So I think it may be possible that LoggerRemoteView isn't really necessary and
could be removed from LoggingEvent... remembering the Logger name instead.

fqnOfLoggerClass could be omitted because the CallerData would be created in
the Logger where fqnOfLoggerClass is known.

And last but not least I'd also change Level into an java.lang.Enum.

I don't expect that all this will be done but I wanted to suggest it
nevertheless because it would both increase performance and produce cleaner
code.

Breaking compatibility would IMHO be worth it - after all, logback isn't 1.0,
yet ;)

If you are interested I'd volunteer to implement those changes, after signing
http://logback.qos.ch/cla.txt of course.

Keywords:
Inversion of Control (IoC), separation of concerns, KISS principle

Related bugs:
http://bugzilla.qos.ch/show_bug.cgi?id=100
http://bugzilla.qos.ch/show_bug.cgi?id=118
http://bugzilla.slf4j.org/show_bug.cgi?id=70


-- 
Configure bugmail: http://bugzilla.qos.ch/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.