Uploaded image for project: 'logback'
  1. logback
  2. LOGBACK-639

Suggestion: Make LoggingEvent dumber, Logger more intelligent

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: logback-classic
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Bugzilla Id:
      134

      Description

      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

        Attachments

          Activity

            People

            • Assignee:
              logback-dev Logback dev list
              Reporter:
              joern@huxhorn.de Joern Huxhorn
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated: