Re: [boost] [Review] Boost.Logging: formal review

From: Caleb Epstein (caleb.epstein_at_[hidden])
Date: 2005-12-15 09:59:57

On 12/15/05, Martin Bonner <martin.bonner_at_[hidden]> wrote:
>
> ----Original Message----
> From: Richard Glanmark [mailto:Richard.Glanmark_at_[hidden]]
> Sent: 15 December 2005 08:33
> To: boost_at_[hidden]
> Subject: Re: [boost] [Review] Boost.Logging: formal review
>
> > Some thoughts and ideas about some of the topics that has been
> > discussed regarding the proposed Boost.Logging library.
> >
> >
> > 1. Enabled/disabled logger, logging macros and efficiency
> >
> > The logging macro BOOST_LOG should accept two arguments instead of
> > one. The logger and the log message. The syntax
> >
> > (1)   BOOST_LOG(some_log) << "my message " << foo() << bar();
> >
> > should be replaced with
> >
> > (2)   BOOST_LOG(some_log,  "my message " << foo() << bar());
> >
> > and the macro being defined something like this
> >
> > (3)   #define BOOST_LOG(logger, msg) \
> >               if(logger.isEnabled()) { /* code that logs msg */ ... }
>
> The current definition is (I believe)
>
> #define BOOST_LOG(logger) \
>         if(!logger.isEnabled()) {} else logger
>
> Which achieves the same ends.  In particular, foo() is not called unless
> logging occurs.

But there is some additional overhead with the implementation in the
proposed (and withdrawn) Logging library.  The conditional actually creates
a temporary object (enabled_logger) that contains a pointer to a
heap-allocated stringstream (in the case of the log being enabled).  This
stream then used as the target for the << operations.  When the temporary
goes out of scope (e.g. at the end of the logging statement) the
stringstream contents are flushed to the actual log object.  This temporary
object is created even in the case of the log being disabled, though the <<
formatting is never invoked and there is no heap allocation of a
stringstream.

I for one agree with Richard that any macro interface to logging should work
by surrounding the entire invocation in parens, not just the log object.  It
would remove the need for the enabled_logger class and the additional heap
allocation of the stream.

--
Caleb Epstein
caleb dot epstein at gmail dot com