Re: [boost] [log] Boost.Log formal review

Subject: Re: [boost] [log] Boost.Log formal review
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2010-03-08 10:01:59

On 03/08/2010 05:12 PM, strasser_at_[hidden] wrote:
> Zitat von Andrey Semashev <andrey.semashev_at_[hidden]>:
>>> I think(!) it wouldn't require templating the whole library on the
>>> attribute set, but the filtering etc. could be instantiated on the
>>> attributes passed to the logger.
>>
>> Attribute sets are used in loggers and the core. I don't see how it
>> would be possible not to template them if some kind of type erasure is
>> not used (which brings us to the current solution). Further, the core
>> constructs a view of attribute values out of the attribute sets, which
>> is basically a map, like attribute sets. This view is then processed
>> by filters, formatters and sinks, so they will be templated, too.
>>
>>> I presume attributes are now stored in a std::map<std::string,Attribute>
>>> or similar. this is not an acceptable performance overhead for WAL.
>>
>> And what is your suggestion? Use fusion::map? Or
>> std::map<std::type_info, Attribute>?
>
> fusion::map, or not constructing a (dynamic) view of the attributes but
> passing it through.

It looks like you're missing the difference between attributes and 
attribute values.
Attributes do not represent any values, most of the time. For instance, 
a function that returns the current time stamp can be an attribute. 
Attribute values, on the other hand, do represent values (the concrete 
time readings in the example), and these values are processed by the 
most part of the library. So there is no way around constructing a view 
of attribute values. Also, dealing with a single view is much easier 
than with a number of distinct sets.

> I don't think that would mean parametrizing the core by the whole
> attribute set of every attribute ever used throughout the application,
> but it probably does mean templating the core e.g. by the filters.

That approach doesn't look any better to me, since it has the same 
drawbacks I outlined earlier.

> I understand that you can write your own sink and store the information
> in whatever way, but with most use cases of binary logging come several
> requirements, e.g. performance-wise it should be similar to:
>
> http_connection_requested record(host,port);
>
> stream.sputc( get_record_identity(record) );
> stream.sputn( &record, sizeof(record) );
>
> (assuming that the record is bitwise-serializable)

That assumption may be true in your particular case. But it's absolutely 
no go in a generic library, which Boost.Log is. In fact, even in more 
specialized cases I tend to reject such code.

> a lot of stuff that is now done at runtime, like creating a view of the
> attributes and filtering records based on that view, would have to be
> done at compile-time.

The implications of such compile-time-biased approach look unacceptable 
to me.

> so before I write a lengthy review from that angle, that would most
> likely result in a NO vote, let me know if this is beyond the scope of
> the library.
>
> this question is not only directed towards you but also towards other
> reviewers; if that use case is considered within the scope of a boost
> logging library.

I said this before the review. This case was not considered as the main 
one. So currently, no, the library is not aimed for it. However, I 
consider it as a possible way for further improvement of the library.