|
|
Boost : |
Subject: Re: [boost] [log] Comments
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2010-03-15 15:39:05
On 03/15/2010 09:10 PM, Vladimir Prus wrote:
> On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
>>
>> I can't provide logger features for every case. Sometimes users will
>> have to extend the library. Writing a class template with a couple
>> constructors and a trivial method doesn't look very complex to me.
>
> First, it seems to me that if two major projects use component/severity scheme,
> then presumably it's of general utility. Second, in the thread you have
> mentioned, you yourself provide example solution that are far from trivial.
> Maybe I'm missing something? In essence, I would like to have:
>
> magic_logger lg;
> DO_MAGIC(lg, "my_component", debug)<< "hi there";
>
> Could you show what will it take?
I've shown that, in the thread you mentioned. It simply appears that I
don't think that's complex and you do. No problem, if it deems that
necessary, I can add yet another feature to the library distribution.
>> It's not scary, it's verbose.
>> If you want to get down to the guts of the library, you're welcome. You
>> get the full control at the cost of verbosity. But most people won't
>> need it and will just use the shorthands, such as "extract" and "attr".
>> I see no problem with it.
>
> As already expressed by Tom, logging library is fairly basic component
> that should be useable by everybody. And lambda functions in current C++
> in not something everybody know so forcing the user to pick between
> using lambda, and much more verbose non-lambda style does not help.
> Another aspect mentioned during review is that you roll your own lambda
> implementation. Which means that folks that are familiar with boost::lambda
> might run into things that don't work, or work differently. Further,
> suppose one has access to a compiler that already implement native lambda.
> He probably would prefer that to your lambda emulation, but you impose
> a convenience tax on that.
>
> Why cannot you provide and equally convenient ordinary functions?
Can you suggest a better interface? Because I can't imagine how to make
it shorter, while keeping the other features intact.
>>> * Why can't I use the library without named parameters? Surely,
>>> there are low-tech ways to specify things.
>>
>> What are these?
>
> The simplest would be:
>
> file_log_parameters p;
> p.filter = ... ;
> p.formatter = ... ;
> init_log_to_file(p);
>
> This is straightward for any user, produces clear error messages,
> and is not much of a inconvenience.
The filter is the property of the log frontend, and formatter is the
property of the backend. Using named parameters allows to direct the
appropriate parameters to their particular receivers. Also, it's more
efficient than filling the structure.
>> Boost.Parameter is used in various places of the library, but its
>> primary goal is to provide abstraction between components that would
>> otherwise be tightly coupled. For instance, loggers consist of number of
>> features, each of which is independent from others. Named parameters
>> elegantly allow to interact with a particular feature without bothering
>> other features.
>
> Could you give more examples?
In the end of the section you can see how two logger features are
composed into a single logger. Both features require a parameter to open
a log record. Named parameters allow to pass these parameters to the
respective features, while neither of the features is aware of the other.
Another example. init_log_to_file may receive a number of named
parameters. Some of them are used by the sink frontend, some - by the
backend. In fact, you can actually pass all parameters to the frontend
constructor - it will pick those it needs and pass the rest to the
backend. That way the frontend is not aware of parameters that the
backend requires.
>>> * The library went too far with namespaces. Why are *six* nested namespaces
>>> necessary? This appears to be just asking for extra typing for no good
>>> reason.
>>
>> Use namespace aliases. Most of the time you will only need the sources
>> namespace. Other namespaces were introduced to keep code clarity and
>> avoid name clashes.
>
> Of course I can use namespace aliases to alleviate the problem. However,
> I don't see how namespaces are necessary. Are there actually things with
> the same names in those different namespaces?
Yes. attr is a formatter and a filter, for example. There are "format"
names in "formatters" and "keywords" namespaces. Perhaps other exist, I
didn't examine the code thoroughly.
>> Record identifiers proved to be very useful in communication,
>> surrounding the piece of log. It's much easier to write "see, there's
>> that error in line 2764" than to go for lengthy explanations of how to
>> find it in the file.
>
> What other libraries use this record id by default?
None, perhaps. That doesn't make this feature any less valuable.
>>> Also, it prints the thread "number",
>>> on my system a hex string. I am not sure thread id should be part of trivial
>>> logging.
>>
>> Why not? Why can't a multithreaded application use trivial logging?
>
> Because hex string, in a log, is completely useless. It is of any use if you
> are attached to a still-running application, and can actually figure what
> a thread id means in your program.
I believe, Robers has already answered this. I agree with him.
>>> Finally, I think by default, everybody things of logging as glorified
>>> std::cout, so trivial logging better print messages to console, not to a file.
>>
>> I disagree. First, you don't need the library to spam the console.
>
> Well, I do. Maybe because my console is usually not cmd.exe ;-)
You mean you can't use std::cout? What kind of console are we talking about?
>> Second, I consider spamming the console to be a bad idea in the first
>> place. In my practice there are cases when writing _anything_ on the
>> console is forbidden.
>
> It seems like a fairly specialized requirement. What is trivial logging
> supposed to be? If it is supposed to be printf logging on steriods, then it
> should behave like printf as much as possible, including printing to console.
Log, in the common sense, is perceived as a file. Console is not a good
place to write logs, since this is a place for user interaction, not for
a protocol of execution.
>>> * Why does the library have light_rw_mutex? I would really prefer if everything
>>> thread-related be located inside Boost.Thread, so that at least the code can
>>> be readily audited.
>>
>> light_rw_mutex can be more efficient than shared_mutex.
>
> Can, or is?
It is, in some configurations. In particular, on POSIX and NT6 (when
enabled). On POSIX someone on this list presented tests comparing
pthread_rwlock_t (which is used by light_rw_mutex) against shared_mutex,
former being faster. On NT6 light_rw_mutex is of size of one pointer
which is ~6 times smaller than shared_mutex. The performance is
comparable, according to my tests.
>> If it gets to Boost.Thread, I'll be first to use it.
>
> It's not like maintainer of Boost.Thread left on a space mission to outer
> space and is not coming back ;-) Could you just propose this for inclusion?
> We have too many bits hidden in detail namespace of various libraries.
This matter is kind of separate from the library review. I have no
problems proposing this component for inclusion Boost.Thread, after the
review.
>>> * It appears that the default behaviour of fmt::attr is to throw is the attribute
>>> with such name is not found. This seems like a bad choice to me. A logging
>>> library should never fail -- it is the last resort at diagnosing a problem in the field,
>>> and if it throws by default, it's useless.
>>
>> That was my thought initially. Then this thread had happened:
>>
>> http://article.gmane.org/gmane.comp.lib.boost.devel/185956
>>
>> You can suppress exceptions with exception handlers on different levels
>> of the library.
>
> I saw that -- my point is that *default* behaviour should be to go on no
> matter what is wrong.
I think, this is probably the point where different opinions collide
with none being right or wrong. In a classical case of logging as an
application execution protocol writing, you're absolutely right. In
other cases, when logging plays more central role in the application,
the other way around is more desirable.
>>> I'd recommend working with maintainer of boost/throw_exception.hpp to add noreturn attributes on
>>> gcc, and silence the warning in other ways on other compilers.
>>
>> AFAIK, it already has that attribute. Not sure why it doesn't work for
>> you. What compiler do you use?
>
> gcc 4.4 with relatively recent svn trunk.
Strange, my GCC 4.4.1 and release branch I get no warnings. Tomorrow
I'll try again with trunk.
>>> * Building the library with threading=single results in unresolved references to pthread functions.
>>> I belive that library should not use any sync mechanisms in this case.
>>
>> Yes, I've been told that. Presumably, the references are from
>> Boost.ASIO, which is used by the syslog backend. Not sure what would be
>> the best solution - to drop single-threaded builds or to exclude syslog
>> in ST builds.
>
> Well, if this the case, Boost.ASIO should be fixed not to contain its own
> thread-related mechanisms -- which only reinforces my position about
> rw_mutex.
No, it doesn't reinforce your point. ASIO may well not be suited to be
used in a ST environment.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk