Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.10.0
    • Fix Version/s: 0.10.0
    • Component/s: None
    • Labels:
      None
    • Environment:
      n/a

      Description

      A stream which provides a nice abstracted way to only do formatting when your current log level is enabled. See list archives for details.

      #ifndef LOGGER_STREAM_HPP__INCLUDE
      #define LOGGER_STREAM_HPP__INCLUDE

      #include <log4cxx/logger.h>
      #ifndef DISABLE_IOSTREAMS
      #include <sstream>
      #endif
      #include <cassert>

      template<typename B>
      class LoggerStream {
      public:
      typedef enum

      { endl }

      modifiers;
      typedef B buffer_type;
      typedef LoggerStream<buffer_type> self_type;

      inline LoggerStream(log4cxx::LoggerPtr& aLogger,
      const log4cxx::LevelPtr& aLevel,
      const char* aFile = 0, int aLine = -1)
      : logger(aLogger),
      level(aLevel),
      file(aFile),
      line(aLine),
      buffer((aLogger->isEnabledFor(aLevel)) ? (new buffer_type()) : 0) {
      }

      inline ~LoggerStream()

      { delete(buffer); //no need to check for null }

      inline bool isEnabled() const

      { return buffer != 0; }

      template<typename T>
      inline LoggerStream& operator<<(const T& aValue) {
      if (isEnabled())

      { (*buffer) << aValue; }

      return *this;
      }

      inline LoggerStream& operator<<(const modifiers& aValue)

      { assert(aValue == endl); flush(); }

      inline LoggerStream& flush() {
      if (isEnabled())

      { logger->forcedLog(level, buffer->str().c_str(), file, line); }

      return *this;
      }

      private:
      log4cxx::LoggerPtr& logger;
      const log4cxx::LevelPtr& level;
      const char* file;
      const int line;
      buffer_type* const buffer;
      };

      //helpful macros that use STL IOSTREAMS
      #define LOG_STREAM_EX(stream_type, logger, level) LoggerStream<stream_type>(logger, level, _FILE, __LINE_)
      #define LOG_STREAM(logger, level) LOG_STREAM_EX(std::ostringstream, logger, level)
      #define WLOG_STREAM(logger, level) LOG_STREAM_EX(std::wostringstream, logger, level)
      #define ENDL LoggerStream<std::ostringstream>::endl
      #define WENDL LoggerStream<std::wostringstream>::endl
      #endif

      1. LoggerStream.hpp
        2 kB
        Christopher Smith
      2. stream.patch2
        8 kB
      3. benchmark.tar.gz
        2 kB
        Christopher Smith
      4. stream.patch
        19 kB
        Curt Arnold
      5. LoggerStream.hpp
        2 kB
        Christopher Smith

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        15d 10h 41m 1 Curt Arnold 22/Oct/04 05:59
        In Progress In Progress Resolved Resolved
        1209d 13h 50m 1 Curt Arnold 13/Feb/08 18:50
        Curt Arnold made changes -
        Fix Version/s 0.10.0 [ 10782 ]
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Curt Arnold added a comment -

        The current logstream implementation is relatively decent. Closing this issue.

        Show
        Curt Arnold added a comment - The current logstream implementation is relatively decent. Closing this issue.
        Hide
        Curt Arnold added a comment -

        In a message (http://marc.theaimsgroup.com/?l=log4cxx-user&m=115280471300714&w=2) on log4cxx-user on 2006-07-13, David M. Lee wrote:

        I've just been hit by an oddity about the way logstream behaves on a system that has wchar_t support - even if you don't use (or care) about wide characters.

        On such a system, logstream derives from basic_ios<wchar_t>. This works fine for most native types, but means that operator<< for ArbitraryType will try to invoke operator<<(wostream, ArbitraryType) - the wide character streaming operator.

        If your type only defines operator<<(ostream, ArbitraryType), the compile will fail with a big pile of STL errors (well, one error of no match for 'operator<<', with about a gazillion candidates).

        There are two solutions to this problem.

        The first is to change my code to be wide-output friendly: operator<<(basic_ostream<C>, ArbitraryType). Easy enough, but produces a lot of clutter in code that will never need wide character support.

        What would be preferable would be for logstream to become a more direct model of ostream - meaning that you have a basic_logstream<C>, with 'typedef basic_logstream<char> logstream' and 'typedef basic_logstream<wchar_t> wlogstream'.

        Thoughts?
        dave

        Show
        Curt Arnold added a comment - In a message ( http://marc.theaimsgroup.com/?l=log4cxx-user&m=115280471300714&w=2 ) on log4cxx-user on 2006-07-13, David M. Lee wrote: I've just been hit by an oddity about the way logstream behaves on a system that has wchar_t support - even if you don't use (or care) about wide characters. On such a system, logstream derives from basic_ios<wchar_t>. This works fine for most native types, but means that operator<< for ArbitraryType will try to invoke operator<<(wostream, ArbitraryType) - the wide character streaming operator. If your type only defines operator<<(ostream, ArbitraryType), the compile will fail with a big pile of STL errors (well, one error of no match for 'operator<<', with about a gazillion candidates). There are two solutions to this problem. The first is to change my code to be wide-output friendly: operator<<(basic_ostream<C>, ArbitraryType). Easy enough, but produces a lot of clutter in code that will never need wide character support. What would be preferable would be for logstream to become a more direct model of ostream - meaning that you have a basic_logstream<C>, with 'typedef basic_logstream<char> logstream' and 'typedef basic_logstream<wchar_t> wlogstream'. Thoughts? dave
        Christopher Smith made changes -
        Attachment LoggerStream.hpp [ 18014 ]
        Hide
        Christopher Smith added a comment -

        Took another run at the alternative approach which attempts to address most of the issues with the original LoggerStream I submitted. A number of methods still need to be overridden (actually the same is true for stream.h in CVS), but it does have most of the key traits that have been discussed. It turns out that I was getting no measurable performance win by preventing the log level from changing (classic case of premature optimization). There is some ugliness in how I setup BufferAdaptor (probably nicer if it inherits from the buffer type, rather than providing static methods). So, here's the interesting properties of this new version:

        1) Significantly faster in the disabled case than even the new std::ios_base derived stream, at least on my system (gcc-3.3 and P4) I had about 12x faster performance. It's possible that gcc-3.4 would reduce this win as it's iostreams package is much more efficient. I am also not entirely certain why I'm seeing this much of a performance difference, but I'm guessing it's from having to construct a vtable as well as some overhead from ios_base.

        2) Same performance as the std::ios_base derived stream in the enabled case.

        3) Supports changing of log levels. The change isn't thread safe (although it could be made that way with a bit of effort), but the implementation in CVS has that problem as well, and it's probably bad to share a log stream across threads anyway. This approach does make changing log levels expensive, but I'd expect this to be a fairly infrequent operation. Going with the getStream() approach may prove to be better.

        4) Supports reuse of the log stream.

        5) No requirement to use iostreams at all. Through it's BufferAdaptor class, you can plug in any kind of buffer you want.

        Show
        Christopher Smith added a comment - Took another run at the alternative approach which attempts to address most of the issues with the original LoggerStream I submitted. A number of methods still need to be overridden (actually the same is true for stream.h in CVS), but it does have most of the key traits that have been discussed. It turns out that I was getting no measurable performance win by preventing the log level from changing (classic case of premature optimization). There is some ugliness in how I setup BufferAdaptor (probably nicer if it inherits from the buffer type, rather than providing static methods). So, here's the interesting properties of this new version: 1) Significantly faster in the disabled case than even the new std::ios_base derived stream, at least on my system (gcc-3.3 and P4) I had about 12x faster performance. It's possible that gcc-3.4 would reduce this win as it's iostreams package is much more efficient. I am also not entirely certain why I'm seeing this much of a performance difference, but I'm guessing it's from having to construct a vtable as well as some overhead from ios_base. 2) Same performance as the std::ios_base derived stream in the enabled case. 3) Supports changing of log levels. The change isn't thread safe (although it could be made that way with a bit of effort), but the implementation in CVS has that problem as well, and it's probably bad to share a log stream across threads anyway. This approach does make changing log levels expensive, but I'd expect this to be a fairly infrequent operation. Going with the getStream() approach may prove to be better. 4) Supports reuse of the log stream. 5) No requirement to use iostreams at all. Through it's BufferAdaptor class, you can plug in any kind of buffer you want.
        Hide
        Curt Arnold added a comment -

        Took another shot to improve performance when disabled. log4cxx::logstream now derived from std::ios_base which delays expensive construction until needed but allows standard format manipulators to work. Construction approximately 3 times slower that original proposal, however that should be offset by ability to use stream for multiple log requests.

        Added streamtestcase.cpp to test suite (but not to Makefile.am since I don't quite know what is going on there). testWidth was causing an exception and is currently disabled so I can look at it on a different platform.

        Show
        Curt Arnold added a comment - Took another shot to improve performance when disabled. log4cxx::logstream now derived from std::ios_base which delays expensive construction until needed but allows standard format manipulators to work. Construction approximately 3 times slower that original proposal, however that should be offset by ability to use stream for multiple log requests. Added streamtestcase.cpp to test suite (but not to Makefile.am since I don't quite know what is going on there). testWidth was causing an exception and is currently disabled so I can look at it on a different platform.
        Hide
        Curt Arnold added a comment -

        There has been additional benchmarking discussions on the mailing list. It appears that it will be unlikely to get the creation of even the simplest std::basic_stream creation time down sufficiently to make the no logging case fast enough. I think it is still good that the logging stream implementation really be a std::basic_stream, but will probably need to split the implementation into a proxy class and an implementation and only create the implementation when logging is effective.

        Show
        Curt Arnold added a comment - There has been additional benchmarking discussions on the mailing list. It appears that it will be unlikely to get the creation of even the simplest std::basic_stream creation time down sufficiently to make the no logging case fast enough. I think it is still good that the logging stream implementation really be a std::basic_stream, but will probably need to split the implementation into a proxy class and an implementation and only create the implementation when logging is effective.
        Curt Arnold made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Anonymous made changes -
        Attachment stream.patch2 [ 17857 ]
        Christopher Smith made changes -
        Attachment benchmark.tar.gz [ 17856 ]
        Hide
        Christopher Smith added a comment -

        Added a crude benchmark (benchmark.tar.gz) which compiles against the CVS version and includes my version of logging streams.

        Show
        Christopher Smith added a comment - Added a crude benchmark (benchmark.tar.gz) which compiles against the CVS version and includes my version of logging streams.
        Curt Arnold made changes -
        Attachment stream.patch [ 17854 ]
        Hide
        Curt Arnold added a comment -

        The attached file is an alternative approach to a stream based logging interface. The most significant difference is that this logstream is derived from std::basic_stream and can be passed to methods expecting a normal stream and all modifiers like std::width, std::scientific, etc can be applied to the stream.

        The most frequently called insertion operations should be short-circuited when the logger is disabled. Attempting to provide a template for the short-circuiting resulted in ambiguity errors, insertion operations that do not have a logstream counterpart will not be short-circuited when the logger is disabled.

        To complete a message, insert LOG4CXX_ENDMSG.

        See examples/stream.cpp for a trivial example of usage.

        Show
        Curt Arnold added a comment - The attached file is an alternative approach to a stream based logging interface. The most significant difference is that this logstream is derived from std::basic_stream and can be passed to methods expecting a normal stream and all modifiers like std::width, std::scientific, etc can be applied to the stream. The most frequently called insertion operations should be short-circuited when the logger is disabled. Attempting to provide a template for the short-circuiting resulted in ambiguity errors, insertion operations that do not have a logstream counterpart will not be short-circuited when the logger is disabled. To complete a message, insert LOG4CXX_ENDMSG. See examples/stream.cpp for a trivial example of usage.
        Curt Arnold made changes -
        Assignee Michaël CATANZARITI [ mcatan ] Curt Arnold [ carnold@apache.org ]
        Hide
        Christopher Smith added a comment -

        Sigh... and the typedef for endl should be removed as it is no longer necessary.....

        Show
        Christopher Smith added a comment - Sigh... and the typedef for endl should be removed as it is no longer necessary.....
        Hide
        Christopher Smith added a comment -

        In retrospect, the LOG_STREAM_EX is no longer necessary with the Endl change. LOG_STREAM & WLOG_STREAM are perhaps of value, but could probably be replaced with typedef's.

        Show
        Christopher Smith added a comment - In retrospect, the LOG_STREAM_EX is no longer necessary with the Endl change. LOG_STREAM & WLOG_STREAM are perhaps of value, but could probably be replaced with typedef's.
        Christopher Smith made changes -
        Field Original Value New Value
        Attachment LoggerStream.hpp [ 17844 ]
        Hide
        Christopher Smith added a comment -

        Sorry, this version is granted under the ASF license, and it uses it's own Endl class to wrap _FILE_ and _LINE_ macros. Much cleaner.

        Show
        Christopher Smith added a comment - Sorry, this version is granted under the ASF license, and it uses it's own Endl class to wrap _ FILE _ and _ LINE _ macros. Much cleaner.
        Christopher Smith created issue -

          People

          • Assignee:
            Curt Arnold
            Reporter:
            Christopher Smith
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development