Re: [boost] Boost.Logging: formal review

From: Gennadiy Rozental (gennadiy.rozental_at_[hidden])
Date: 2005-11-14 14:58:05

>> <sstream>
>
> Needed for buffering the messages.
> I've measured the effects of including or not including this, and at
> least for VC7.1, including <sstream> is compile-time wise virtually same
> as not including it.
>
> In fact, the only headers I include, in log.hpp, are <sstream> and 
> <string>.

Well , I guess it's compiler dependent. But still it would be nice to have.

>>>>IMO better model would be to apply modifiers to outputs instead.
>>>>
>>>
>>>Yes, I will consider this. IMO, is better to have a working model, which
>>>can be optimized later.
>>
>>
>> You couldn't change an API post review and introduce completely new
>> modifiers model. It's a different library that needs different review
>>
>
> I was only talking about changing (optimizing) the implementation.

You couldn't really optimize this without changing modifiers model.

>> How DLOG( DATA_FLOW) is more to type then BOOST_LOG( DEBUG )? Depends on
>
> First of all, you'd have to prefix it with BOOST_:

First of all I don't. I (as end user) could name my macros the way I prefer.

> So lets see:
> BOOST_LOG(DEBUG)
>
> BOOST_DLOG(DATA_FLOW)
> BOOST_LOG(DEBUG, RETURN_VALUE)
> BOOST_LOG(INFO, PROG_FLOW, "SocketLib")

So lets see:

L_(DATA_FLOW)
L_(DEBUG, RETURN_VALUE)
L_(INFO, PROG_FLOW, "SocketLib")

Note the last is used rarely this keyword usually shared.

>> Sorry typo. I want filtering on thread id. So that entry appear or not
>> appear in all outputs based on which is current thread id.
>
> Using the only_for_thread_id, combined with other appenders (for
> instance, appender_array), you can accomplish that:
>
> manipulate_logs("*").add_appender(
>   only_for_thread_id(1552,appender_array()
>     .add(write_to_cout)
>     .add(write_to_file("out.txt") )
>     .add(write_to_dbg_wnd) ) );

Whatever you do you couldn't mimic filter with use of appender. Your 
favorite example

LOG << very_long_function();

hits you immediately.  With above solution very_long_function() will be 
called. Filter is something that checked before log statement are invoked.

>> Ok Useless is wrong word. I meant to say that I would never use it since
>> single configure string is so much more clear and convenient.
>
> Yes, it is. But as you app grows bigger and bigger, I think that single
> string could get pretty complex.

Why? How my growing application affects my log configuration string?

> Basically, on top of the lib, you can build your Configuration Support,
> or what I envisioned -- using multiple lines. It's a matter of choice.

Basically logs always externally configured. Always from within some king of 
configuration file using some kind of configuration parameters/strings. Why 
would you make user to jump through hoops to  apply this configuration, when 
the most natural and convenient is to support this on library level? This is 
all beside the point that modifiers also unsafe since you need to keep track 
which one has which index and it will soon enough became maintenance 
nightmare.

>> I don't see how log name comes into play here (it could but only if end 
>> user
>> will choose to). I may have single log that doesn't care about the name 
>> at
>> all. Or I may have 2 different named config parameters:
>>
>> config.get( "system_log_config" )
>> config.get( "admin_log_config" )
>>
>> But this strings has nothing to do with log names either.
>>
>
> How do you set the log's settings, when you read them from the
> Configuration file?

sys_log.configure( config.get( "system_log_config" ) );
adm_log.configure( config.get( "admin_log_config" ) );

>> Let me repeat: Much more reliable solution is to use some kind of startup
>> log.
>> IOW we use dedicated log that only used during startup.
>>
>
> Well, I will allow for a startup log to be used in case the application
> ends, and the logs have not been initialized yet.

What is important is not what you allow, but what you prevent. Library needs 
to prevent misuse it advertise in "cashing" chapters of docs.

>> And what if I want to filter out specific subsystem? Do I need to
>> register/unregister outputs?  But this will still cause log statement to 
>> be
>> executed.
>
> If you want to filter out a specific subsystem, you will disable the
> logs corresponding to that subsystem.
>
> When a message is about to be written to the log, if the log is
> disabled, the whole message is ignored.

Ok. I guess you could mimic keywords with use of named logs. But now you 
have 2 different concepts (named log and log level) to do essentially the 
same job: filter log entry. And these wouldn't help you a bit if you need to 
filter based on any other criteria: category, time of day, thread id etc. 
Why not just use single powerful concept?

> // assume x is disabled
> BOOST_LOG(x) << even << if << this is costly << it will not be executed;
>
>>
>> Why would I do that instead of common filtering mechanism with multiple
>> filters?
>
> Efficiency, for starters.

How efficiency has anything to do with filters I advocate? Or rather How the 
usage of filters would decrease performance? I easily could prove that 
without custom filter support your performance will suffer (like above in 
thread id example)

>>>>>Please enlighten me: how can you implement a disabled log at macro 
>>>>>level?
>>>>
>>>>Numerous ways. Here is from top of my head:
>>>>
>>>>struct nil_stream {}
>>>>
>>>>template<typename T>
>>>>nil_stream const& operator<<( nil_stream const& ns, T const&) { return
>>>>ns; }
>>>>
>>>>#define LOG( .... ) if(true) {} else nil_stream() <<
>>>
>>>Amazing...
>>>
>>>LOG << some_lengthy_function();
>>>
>>>some_lengthy_function() will still be executed, even though logging is
>>>disabled...
>>
>> How did you case to such conclusion? Since when false branch gets 
>> executed?
>
> When the false branch gets executed, it will look like:

Since when false branch gets executed??

> The second statement was printed on the same line, because you did not
> add the append_enter to the appenders for LOG.

Yeah. That's another annoying thing: I would really prefer '\n'  to be added 
without extra efforts on my side.

> Finally, I'd really love to see a construct that will work in the way
> you expect.

>>>>>>If we do why all the appenders are so heavy?
>>>>>
>>>>>I don't understand what you mean.
>>>>
>>>>So costly to copy.
>>>
>>>How would you know?
>>
>> Just do sizeof(logger). Keep in mind that your design assumes that may 
>> copy
>> logger for *every* log entry.
>
> sizeof(logger) is 12

>From you code:

// note: each logger has its own copy of a write_to_file object. This needs 
to be shared,
//       so that different copies don't overwrite each-other

I assumed Each logger has a copy of each appender. Is it right?

> And BOOST_LOG uses a reference to a logger, so the logger is never copied.

What if I use scoped logger extensively?

Gennadiy