Christopher Smith
added a comment - 06/Oct/04 20:30 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.

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.

Curt Arnold
added a comment - 07/Oct/04 18:52 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.

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
added a comment - 22/Oct/04 05:04 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.

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.

Curt Arnold
added a comment - 25/Oct/04 05:03 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.

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.

Christopher Smith
added a comment - 25/Oct/04 19:14 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.

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'.

Curt Arnold
added a comment - 17/Jul/06 04:12 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