[
http://jira.qos.ch/browse/LBCLASSIC-45?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=10577#action_10577
]
Ralph Goers commented on LBCLASSIC-45:
--------------------------------------
CallerData isn't normally resolved until something wants it. This is done
presumably for performance reasons. The change you suggest could potentially
make logging much slower (although I haven't benchmarked it). However, the way
it is currently implemented, if the LoggingEvent was passed to another thread
for asynchronous processing calling getCallerData() there would yield incorrect
results.
> Suggestion: Make LoggingEvent dumber, Logger more intelligent
> -------------------------------------------------------------
>
> Key: LBCLASSIC-45
> URL: http://jira.qos.ch/browse/LBCLASSIC-45
> Project: logback-classic
> Issue Type: Improvement
> Components: Other
> Affects Versions: unspecified
> Environment: Operating System: All
> Platform: All
> Reporter: Joern Huxhorn
> Assignee: Logback dev list
> Priority: Minor
>
> 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
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.qos.ch/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
_______________________________________________
logback-dev mailing list
[email protected]
http://qos.ch/mailman/listinfo/logback-dev
