Hello, I have a few comments and questions about the Boost.Log library currently being reviewed. This email is not actually a review as such. It will help if I explain where I'm coming from. I am most familiar with two logging mechanism. One is from KDE libraries. There, you have severities and component ids -- which are just integers. So, source code might have kDebug(9012) << "doing something"; There's GUI to configure which components have logging enabled, and there's a way to omit explict component id. Another logging mechanism is from Eclipse. There, you specify severity and component id -- this time a string. In KDE, output goes to a console. In Eclipse -- to a log file. I would those mechanism to be sufficient for all my needs. Looking at the proposed libraries, I see two major issues: * There's no default setup that is comparable with the two mechanisms above. BOOST_LOG_TRIVIAL has only severity and it appears adding component information is complicated. In a sense, it looks like the library provides building blocks, but not a solution I can immediately use. * I think the library elegantly uses various facilities like Boost.Parameter and lambda expressions to provide an easy interface. Unfortunately, it seems like it's failing to provide any other interface for folks who might be unhappy with lambdas and named parameters. There's a more detailed explanation of those major issues: * It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above. The proposed solutions at: http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793 sound relatively complex. * The example of specifying filter without lambda functions given in http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793 is frankly scary. I believe that in most languages and environments I worked with, lambda functions are just convenient alternative to defining the same function explicitly. Why is in Boost.Log, lambda expressions are blessed with extra convenience? * Why can't I use the library without named parameters? Surely, there are low-tech ways to specify things. * Why does channel_logger has a *single* parameter, and still requires that you pass it via named parameter. That is: src::channel_logger<> cl("foobar"); produces incomprehensible error message. Other concerns: * 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. * Why is trivial logging printing the sequental number of each log message? I don't think this helps anything. Also, it prints the thread "number", on my system a hex string. I am not sure thread id should be part of trivial logging. 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. * 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. * 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. * I have used the attached script to measure code size impact. The script basically creates a test file with a single function calling BOOST_LOG_TRIVIAL and measures the size of that function depending on the number of calls. I've got 139 bytes per call for 'g++ -Os' and 174 per call for 'g++ -O3', which seems nice. This is function code only. When counting file size I get 324 and 514 respectively. For reference, kDebug gives 136 and 328 of file_size_bytes per call. I guess these results are OK. * I see lots of warnings in slim_string.cpp, e.g: ../../../../libs/log/src/slim_string.cpp:237: warning: control reaches end of non-void function 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. * 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. Thanks, Volodya
On Mon, 2010-03-15 at 13:45 +0300, Vladimir Prus wrote:
Looking at the proposed libraries, I see two major issues:
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above.
Aside from the examples given, syslog still exists (complete with printf interface - who needs another logging lib anyway :) and has the same component (the tag) and severity (severity) concept, plus a time stamp and a "host" identifier. Common usage also includes a process ID. It would be instructive to see an example usage with the necessary attributes to construct a conventional syslog message.
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything. Also, it prints the thread "number", on my system a hex string. I am not sure thread id should be part of trivial logging.
I agree. But that said, I'm not sure what the "right" base attribute set should be - I'm not sure there should be one - it would be better to make composition from a set of commonly used attributes easier.
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.
And everybody thinks the console is a file (platforms where it isn't are wrong :-) Trivial logging shouldn't be so trivial it can't be used in a program that can output to either.
On Monday 15 March 2010 15:30:20 Darryl Green wrote:
On Mon, 2010-03-15 at 13:45 +0300, Vladimir Prus wrote:
Looking at the proposed libraries, I see two major issues:
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above.
Aside from the examples given, syslog still exists (complete with printf interface - who needs another logging lib anyway :) and has the same component (the tag) and severity (severity) concept, plus a time stamp and a "host" identifier. Common usage also includes a process ID. It would be instructive to see an example usage with the necessary attributes to construct a conventional syslog message.
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything. Also, it prints the thread "number", on my system a hex string. I am not sure thread id should be part of trivial logging.
I agree. But that said, I'm not sure what the "right" base attribute set should be - I'm not sure there should be one - it would be better to make composition from a set of commonly used attributes easier.
Maybe so -- right now it seems like you either get trivial logging with fixed set of attributes and fixed idea where output goes, or you need to do much more configuration. Thanks, Volodya
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
Looking at the proposed libraries, I see two major issues:
* There's no default setup that is comparable with the two mechanisms above. BOOST_LOG_TRIVIAL has only severity and it appears adding component information is complicated. In a sense, it looks like the library provides building blocks, but not a solution I can immediately use.
Right. There is no silver bullet for everyone. I learned that watching the first review of the library by John Torjo, participating his second review and collecting the requirements and feedback on my library. This review confirms it even further. Therefore, it's useful to provide a flexible library with a few most obvious and frequent cases available at hand and a great potential of extension and configurability. Regarding the trivial logging. As the naming implies, it is trivial, and is intended to be a drop-in replacement for std::cerr, but with boosters. (pardon my pun). :) But I understand that one or two options could be of use, such as the file name.
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above. The proposed solutions at:
http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793
sound relatively complex.
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.
* The example of specifying filter without lambda functions given in http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793 is frankly scary. I believe that in most languages and environments I worked with, lambda functions are just convenient alternative to defining the same function explicitly. Why is in Boost.Log, lambda expressions are blessed with extra convenience?
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.
* Why can't I use the library without named parameters? Surely, there are low-tech ways to specify things.
What are these? 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.
* Why does channel_logger has a *single* parameter, and still requires that you pass it via named parameter. That is:
src::channel_logger<> cl("foobar");
produces incomprehensible error message.
Yes, that should be fixed.
* 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.
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything.
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.
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?
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. 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.
* 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. If it gets to Boost.Thread, I'll be first to use it.
* 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'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?
* 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.
Andrey Semashev wrote:
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
Looking at the proposed libraries, I see two major issues:
* There's no default setup that is comparable with the two mechanisms above. BOOST_LOG_TRIVIAL has only severity and it appears adding component information is complicated. In a sense, it looks like the library provides building blocks, but not a solution I can immediately use.
Right. There is no silver bullet for everyone. I learned that
[snip]
Therefore, it's useful to provide a flexible library with a few most obvious and frequent cases available at hand and a great potential of extension and configurability.
Certainly a good approach. The issue is determining the common use cases you should support.
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above. The proposed solutions at:
http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793
sound relatively complex.
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.
Vladimir's point is that severity/component is a very common use case that you should support out of the box. That's how our logging works, so Vladimir and I would have to recreate the same wheel if you don't provide direct support. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/15/2010 08:58 PM, Stewart, Robert wrote:
Therefore, it's useful to provide a flexible library with a few most obvious and frequent cases available at hand and a great potential of extension and configurability.
Certainly a good approach. The issue is determining the common use cases you should support.
Right. But I guess, there will always be someone not happy with the choice.
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above. The proposed solutions at:
http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793
sound relatively complex.
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.
Vladimir's point is that severity/component is a very common use case that you should support out of the box. That's how our logging works, so Vladimir and I would have to recreate the same wheel if you don't provide direct support.
The library supports severity out of the box. Two different severities are unexpected and thus need a little more effort to support.
On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
Looking at the proposed libraries, I see two major issues:
* There's no default setup that is comparable with the two mechanisms above. BOOST_LOG_TRIVIAL has only severity and it appears adding component information is complicated. In a sense, it looks like the library provides building blocks, but not a solution I can immediately use.
Right. There is no silver bullet for everyone. I learned that watching the first review of the library by John Torjo, participating his second review and collecting the requirements and feedback on my library. This review confirms it even further.
Therefore, it's useful to provide a flexible library with a few most obvious and frequent cases available at hand and a great potential of extension and configurability.
Regarding the trivial logging. As the naming implies, it is trivial, and is intended to be a drop-in replacement for std::cerr, but with boosters. (pardon my pun). :) But I understand that one or two options could be of use, such as the file name.
* It appears that out of the box, the proposed library does not address this component/severity style of logging that I've explained above. The proposed solutions at:
http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793
sound relatively complex.
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?
* The example of specifying filter without lambda functions given in http://permalink.gmane.org/gmane.comp.lib.boost.devel/200793 is frankly scary. I believe that in most languages and environments I worked with, lambda functions are just convenient alternative to defining the same function explicitly. Why is in Boost.Log, lambda expressions are blessed with extra convenience?
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?
* 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.
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?
* Why does channel_logger has a *single* parameter, and still requires that you pass it via named parameter. That is:
src::channel_logger<> cl("foobar");
produces incomprehensible error message.
Yes, that should be fixed.
* 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?
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything.
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?
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.
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 ;-)
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.
* 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?
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.
* 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'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.
* 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. - Volodya
Vladimir Prus wrote:
On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
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 disagree. The distinct thread IDs allow one to distill the output contributed by different threads. One can filter the output to see only that for a particular thread, for example. The output logged will indicate the thread, if the threads are created for different tasks. If the threads are simply divvying up parallelizable work, then the separate IDs are still useful to understand how much work was done by each thread and, perhaps, how the work was divided. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Monday 15 March 2010 21:36:17 Stewart, Robert wrote:
Vladimir Prus wrote:
On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
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 disagree. The distinct thread IDs allow one to distill the output contributed by different threads. One can filter the output to see only that for a particular thread, for example. The output logged will indicate the thread, if the threads are created for different tasks. If the threads are simply divvying up parallelizable work, then the separate IDs are still useful to understand how much work was done by each thread and, perhaps, how the work was divided.
Ok, then thread ids have some uses. Still, it's probably more like advanced usage than trivial logging. Further, note that on Linux, Boost is always built with threading=multi (and Boost.Log does not even build with threading=single), so you'll get thread id even for applications that do not, in fact, create any threads. It seems that some configurability for trivial logging is needed -- which is recurring conclusion. BTW, it's not even clear to me how such configurability can be done. Ideally, I want to do something like: get_trivial_logging_sink()->filter_out_attribute("ThreadID"); as opposed to get_trivial_logging_sink()->locked_backend()->set_formatter( formatters::stream << formatters::attr< unsigned int >(traits_t::line_id_attr_name()) << " [" << formatters::date_time< posix_time::ptime >(traits_t::time_stamp_attr_name()) << "] [" << formatters::attr< severity_level >(sources::aux::severity_attribute_name< char >::get()) << "] " << formatters::message< char >(); as the latter would be essentially a copy-paste. - Volodya
Vladimir Prus wrote:
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?
We are using component/severity, too. I decided to interpret the channel as component. Some sample code is attached to my review: http://lists.boost.org/Archives/boost/2010/03/162902.php After some preparation in the init function, developers can do the following: src::severity_channel_logger<severity_level> lg1(keywords::channel = "FirstComponent"); BOOST_LOG_SEV(lg1, trace) << "A trace severity message"; BOOST_LOG_SEV(lg1, debug) << "A debug severity message"; I plan to hide most of the first line in a typedef and a convenience function.
I saw that -- my point is that *default* behaviour should be to go on no matter what is wrong.
I second that (and, if that were is possible in English, I'd third and forth that). Logging must not fail until the machine crumbles to dust. Especially: No message must be lost. I know that is not really possible, but providing a very high level of certainty that messages do not get lost is of extreme importance.
On Monday 15 March 2010 22:07:57 Roland Bock wrote:
Vladimir Prus wrote:
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?
We are using component/severity, too. I decided to interpret the channel as component. Some sample code is attached to my review:
http://lists.boost.org/Archives/boost/2010/03/162902.php
After some preparation in the init function, developers can do the following:
src::severity_channel_logger<severity_level> lg1(keywords::channel = "FirstComponent");
BOOST_LOG_SEV(lg1, trace) << "A trace severity message"; BOOST_LOG_SEV(lg1, debug) << "A debug severity message";
I plan to hide most of the first line in a typedef and a convenience function.
What I don't like about this solution is that it requires that you define a new object for every componen, which can be inconvenient, including for header-only libraries. What I would like to have is something like // myproject.hpp #ifndef COMPONENT #define COMPONENT "unspecified" #endif #define DEBUG BOOST_LOG_XXX(mygloballogger, COMPONENT, debug) - Volodya
AMDG Vladimir Prus wrote:
On Monday 15 March 2010 22:07:57 Roland Bock wrote:
After some preparation in the init function, developers can do the following:
src::severity_channel_logger<severity_level> lg1(keywords::channel = "FirstComponent");
BOOST_LOG_SEV(lg1, trace) << "A trace severity message"; BOOST_LOG_SEV(lg1, debug) << "A debug severity message";
I plan to hide most of the first line in a typedef and a convenience function.
What I don't like about this solution is that it requires that you define a new object for every componen, which can be inconvenient, including for header-only libraries. What I would like to have is something like
// myproject.hpp #ifndef COMPONENT #define COMPONENT "unspecified" #endif
#define DEBUG BOOST_LOG_XXX(mygloballogger, COMPONENT, debug)
I guess that what I'd like is a way to specify arbitrary attributes when logging, so BOOST_LOG_XXX would be defined like #define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component)) In Christ, Steven Watanabe
On 03/15/2010 10:50 PM, Steven Watanabe wrote:
I guess that what I'd like is a way to specify arbitrary attributes when logging, so BOOST_LOG_XXX would be defined like
#define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component))
That should be quite possible, if there was some way to select the attributes from the rest of the named parameters for the logger. Although that would mean the attributes are added/removed from the logger at each record.
AMDG Andrey Semashev wrote:
On 03/15/2010 10:50 PM, Steven Watanabe wrote:
I guess that what I'd like is a way to specify arbitrary attributes when logging, so BOOST_LOG_XXX would be defined like
#define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component))
That should be quite possible, if there was some way to select the attributes from the rest of the named parameters for the logger. Although that would mean the attributes are added/removed from the logger at each record.
You might be able to create special keywords that are associated with attribute names. In Christ, Steven Watanabe
On 03/15/2010 11:36 PM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
On 03/15/2010 10:50 PM, Steven Watanabe wrote:
I guess that what I'd like is a way to specify arbitrary attributes when logging, so BOOST_LOG_XXX would be defined like
#define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component))
That should be quite possible, if there was some way to select the attributes from the rest of the named parameters for the logger. Although that would mean the attributes are added/removed from the logger at each record.
You might be able to create special keywords that are associated with attribute names.
I think, it should be a single keyword that accumulates all attributes to be added. It's just how to store them in the named argument. In a fusion::vector, perhaps?..
AMDG Andrey Semashev wrote:
On 03/15/2010 11:36 PM, Steven Watanabe wrote:
Andrey Semashev wrote:
That should be quite possible, if there was some way to select the attributes from the rest of the named parameters for the logger. Although that would mean the attributes are added/removed from the logger at each record.
I think, it should be a single keyword that accumulates all attributes to be added. It's just how to store them in the named argument. In a fusion::vector, perhaps?..
fusion::vector could work. Something like fusion::make_vector(std::make_pair("Severity", severity), std::make_pair("Channel", component)) In Christ, Steven Watanabe
Steven Watanabe wrote:
Andrey Semashev wrote:
On 03/15/2010 10:50 PM, Steven Watanabe wrote:
I guess that what I'd like is a way to specify arbitrary
attributes when logging, so BOOST_LOG_XXX would be defined like
#define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component))
That should be quite possible, if there was some way to select the attributes from the rest of the named parameters for the logger. Although that would mean the attributes are added/removed from the logger at each record.
You might be able to create special keywords that are associated with attribute names.
That reminds me. Seeing the strings in the library's interface such as proposed above make me nervous. In our environment, logging has to be very low latency in the calling thread. Checking to see whether a severity/component is enabled has to be very fast. Strings, such as those appearing in the suggestion above, imply higher latency than ideal. It would be really nice to have a string-to-ID mapping whereby the attribute names can be turned into identifiers that can be used thereafter for constant time lookup. In the example above, an extern std::size_t, say, would hold the ID for "Severity" and another for "Channel." Then, the macro would reference those std::size_t's rather than the strings, making all uses of that macro much faster. Adding support for that approach would, most likely, increase the API by adding overloads to various functions because the simpler, string-based approach may be adequate for many applications. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/16/2010 02:19 PM, Stewart, Robert wrote:
That reminds me. Seeing the strings in the library's interface such as proposed above make me nervous. In our environment, logging has to be very low latency in the calling thread. Checking to see whether a severity/component is enabled has to be very fast. Strings, such as those appearing in the suggestion above, imply higher latency than ideal.
It would be really nice to have a string-to-ID mapping whereby the attribute names can be turned into identifiers that can be used thereafter for constant time lookup. In the example above, an extern std::size_t, say, would hold the ID for "Severity" and another for "Channel." Then, the macro would reference those std::size_t's rather than the strings, making all uses of that macro much faster.
Adding support for that approach would, most likely, increase the API by adding overloads to various functions because the simpler, string-based approach may be adequate for many applications.
That looks like a possible optimization, although I'm not sure the final outcome will be positive. That approach should be well benchmarked before inclusion.
On Monday 15 March 2010 22:50:59 Steven Watanabe wrote:
// myproject.hpp #ifndef COMPONENT #define COMPONENT "unspecified" #endif
#define DEBUG BOOST_LOG_XXX(mygloballogger, COMPONENT, debug)
I guess that what I'd like is a way to specify arbitrary attributes when logging, so BOOST_LOG_XXX would be defined like
#define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component))
Yes, I think this would be nice. There's another thing about component/severity logging that I have apparently forgot about -- filtering. It is surely necessary to change filtered components dynamically -- either while the application is running, or between runs, therefore lambda expression that compares to a fixed string will not be enough -- we need a lookup in a map. It's not obvious to me how to form a lambda expression to do a lookup in a map using the library, e.g. something like: enabled_components.count(attr<string>("Component")) So I wonder: 1. What is the actual syntax to do this inside lambda expression. 2. Can we implement a more convenient out-of-box mechanism. For example: logging::core()->enabled_components().add("c1") where this automatically affects the "Component" attribute. - Volodya
On 15 March 2010 14:31, Vladimir Prus <ghost@cs.msu.su> wrote:
On Monday 15 March 2010 22:50:59 Steven Watanabe wrote:
// myproject.hpp #ifndef COMPONENT #define COMPONENT "unspecified" #endif
#define DEBUG BOOST_LOG_XXX(mygloballogger, COMPONENT, debug)
I guess that what I'd like is a way to specify arbitrary attributes when logging, so BOOST_LOG_XXX would be defined like
#define BOOST_LOG_XXX(lg, component, severity) BOOST_LOG_WITH_ATTRS(lg, ("Severity", severity)("Channel", component))
Yes, I think this would be nice.
There's another thing about component/severity logging that I have apparently forgot about -- filtering. It is surely necessary to change filtered components dynamically -- either while the application is running, or between runs, therefore lambda expression that compares to a fixed string will not be enough -- we need a lookup in a map. It's not obvious to me how to form a lambda expression to do a lookup in a map using the library, e.g. something like:
enabled_components.count(attr<string>("Component"))
This was part of my questions too ( http://comments.gmane.org/gmane.comp.lib.boost.devel/200785). I found it quite difficult to setup the system so could enable/disable logs to std::cout dynamically, based on component (although I called it System) and severity. That's why the lambda style filters didn't help me, since they were designed for statically defining filters, while I need to be able to modify them at runtime. / christian
AMDG Vladimir Prus wrote:
There's another thing about component/severity logging that I have apparently forgot about -- filtering. It is surely necessary to change filtered components dynamically -- either while the application is running, or between runs, therefore lambda expression that compares to a fixed string will not be enough -- we need a lookup in a map. It's not obvious to me how to form a lambda expression to do a lookup in a map using the library, e.g. something like:
enabled_components.count(attr<string>("Component"))
So I wonder:
1. What is the actual syntax to do this inside lambda expression.
There is no simple syntax for it. (The lambda functionality is rather crippled compared to a real lambda library.) You could probably do it with boost::bind and the satisfies function. Also, this by itself would be dangerous, since filters need to be thread safe.
2. Can we implement a more convenient out-of-box mechanism. For example:
logging::core()->enabled_components().add("c1")
where this automatically affects the "Component" attribute.
In Christ, Steven Watanabe
On 03/15/2010 11:47 PM, Steven Watanabe wrote:
AMDG
Vladimir Prus wrote:
There's another thing about component/severity logging that I have apparently forgot about -- filtering. It is surely necessary to change filtered components dynamically -- either while the application is running, or between runs, therefore lambda expression that compares to a fixed string will not be enough -- we need a lookup in a map. It's not obvious to me how to form a lambda expression to do a lookup in a map using the library, e.g. something like:
enabled_components.count(attr<string>("Component"))
So I wonder:
1. What is the actual syntax to do this inside lambda expression.
There is no simple syntax for it. (The lambda functionality is rather crippled compared to a real lambda library.) You could probably do it with boost::bind and the satisfies function. Also, this by itself would be dangerous, since filters need to be thread safe.
In my view, the filter should be replaced on update. You can have the map of components or whatever, and the filter pointing to them. On update you create a new map and a new filter, and set it to the core/sinks. No additional locking required, no special syntax, everything's safe and clean. Same goes for formatters.
On Tuesday 16 March 2010 01:34:52 Andrey Semashev wrote:
On 03/15/2010 11:47 PM, Steven Watanabe wrote:
AMDG
Vladimir Prus wrote:
There's another thing about component/severity logging that I have apparently forgot about -- filtering. It is surely necessary to change filtered components dynamically -- either while the application is running, or between runs, therefore lambda expression that compares to a fixed string will not be enough -- we need a lookup in a map. It's not obvious to me how to form a lambda expression to do a lookup in a map using the library, e.g. something like:
enabled_components.count(attr<string>("Component"))
So I wonder:
1. What is the actual syntax to do this inside lambda expression.
There is no simple syntax for it. (The lambda functionality is rather crippled compared to a real lambda library.) You could probably do it with boost::bind and the satisfies function. Also, this by itself would be dangerous, since filters need to be thread safe.
In my view, the filter should be replaced on update. You can have the map of components or whatever, and the filter pointing to them. On update you create a new map and a new filter, and set it to the core/sinks. No additional locking required, no special syntax, everything's safe and clean. Same goes for formatters.
What about performance? If I have 100 components, copying map on each update seems not too good. Locking is probably better. - Volodya
On 03/16/2010 08:50 AM, Vladimir Prus wrote:
On Tuesday 16 March 2010 01:34:52 Andrey Semashev wrote:
In my view, the filter should be replaced on update. You can have the map of components or whatever, and the filter pointing to them. On update you create a new map and a new filter, and set it to the core/sinks. No additional locking required, no special syntax, everything's safe and clean. Same goes for formatters.
What about performance? If I have 100 components, copying map on each update seems not too good. Locking is probably better.
It's you use case, and you probably know better. But I seriously doubt that locking the filter on each log record (which, I assume, is rather often) can be better than filling a 100-element container on occasional config updates.
On Tuesday 16 March 2010 18:00:10 Andrey Semashev wrote:
On 03/16/2010 08:50 AM, Vladimir Prus wrote:
On Tuesday 16 March 2010 01:34:52 Andrey Semashev wrote:
In my view, the filter should be replaced on update. You can have the map of components or whatever, and the filter pointing to them. On update you create a new map and a new filter, and set it to the core/sinks. No additional locking required, no special syntax, everything's safe and clean. Same goes for formatters.
What about performance? If I have 100 components, copying map on each update seems not too good. Locking is probably better.
It's you use case, and you probably know better. But I seriously doubt that locking the filter on each log record (which, I assume, is rather often) can be better than filling a 100-element container on occasional config updates.
In either case, this is more like implementation choice than interface one, so we don't need to come with any decision right now. Thanks, Volodya
----- Original Message ----- From: "Roland Bock" <rbock@eudoxos.de> To: <boost@lists.boost.org> Sent: Monday, March 15, 2010 8:07 PM Subject: Re: [boost] [log] Comments
I saw that -- my point is that *default* behaviour should be to go on no matter what is wrong.
I second that (and, if that were is possible in English, I'd third and forth that). Logging must not fail until the machine crumbles to dust.
Especially: No message must be lost.
I know that is not really possible, but providing a very high level of certainty that messages do not get lost is of extreme importance.
IMO, if a message must be lost the library must be able to log this fact, otherwise the user can not be confident with the log. Of course, if recod_id are used, there is no need to add an alert log. Best, Vicente
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?
http://boost-log.sourceforge.net/libs/log/doc/html/log/extension/sources.htm... 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.
On Monday 15 March 2010 22:39:05 Andrey Semashev wrote:
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.
OK.
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.
Your example when using a freestanding function was: shared_ptr< logging::attribute > attr = attrs["System"]; if (attr) { optional< System > sys = attr->get< System >(); if (!sys) throw runtime_error("The System attribute has invalid type"); if (sys.get() != m_sys) return false; } else throw runtime_error("The System attribute not found"); The corresponding code using lambda would be, presumably: flt::attr< ... >("System") == m_sys Would it be possible to make the following work in the first case: if (attrs["System"] != m_sys) return false; ? Or, if changing type/behaviour of operator[] is undesirable, what about if (attrs->get_nothrow("System") != m_sys) return false; ?
* 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.
Is this some magic provided by Boost.Parameters, or you still need to manuall route the properties to relevant components?
Also, it's more efficient than filling the structure.
This is init code, so performance is likely not very important.
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?
http://boost-log.sourceforge.net/libs/log/doc/html/log/extension/sources.htm...
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.
I agree that this composition of features won't work with 'big-struct-with-all-parameters' approach suggested above. Then, what about this: my_composite_logger lg; lg.set_feature_1(...); lg.set_feature_2(...); It seems to me that such interface will not require that features know about each 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.
Just to clarify -- is there a check that all named parameters are actually used. I admit I don't know much about Boost parameter, so might be doing something wrong, but the attached diff to basic_usage example still compiles. Is this a bug or a feature?
* 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.
Oh, indeed! But.. why? Cannot one object serve both roles?
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?
Oh, of course I can use std::cout to print to console. However, I want to be able to enable and disable log message from specific components, and std::cout cannot do that.
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.
I think that many non-deamon Linux program that permits enabling logging logs things to stdout. In fact, I don't remember any non-daemon program that would put logs to a file when invoked by user.
* 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'm not sure I understand those other cases. Let's wait what others will say.
* 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.
It might not be suited, but by using custom threading code as opposed to Boost.Thread it manifests this unsuitability in a inconvenient way. - Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Thanks,
On 03/15/2010 11:22 PM, Vladimir Prus wrote:
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.
Your example when using a freestanding function was:
shared_ptr< logging::attribute> attr = attrs["System"]; if (attr) { optional< System> sys = attr->get< System>(); if (!sys) throw runtime_error("The System attribute has invalid type");
if (sys.get() != m_sys) return false; } else throw runtime_error("The System attribute not found");
The corresponding code using lambda would be, presumably:
flt::attr< ...>("System") == m_sys
Would it be possible to make the following work in the first case:
if (attrs["System"] != m_sys) return false;
It doesn't specify the type of the value.
? Or, if changing type/behaviour of operator[] is undesirable, what about
if (attrs->get_nothrow("System") != m_sys) return false;
That should be: if (attrs.get_nothrow< System >("System") != m_sys) return false; Yes, it could be done. But as stated elsewhere in the discussion, this would require System to be copyable (which might be fine in this case). And it doesn't really differ from: bool result = false; extract< System >("System", attrs, var(result) = _1 != m_sys); return result; but with extract it doesn't have the copyability restriction.
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.
Is this some magic provided by Boost.Parameters, or you still need to manuall route the properties to relevant components?
No, I just pass on the whole pack of arguments.
Also, it's more efficient than filling the structure.
This is init code, so performance is likely not very important.
open_record is called for every log record.
I agree that this composition of features won't work with 'big-struct-with-all-parameters' approach suggested above. Then, what about this:
my_composite_logger lg; lg.set_feature_1(...); lg.set_feature_2(...);
It seems to me that such interface will not require that features know about each other.
It will not, unless they clash member functions, but we can ignore that for now. But it doesn't help with constructors, and makes writing macros around this interface a bit harder.
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.
Just to clarify -- is there a check that all named parameters are actually used. I admit I don't know much about Boost parameter, so might be doing something wrong, but the attached diff to basic_usage example still compiles. Is this a bug or a feature?
Unused parameters are ignored, no compile-time or run-time errors. I can't call that a feature, but it certainly is not a bug. Just an expected behavior by design.
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.
Oh, indeed! But.. why? Cannot one object serve both roles?
One is filter and the other is formatter. They have different interfaces (with regard to the library and the user) and serve different purposes (one - to compose filters, the other - formatters). Also, thread safety requirements are different. So I don't think that mixing them is a good idea. And still, having the same name makes sense, because syntactically, both in filters and formatters these placeholders represent an attribute value.
AMDG Andrey Semashev wrote:
Yes. attr is a formatter and a filter, for example.
Oh, indeed! But.. why? Cannot one object serve both roles?
One is filter and the other is formatter. They have different interfaces (with regard to the library and the user)
Are the interfaces incompatible?
and serve different purposes (one - to compose filters, the other - formatters).
Why does this matter? spirit uses the same placeholders in both qi and karma, for instance.
Also, thread safety requirements are different.
Why? In any sane implementation that I can think of, both attr's should store the same immutable data. In Christ, Steven Watanabe
On 03/16/2010 01:49 AM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
Yes. attr is a formatter and a filter, for example.
Oh, indeed! But.. why? Cannot one object serve both roles?
One is filter and the other is formatter. They have different interfaces (with regard to the library and the user)
Are the interfaces incompatible?
Yes. Filters and formatters do different things, how can they share the interface?
and serve different purposes (one - to compose filters, the other - formatters).
Why does this matter? spirit uses the same placeholders in both qi and karma, for instance.
I'm not a specialist in Qi and Karma internals and cannot judge on how that design decision is applicable to Boost.Log. But from my viewpoint, mixing filters and formatters is absolutely not a good idea.
Also, thread safety requirements are different.
Why? In any sane implementation that I can think of, both attr's should store the same immutable data.
The formatter attr can contain a boost::format member, which makes it thread-unsafe. Thread unsafe design is common for most formatters.
AMDG Andrey Semashev wrote:
On 03/16/2010 01:49 AM, Steven Watanabe wrote:
Andrey Semashev wrote:
Yes. attr is a formatter and a filter, for example.
Oh, indeed! But.. why? Cannot one object serve both roles?
One is filter and the other is formatter. They have different interfaces (with regard to the library and the user)
Are the interfaces incompatible?
Yes. Filters and formatters do different things, how can they share the interface?
I was really asking whether the interfaces conflicted with each other, not whether they were different. For that matter, I see no particular reason why attr needs to have a different interface when it is used in a formatter than when it is used as a filter. In either case, it should be a fairly light weight object that stores enough information to extract the attribute value from the attribute set. How this value is used can be determined from the context.
and serve different purposes (one - to compose filters, the other - formatters).
Why does this matter? spirit uses the same placeholders in both qi and karma, for instance.
I'm not a specialist in Qi and Karma internals and cannot judge on how that design decision is applicable to Boost.Log. But from my viewpoint, mixing filters and formatters is absolutely not a good idea.
The point is that attr has the same meaning in both a filter and a formatter.
Also, thread safety requirements are different.
Why? In any sane implementation that I can think of, both attr's should store the same immutable data.
The formatter attr can contain a boost::format member, which makes it thread-unsafe. Thread unsafe design is common for most formatters.
Is it necessary to have a boost::format object in the result of attr? In Christ, Steven Watanabe
On 03/16/2010 06:44 PM, Steven Watanabe wrote:
One is filter and the other is formatter. They have different interfaces (with regard to the library and the user)
Are the interfaces incompatible?
Yes. Filters and formatters do different things, how can they share the interface?
I was really asking whether the interfaces conflicted with each other, not whether they were different. For that matter, I see no particular reason why attr needs to have a different interface when it is used in a formatter than when it is used as a filter. In either case, it should be a fairly light weight object that stores enough information to extract the attribute value from the attribute set. How this value is used can be determined from the context.
Technically, it is possible to merge the filter and formatter into a single class. However, this looks like a bad design to me, since the attr usage syntax becomes ambiguous in different ways, and its implementation includes two different and unrelated things. That approach could probably be reasonable in a general-purpose lambda library, but filters and formatters, as they are now, are quite specialized (which is not bad).
and serve different purposes (one - to compose filters, the other - formatters).
Why does this matter? spirit uses the same placeholders in both qi and karma, for instance.
I'm not a specialist in Qi and Karma internals and cannot judge on how that design decision is applicable to Boost.Log. But from my viewpoint, mixing filters and formatters is absolutely not a good idea.
The point is that attr has the same meaning in both a filter and a formatter.
Yes, but offers different capabilities. This makes sense for a filter: attr< std::string >("Tag").begins_with("Important") but is completely nonsense as a formatter.
Also, thread safety requirements are different.
Why? In any sane implementation that I can think of, both attr's should store the same immutable data.
The formatter attr can contain a boost::format member, which makes it thread-unsafe. Thread unsafe design is common for most formatters.
Is it necessary to have a boost::format object in the result of attr?
It's a valuable optimization, since it saves from parsing the format string on every log record.
AMDG Andrey Semashev wrote:
Technically, it is possible to merge the filter and formatter into a single class. However, this looks like a bad design to me, since the attr usage syntax becomes ambiguous in different ways, and its implementation includes two different and unrelated things.
How is it ambiguous? I don't like having to refer to the same thing by different names depending on how I'm going to use it. You should be able to determine how to treat the attr from the context that it's used in.
That approach could probably be reasonable in a general-purpose lambda library, but filters and formatters, as they are now, are quite specialized (which is not bad).
This specialization is bad if it forces you to have separate attrs.
The point is that attr has the same meaning in both a filter and a formatter.
Yes, but offers different capabilities. This makes sense for a filter:
attr< std::string >("Tag").begins_with("Important")
but is completely nonsense as a formatter.
Then I won't use this expression as a formatter. Anyway, if you want to avoid coupling specific tests to the attr, non-members are your friends.
The formatter attr can contain a boost::format member, which makes it thread-unsafe. Thread unsafe design is common for most formatters.
Is it necessary to have a boost::format object in the result of attr?
It's a valuable optimization, since it saves from parsing the format string on every log record.
Have you measured the effect? Oh, I see what you mean. I would have no problem if you dropped the format parameter of attr entirely. In Christ, Steven Watanabe
On 03/16/2010 08:35 PM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
Technically, it is possible to merge the filter and formatter into a single class. However, this looks like a bad design to me, since the attr usage syntax becomes ambiguous in different ways, and its implementation includes two different and unrelated things.
How is it ambiguous? I don't like having to refer to the same thing by different names depending on how I'm going to use it. You should be able to determine how to treat the attr from the context that it's used in.
attr< int >("X", "%04x") > 10 Is that a formatter or a filter? If attr was a lambda placeholder, as you suggest, this expression would probably compile, until being assigned to a variable or being invoked either way. Also, if attr was polymorphic, would this compile: function< bool (attribute_values_view const&) > f = attr< int >("X"); ? That shouldn't compile, assuming that attr< int >("X") is a formatter (because the signature differs) or a filter (because that filter is incomplete).
That approach could probably be reasonable in a general-purpose lambda library, but filters and formatters, as they are now, are quite specialized (which is not bad).
This specialization is bad if it forces you to have separate attrs.
From their implementation standpoint, they have nearly nothing in common.
The point is that attr has the same meaning in both a filter and a formatter.
Yes, but offers different capabilities. This makes sense for a filter:
attr< std::string >("Tag").begins_with("Important")
but is completely nonsense as a formatter.
Then I won't use this expression as a formatter. Anyway, if you want to avoid coupling specific tests to the attr, non-members are your friends.
You mean, like this: bind(begins_with, bind(attr< std::string >("Tag"), _1)), "Important") ? Compared to my syntax, this looks cryptic, to say the least.
The formatter attr can contain a boost::format member, which makes it thread-unsafe. Thread unsafe design is common for most formatters.
Is it necessary to have a boost::format object in the result of attr?
It's a valuable optimization, since it saves from parsing the format string on every log record.
Have you measured the effect? Oh, I see what you mean. I would have no problem if you dropped the format parameter of attr entirely.
But I don't want to drop it. It's quite useful.
AMDG Andrey Semashev wrote:
On 03/16/2010 08:35 PM, Steven Watanabe wrote:
Andrey Semashev wrote:
Technically, it is possible to merge the filter and formatter into a single class. However, this looks like a bad design to me, since the attr usage syntax becomes ambiguous in different ways, and its implementation includes two different and unrelated things.
How is it ambiguous? I don't like having to refer to the same thing by different names depending on how I'm going to use it. You should be able to determine how to treat the attr from the context that it's used in.
attr< int >("X", "%04x") > 10
Is that a formatter or a filter? If attr was a lambda placeholder, as you suggest, this expression would probably compile, until being assigned to a variable or being invoked either way.
Why does it have to compile? The obvious implementation would be for this attr to produce a string when invoked, which obviously cannot be compared to an int. Anyway, this seems like yet another reason to remove Boost.Format from the attr function.
Also, if attr was polymorphic, would this compile:
function< bool (attribute_values_view const&) > f = attr< int >("X");
?
That shouldn't compile, assuming that attr< int >("X") is a formatter (because the signature differs) or a filter (because that filter is incomplete).
I would expect this to compile, but issue a warning about converting int to bool. This conversion is a general problem of C++. I don't see that you gain much by forbidding it.
That approach could probably be reasonable in a general-purpose lambda library, but filters and formatters, as they are now, are quite specialized (which is not bad).
This specialization is bad if it forces you to have separate attrs.
From their implementation standpoint, they have nearly nothing in common.
Then all the sophisticated implementation doesn't belong in attr. It belongs in the whole formatter/filter object.
You mean, like this:
bind(begins_with, bind(attr< std::string >("Tag"), _1)), "Important")
?
Compared to my syntax, this looks cryptic, to say the least.
No I meant, begins_with(attr<std::string>("Tag"), "Important"). I don't understand why you think that random extra stuff is needed just because a member function is turned into a non-member. FWIW, the bind syntax would mostly work as is with no extra work from you, if attr just returned a function object, so I don't think it's necessarily a bad thing, since I really do not like to see you reinventing the wheel in so many places.
Have you measured the effect? Oh, I see what you mean. I would have no problem if you dropped the format parameter of attr entirely.
But I don't want to drop it. It's quite useful.
You can already use Boost.Format at the top level. From what you've said, this use of Boost.Format conflicts with things that I consider much more important. In Christ, Steven Watanabe
On 03/16/2010 09:44 PM, Steven Watanabe wrote:
AMDG
This specialization is bad if it forces you to have separate attrs.
From their implementation standpoint, they have nearly nothing in common.
Then all the sophisticated implementation doesn't belong in attr. It belongs in the whole formatter/filter object.
You mean, like this:
bind(begins_with, bind(attr< std::string >("Tag"), _1)), "Important")
?
Compared to my syntax, this looks cryptic, to say the least.
No I meant, begins_with(attr<std::string>("Tag"), "Important"). I don't understand why you think that random extra stuff is needed just because a member function is turned into a non-member. FWIW, the bind syntax would mostly work as is with no extra work from you, if attr just returned a function object, so I don't think it's necessarily a bad thing, since I really do not like to see you reinventing the wheel in so many places.
I understand where you're leading - to rewrite filters and formatters in terms of Proto or Phoenix. I admit that you made a few very good points, and I'm not against this move. But I'm not ready to drop my syntax advantages in the process. Ideally, I would prefer that the current syntax was possible without changes in the rewritten code. In order to complete this task I'll need time to study these libraries closer.
Have you measured the effect? Oh, I see what you mean. I would have no problem if you dropped the format parameter of attr entirely.
But I don't want to drop it. It's quite useful.
You can already use Boost.Format at the top level.
This feature is useful in streaming formatters.
Hi, I've tried to follow this discussion, without reading the Boost.Log documentation. ----- Original Message ----- From: "Andrey Semashev" <andrey.semashev@gmail.com> To: <boost@lists.boost.org> Sent: Tuesday, March 16, 2010 8:19 PM Subject: Re: [boost] [log] Comments
On 03/16/2010 09:44 PM, Steven Watanabe wrote:
AMDG
This specialization is bad if it forces you to have separate attrs.
From their implementation standpoint, they have nearly nothing in common.
Then all the sophisticated implementation doesn't belong in attr. It belongs in the whole formatter/filter object.
You mean, like this:
bind(begins_with, bind(attr< std::string >("Tag"), _1)), "Important")
?
Compared to my syntax, this looks cryptic, to say the least.
No I meant, begins_with(attr<std::string>("Tag"), "Important").
I like this. It is clear the result of begins_with is a filter on std::string, which is independent of the specific attribute itself.
I don't understand why you think that random extra stuff is needed just because a member function is turned into a non-member. FWIW, the bind syntax would mostly work as is with no extra work from you, if attr just returned a function object, so I don't think it's necessarily a bad thing, since I really do not like to see you reinventing the wheel in so many places.
I understand where you're leading - to rewrite filters and formatters in terms of Proto or Phoenix. I admit that you made a few very good points, and I'm not against this move. But I'm not ready to drop my syntax advantages in the process. Ideally, I would prefer that the current syntax was possible without changes in the rewritten code. In order to complete this task I'll need time to study these libraries closer.
I don't see why do you need Phoenix to implement this function, However Proto will help you surely.
From my point of view
attr< std::string >("Tag").begins_with("Important") is more intrusive than begins_with(attr<std::string>("Tag"), "Important"). And the last is as clear as the first. In addition, as the attributed are typed, The following has no sense to me attr< int>("Att").begins_with("1") Does this works now? What it does?
Have you measured the effect? Oh, I see what you mean. I would have no problem if you dropped the format parameter of attr entirely.
But I don't want to drop it. It's quite useful.
You can already use Boost.Format at the top level.
This feature is useful in streaming formatters.
I think attributes must be a different entity from filters and formatters. A filter or a formatter can concerns several attributes. The following is an attributte or a formatted attribute. attr< int >("X", "%04x") Which is its value type int or std::string? The following is clearly the format of an attribute. There is no ambiguity. format("%04x",attr< int >("X")) Best, Vicente
On 03/16/2010 11:02 PM, vicente.botet wrote:
In addition, as the attributed are typed,
The following has no sense to me
attr< int>("Att").begins_with("1")
Does this works now? What it does?
It won't compile since the attribute value type is not a string.
Have you measured the effect? Oh, I see what you mean. I would have no problem if you dropped the format parameter of attr entirely.
But I don't want to drop it. It's quite useful.
You can already use Boost.Format at the top level.
This feature is useful in streaming formatters.
I think attributes must be a different entity from filters and formatters. A filter or a formatter can concerns several attributes.
They are different entities now.
The following is an attributte or a formatted attribute. attr< int>("X", "%04x") Which is its value type int or std::string?
The result of this is a formatter, which is not an int or string. But the formatter expects an attribute value of type int to be present in the log record. Invoking this formatter results in the attribute value being printed according to the "%04x" format.
AMDG Andrey Semashev wrote:
On 03/15/2010 11:22 PM, Vladimir Prus wrote:
The corresponding code using lambda would be, presumably:
flt::attr< ...>("System") == m_sys
Would it be possible to make the following work in the first case:
if (attrs["System"] != m_sys) return false;
It doesn't specify the type of the value.
You can deduce the type from the type of m_sys.
? Or, if changing type/behaviour of operator[] is undesirable, what about
if (attrs->get_nothrow("System") != m_sys) return false;
That should be:
if (attrs.get_nothrow< System >("System") != m_sys) return false;
Yes, it could be done. But as stated elsewhere in the discussion, this would require System to be copyable (which might be fine in this case). And it doesn't really differ from:
bool result = false; extract< System >("System", attrs, var(result) = _1 != m_sys); return result;
except that the former is more readable.
but with extract it doesn't have the copyability restriction.
Why does this require System to be copyable? Can't you return a reference? In Christ, Steven Watanabe
On 03/16/2010 02:02 AM, Steven Watanabe wrote:
Andrey Semashev wrote:
On 03/15/2010 11:22 PM, Vladimir Prus wrote:
The corresponding code using lambda would be, presumably:
flt::attr< ...>("System") == m_sys
Would it be possible to make the following work in the first case:
if (attrs["System"] != m_sys) return false;
It doesn't specify the type of the value.
You can deduce the type from the type of m_sys.
Ah, right. Clever trick.
? Or, if changing type/behaviour of operator[] is undesirable, what about
if (attrs->get_nothrow("System") != m_sys) return false;
That should be:
if (attrs.get_nothrow< System >("System") != m_sys) return false;
Yes, it could be done. But as stated elsewhere in the discussion, this would require System to be copyable (which might be fine in this case). And it doesn't really differ from:
bool result = false; extract< System >("System", attrs, var(result) = _1 != m_sys); return result;
except that the former is more readable.
but with extract it doesn't have the copyability restriction.
Why does this require System to be copyable? Can't you return a reference?
Returning a reference looks like an unsafe interface design to me. Remember that double dispatch is taken place here, so that the caller's visitor is invoked by the attribute value, with a reference to the actual value. In order to return this reference from get_nothrow, it has to be saved by the visitor and restored after returning from the attribute value dispatch method. At this point the reference may potentially be broken.
AMDG Andrey Semashev wrote:
On 03/16/2010 02:02 AM, Steven Watanabe wrote:
Andrey Semashev wrote:
but with extract it doesn't have the copyability restriction.
Why does this require System to be copyable? Can't you return a reference?
Returning a reference looks like an unsafe interface design to me. Remember that double dispatch is taken place here, so that the caller's visitor is invoked by the attribute value, with a reference to the actual value. In order to return this reference from get_nothrow, it has to be saved by the visitor and restored after returning from the attribute value dispatch method. At this point the reference may potentially be broken.
Then I consider your dispatcher interface to be the unsafe thing. Is there a good reason to allow the attribute value to be constructed on the fly every time it's needed? The only built-in attribute that does this is the current_thread_id attribute, and I don't see a problem with saving the current thread id when getting the attribute's value. In Christ, Steven Watanabe
On 03/16/2010 06:57 PM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
On 03/16/2010 02:02 AM, Steven Watanabe wrote:
Andrey Semashev wrote:
but with extract it doesn't have the copyability restriction.
Why does this require System to be copyable? Can't you return a reference?
Returning a reference looks like an unsafe interface design to me. Remember that double dispatch is taken place here, so that the caller's visitor is invoked by the attribute value, with a reference to the actual value. In order to return this reference from get_nothrow, it has to be saved by the visitor and restored after returning from the attribute value dispatch method. At this point the reference may potentially be broken.
Then I consider your dispatcher interface to be the unsafe thing.
Why? In what way is it unsafe?
Is there a good reason to allow the attribute value to be constructed on the fly every time it's needed?
I tried not to introduce such restriction since it cannot be verified during the compilation.
The only built-in attribute that does this is the current_thread_id attribute, and I don't see a problem with saving the current thread id when getting the attribute's value.
The current_thread_id attribute also exposes itself as the attribute value and thus cannot save the id. Also, attributes, as well as attribute values, may be accessed concurrently, so modifying their state would require locking.
AMDG Andrey Semashev wrote:
On 03/16/2010 06:57 PM, Steven Watanabe wrote:
Andrey Semashev wrote:
Returning a reference looks like an unsafe interface design to me. Remember that double dispatch is taken place here, so that the caller's visitor is invoked by the attribute value, with a reference to the actual value. In order to return this reference from get_nothrow, it has to be saved by the visitor and restored after returning from the attribute value dispatch method. At this point the reference may potentially be broken.
Then I consider your dispatcher interface to be the unsafe thing.
Why? In what way is it unsafe?
I thought that you just described the dangerous behavior.
Is there a good reason to allow the attribute value to be constructed on the fly every time it's needed?
I tried not to introduce such restriction since it cannot be verified during the compilation.
It can if you use a different interface. Ideally, I'd like the interface to be like boost::any (or even /be/ boost::any). i.e. having access to the typeid and the ability to cast to a specific type.
The only built-in attribute that does this is the current_thread_id attribute, and I don't see a problem with saving the current thread id when getting the attribute's value.
The current_thread_id attribute also exposes itself as the attribute value and thus cannot save the id.
Can't it save the thread id in a separate object? It's hard for me to believe that this is a valuable optimization given that none of the other provided attributes behave in this way.
Also, attributes, as well as attribute values, may be accessed concurrently, so modifying their state would require locking.
Then access should be through a reference or a pointer to const. In Christ, Steven Watanabe
On 03/16/2010 08:41 PM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
On 03/16/2010 06:57 PM, Steven Watanabe wrote:
Andrey Semashev wrote:
Returning a reference looks like an unsafe interface design to me. Remember that double dispatch is taken place here, so that the caller's visitor is invoked by the attribute value, with a reference to the actual value. In order to return this reference from get_nothrow, it has to be saved by the visitor and restored after returning from the attribute value dispatch method. At this point the reference may potentially be broken.
Then I consider your dispatcher interface to be the unsafe thing.
Why? In what way is it unsafe?
I thought that you just described the dangerous behavior.
I described unsafe assumption that would have to be taken to implement the suggested interface. There's nothing unsafe in dispatchers.
Is there a good reason to allow the attribute value to be constructed on the fly every time it's needed?
I tried not to introduce such restriction since it cannot be verified during the compilation.
It can if you use a different interface. Ideally, I'd like the interface to be like boost::any (or even /be/ boost::any). i.e. having access to the typeid and the ability to cast to a specific type.
If it had a dispatching (visitation) interface, similar to what I've implemented in Boost.Log, I would have used Boost.Any.
The only built-in attribute that does this is the current_thread_id attribute, and I don't see a problem with saving the current thread id when getting the attribute's value.
The current_thread_id attribute also exposes itself as the attribute value and thus cannot save the id.
Can't it save the thread id in a separate object? It's hard for me to believe that this is a valuable optimization given that none of the other provided attributes behave in this way.
You mean, to save it into a TLS? Sure, it could. But why bother, since with my interface it's not required.
Also, attributes, as well as attribute values, may be accessed concurrently, so modifying their state would require locking.
Then access should be through a reference or a pointer to const.
You lost me here.
On 03/16/2010 10:48 PM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
If it had a dispatching (visitation) interface, similar to what I've implemented in Boost.Log, I would have used Boost.Any.
Oh? See attached.
Well, it resembles my solution rather well (not counting that it tends to bloat the binary size more than necessary). Thinking about it more, it doesn't look as such a good idea. If I used Boost.Any, all attribute values would have to be created as dynamic objects upon acquisition from their attributes. Currently, this is not always the case, which makes the values view composition faster.
AMDG Andrey Semashev wrote:
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. <snip> 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.
Just have a function that returns the attribute or throws an att_not_found exception (Or whatever you want to call it). The library can catch this.
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.
Is there a good reason that the same attr can't be both a formatter and a filter? In Christ, Steven Watanabe
On 03/15/2010 11:30 PM, Steven Watanabe wrote:
AMDG
Andrey Semashev wrote:
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. <snip> 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.
Just have a function that returns the attribute or throws an att_not_found exception (Or whatever you want to call it). The library can catch this.
This looks similar to what Vladimir suggested. See my answer to him.
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.
Is there a good reason that the same attr can't be both a formatter and a filter?
I answered that question to Vladimir, too.
Steven Watanabe wrote:
Andrey Semashev wrote:
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.
Is there a good reason that the same attr can't be both a formatter and a filter?
If not, it would probably make things more readable to have "attr_formatter" and "attr" anyway, though I'd prefer to see "attr" spelled out in each case. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/16/2010 02:13 PM, Stewart, Robert wrote:
Steven Watanabe wrote:
Andrey Semashev wrote:
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.
Is there a good reason that the same attr can't be both a formatter and a filter?
If not, it would probably make things more readable to have "attr_formatter" and "attr" anyway, though I'd prefer to see "attr" spelled out in each case.
formatters::attr_formatter looks too much for me. And it's attr for shortness. I think it's expansion is obvious enough.
Andrey Semashev wrote:
On 03/16/2010 02:13 PM, Stewart, Robert wrote:
Steven Watanabe wrote:
Is there a good reason that the same attr can't be both a formatter and a filter?
If not, it would probably make things more readable to have "attr_formatter" and "attr" anyway, though I'd prefer to see "attr" spelled out in each case.
formatters::attr_formatter looks too much for me. And it's attr for shortness. I think it's expansion is obvious enough.
Given code within the formatters namespace, having a using directive in force, or eliminating the "formatters" namespace as suggested elsewhere, the repetition to which you are objecting would be masked or eliminated. When one sees "attr" in the documentation or code, it won't be confused with "attr_formatter," which is possible when both are named alike unless they are always namespace scoped. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/16/2010 07:04 PM, Stewart, Robert wrote:
Andrey Semashev wrote:
On 03/16/2010 02:13 PM, Stewart, Robert wrote:
Steven Watanabe wrote:
Is there a good reason that the same attr can't be both a formatter and a filter?
If not, it would probably make things more readable to have "attr_formatter" and "attr" anyway, though I'd prefer to see "attr" spelled out in each case.
formatters::attr_formatter looks too much for me. And it's attr for shortness. I think it's expansion is obvious enough.
Given code within the formatters namespace, having a using directive in force, or eliminating the "formatters" namespace as suggested elsewhere, the repetition to which you are objecting would be masked or eliminated.
I think, namespaces do better job in categorizing things than name mangling. As noted, namespaces are much more flexible in possible writings.
When one sees "attr" in the documentation or code, it won't be confused with "attr_formatter," which is possible when both are named alike unless they are always namespace scoped.
The docs always state the name qualified. Also, most of the time it's quite obvious from the context, whether it's a formatter or a filter, even if the qualification wasn't there.
----- Original Message ----- From: "Andrey Semashev" <andrey.semashev@gmail.com> To: <boost@lists.boost.org> Sent: Monday, March 15, 2010 8:39 PM Subject: Re: [boost] [log] Comments
* 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.
If ASIO can not be used on a single thread environement a clear error should be desirable when compiled on this environement. Vicente
On 03/15/2010 11:31 PM, vicente.botet wrote:
If ASIO can not be used on a single thread environement a clear error should be desirable when compiled on this environement.
As I noted, I haven't yet defined the best solution for this problem. In fact, I've been hoping that reviewers suggest which solution might be better: 1. Drop single-threaded builds of Boost.Log. 2. Exclude support for syslog from single-threaded builds. 3. Something else?
Andrey Semashev wrote:
On 03/15/2010 11:31 PM, vicente.botet wrote:
If ASIO can not be used on a single thread environement a clear error should be desirable when compiled on this environement.
As I noted, I haven't yet defined the best solution for this problem. In fact, I've been hoping that reviewers suggest which solution might be better:
AFAIK, Asio can run in a single thread just fine - unless you are using parts of Asio that need a separate thread to emulate asynchronous behavior. These parts are, IIRC, the resolver and the select_reactor on Windows. Cheers, Rutger
On 03/16/2010 06:20 PM, Rutger ter Borg wrote:
Andrey Semashev wrote:
On 03/15/2010 11:31 PM, vicente.botet wrote:
If ASIO can not be used on a single thread environement a clear error should be desirable when compiled on this environement.
As I noted, I haven't yet defined the best solution for this problem. In fact, I've been hoping that reviewers suggest which solution might be better:
AFAIK, Asio can run in a single thread just fine - unless you are using parts of Asio that need a separate thread to emulate asynchronous behavior. These parts are, IIRC, the resolver and the select_reactor on Windows.
Are those parts documented somewhere?
Andrey Semashev wrote:
On 03/15/2010 09:10 PM, Vladimir Prus wrote:
On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
* 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.
Whether Boost.Parameters is the best solution, or as good as any other, for your purpose, don't forget the overhead of compilation time and errors. Template code is, of course, problematic when things go wrong. Parsing the compiler output is often difficult. A simpler, more straightforward solution, along the lines proposed by Vladimir, makes for simpler, more straightforward error messages, and can dramatically reduce compilation time.
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.
I'll note again that this is a narrow view of the matter. We have many applications that log to stderr with the expectation that the command line invocation will redirect stderr to an appropriate file. It leaves the policy -- file pathname -- outside the application. We also have many applications that log to a file but use a command line option to specify the pathname, with a reserved value indicating stderr. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/16/2010 02:35 PM, Stewart, Robert wrote:
Andrey Semashev wrote:
On 03/15/2010 09:10 PM, Vladimir Prus wrote:
On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
* 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.
Whether Boost.Parameters is the best solution, or as good as any other, for your purpose, don't forget the overhead of compilation time and errors. Template code is, of course, problematic when things go wrong. Parsing the compiler output is often difficult. A simpler, more straightforward solution, along the lines proposed by Vladimir, makes for simpler, more straightforward error messages, and can dramatically reduce compilation time.
It's another layer of scaffolding, that I, Vladimir and others so dislike. Boost.Parameter allows to cut down the interface to the essence and also enables the interface abstractions I described earlier. If I provide this additional way of initialization, there will always be the confusion, which way is the recommended one. Thus more complaints from users.
Hi, allow me to jump to this discusion. ----- Original Message ----- From: "Vladimir Prus" <ghost@cs.msu.su> To: <boost@lists.boost.org> Sent: Monday, March 15, 2010 7:10 PM Subject: Re: [boost] [log] Comments
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?
I find useful a record identifier. it allows to sort the log using different criteria and preserving order at the same time (see bellow). Whether the record_id should be in the trivial loggin by default is another issue. IMO the trivial log shouldn't provide any attributes. The library could provide other out of the box logs that take care of some usual configurations.
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 don't agre here. The thread_id, independently of its hexadecimal or a string form, serves to identify the logs of the same thread. I use to work with record_id + thread_id. Then ordering by thread_id and then record_id , gives me a clear sequence by thread. A simple grep of a specific thread_id allows to extract all the log of a thread. Of course, a user defined name for a thread will be more useful. Shouldn't this namming feature be associated to the thread library? <snip>
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.
I agree that it is good to request new features to Boost.Thread, but there are already very old features request for Boost.Thread, and I'm not sure Anthony will have the time to take care of all of them. Maybe someone could help Anthony to integrate the simple features. Best, Vicente
Andrey Semashev wrote:
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything.
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.
Since there's no binary logging, the output is likely to be written to a file. I can use many means to determine a line number from the contents of a text file without embedding a line number proxy.
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.
Who are you to say what we need WRT the console? We frequently write output to stderr with the express intention of redirecting it to a file or pipeline.
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.
That doesn't mean doing so is wrong for anyone else. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/15/2010 09:32 PM, Stewart, Robert wrote:
Andrey Semashev wrote:
On 03/15/2010 01:45 PM, Vladimir Prus wrote:
* Why is trivial logging printing the sequental number of each log message? I don't think this helps anything.
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.
Since there's no binary logging, the output is likely to be written to a file. I can use many means to determine a line number from the contents of a text file without embedding a line number proxy.
Maybe, you're right.
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.
Who are you to say what we need WRT the console? We frequently write output to stderr with the express intention of redirecting it to a file or pipeline.
My point was that you can simply write to stderr without messing with the library. Writing to a file is a more complicated task, and the library provides an easy way to solve it + a few bonuses.
Andrey Semashev wrote:
On 03/15/2010 09:32 PM, Stewart, Robert wrote:
Andrey Semashev wrote:
I disagree. First, you don't need the library to spam the console.
Who are you to say what we need WRT the console? We frequently write output to stderr with the express intention of redirecting it to a file or pipeline.
My point was that you can simply write to stderr without messing with the library. Writing to a file is a more complicated task, and the library provides an easy way to solve it + a few bonuses.
I see. I misread the emphasis in your statement. Isn't English wonderfully vague at times? Still, the formatting and filtering behaviors of the library are still useful to determine what and whether output is written to stderr. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 03/15/2010 10:47 PM, Stewart, Robert wrote:
Andrey Semashev wrote:
On 03/15/2010 09:32 PM, Stewart, Robert wrote:
Andrey Semashev wrote:
I disagree. First, you don't need the library to spam the console.
Who are you to say what we need WRT the console? We frequently write output to stderr with the express intention of redirecting it to a file or pipeline.
My point was that you can simply write to stderr without messing with the library. Writing to a file is a more complicated task, and the library provides an easy way to solve it + a few bonuses.
I see. I misread the emphasis in your statement. Isn't English wonderfully vague at times?
Still, the formatting and filtering behaviors of the library are still useful to determine what and whether output is written to stderr.
True. I guess, what it all comes down to, is that the trivial logging is too trivial and needs a bit more configurability (but just a tiny bit!) :). I agree with that.
On Monday 15 March 2010 13:45:12 Vladimir Prus wrote:
* I have used the attached script to measure code size impact. The script basically creates a test file with a single function calling BOOST_LOG_TRIVIAL and measures the size of that function depending on the number of calls. I've got 139 bytes per call for 'g++ -Os' and 174 per call for 'g++ -O3', which seems nice. This is function code only. When counting file size I get 324 and 514 respectively. For reference, kDebug gives 136 and 328 of file_size_bytes per call. I guess these results are OK.
I have now measured the compile time performance. An file that outputs a string to std::cout and does nothing else takes 0.27 seconds to compile on my system. Compiling the main.cpp from 'log/example/trivial' takes 2.47. For avoidance of doubt, I've repeated timing several times, with no significant deviation, and it's naturally hot cache case. So, including Boost.Log headears appear to add 2.2 seconds to compilation time. This might sounds not much, but it adds up. For example, one part of KDE, named kdevplatform, has 562 files. So, if every one of them starts to use Boost.Log, that would add 1236 seconds, or 20 minutes, to the compilation time. The problem is that complete build of that project (which already uses KDE's logging mechanisms), takes 16 minutes. Hmm, maybe I did something wrong. Ah, right, trivial's main.cpp incluldes filters.hpp. When I try with a source file that includes only trivial.hpp, the compilation time is 1.67 seconds, the overhead being 1.4 per file and estimated overhead on my project being 786 seconds or 13 minutes. Still, compared to 16 minutes it takes to compile "business code", 13 minutes overhead seems unacceptable. Does anybody sees any flaw in my methodology? Unless I'm wrong, I recommend that trivial logging be redesigned to cut down on includes necessary. On the extreme end, printf-style wrappers can reduce compile time overhead to nearly zero. It probably would be possible to abstract things away so that BOOST_LOG_TRIVIAL(...) essentially returns std::ostream, without exposing the real types to the user code. Thanks, Volodya
Vladimir Prus wrote:
I have now measured the compile time performance. An file that outputs a string to std::cout and does nothing else takes 0.27 seconds to compile on my system. Compiling the main.cpp from 'log/example/trivial' takes 2.47. For avoidance of doubt, I've repeated timing several times, with no significant deviation, and it's naturally hot cache case.
So, including Boost.Log headears appear to add 2.2 seconds to compilation time. This might sounds not much, but it adds up. For example, one part of KDE, named kdevplatform, has 562 files. So, if every one of them starts to use Boost.Log, that would add 1236 seconds, or 20 minutes, to the compilation time. The problem is that complete build of that project (which already uses KDE's logging mechanisms), takes 16 minutes.
Sorry for jumping in. At the risk of asking a silly question: shouldn't the include guards kick in and keep this overhead in the realms of constant time instead of linear time? Regards, Rutger
On Tuesday 16 March 2010 11:46:21 Rutger ter Borg wrote:
Vladimir Prus wrote:
I have now measured the compile time performance. An file that outputs a string to std::cout and does nothing else takes 0.27 seconds to compile on my system. Compiling the main.cpp from 'log/example/trivial' takes 2.47. For avoidance of doubt, I've repeated timing several times, with no significant deviation, and it's naturally hot cache case.
So, including Boost.Log headears appear to add 2.2 seconds to compilation time. This might sounds not much, but it adds up. For example, one part of KDE, named kdevplatform, has 562 files. So, if every one of them starts to use Boost.Log, that would add 1236 seconds, or 20 minutes, to the compilation time. The problem is that complete build of that project (which already uses KDE's logging mechanisms), takes 16 minutes.
Sorry for jumping in. At the risk of asking a silly question: shouldn't the include guards kick in and keep this overhead in the realms of constant time instead of linear time?
No, I don't think so. Each source file is compiled separately. - Volodya
On Tuesday 16 March 2010 11:53:21 edouard@fausse.info wrote:
On Tue, 16 Mar 2010 11:29:27 +0300, Vladimir Prus <ghost@cs.msu.su> wrote:
I have now measured the compile time performance.
Which version of g++ did you use?
It is: gcc version 4.4.1 (Ubuntu 4.4.1-4ubuntu9) is it too new or too old? Thanks, Volodya
On Tue, 16 Mar 2010 14:04:11 +0300, Vladimir Prus <ghost@cs.msu.su> wrote:
Which version of g++ did you use?
It is:
gcc version 4.4.1 (Ubuntu 4.4.1-4ubuntu9)
is it too new or too old?
4.4 is the compiler we use here too, but I've been informed that 4.5 improves template-related performances greatly. I'd say that it would be interesting to see the compilation time under MSVC, but what you've found out is already problematic. I agree with you when you say that 2 s per file is too much. Logging is a commodity, it's something that tend to be present all around your software. What strikes me is that - if I understood correctly - Boost.Log is not header only and yet, it greatly increases compilation time. Am I missing something? -Edouard
On 03/16/2010 11:29 AM, Vladimir Prus wrote:
On Monday 15 March 2010 13:45:12 Vladimir Prus wrote:
I have now measured the compile time performance. An file that outputs a string to std::cout and does nothing else takes 0.27 seconds to compile on my system. Compiling the main.cpp from 'log/example/trivial' takes 2.47. For avoidance of doubt, I've repeated timing several times, with no significant deviation, and it's naturally hot cache case.
So, including Boost.Log headears appear to add 2.2 seconds to compilation time. This might sounds not much, but it adds up. For example, one part of KDE, named kdevplatform, has 562 files. So, if every one of them starts to use Boost.Log, that would add 1236 seconds, or 20 minutes, to the compilation time. The problem is that complete build of that project (which already uses KDE's logging mechanisms), takes 16 minutes.
Hmm, maybe I did something wrong. Ah, right, trivial's main.cpp incluldes filters.hpp. When I try with a source file that includes only trivial.hpp, the compilation time is 1.67 seconds, the overhead being 1.4 per file and estimated overhead on my project being 786 seconds or 13 minutes. Still, compared to 16 minutes it takes to compile "business code", 13 minutes overhead seems unacceptable. Does anybody sees any flaw in my methodology?
Unless I'm wrong, I recommend that trivial logging be redesigned to cut down on includes necessary. On the extreme end, printf-style wrappers can reduce compile time overhead to nearly zero. It probably would be possible to abstract things away so that
BOOST_LOG_TRIVIAL(...)
essentially returns std::ostream, without exposing the real types to the user code.
Thanks for the figures. Right now I cannot comment on what is the reason on such performance. I don't think it's unacceptable, but I would surely be willing to minimize it as much as possible (without much impact on the user's interface, if possible). Quick analysis shows that the amount of code brought in by trivial.hpp is rather big, and this might be one source of the problem. For instance, it brings in a great deal of Boost.DateTime (through Boost.Thread) and Boost.MPL and Boost.TypeTraits (partially, through Boost.Parameter). I guess, I'll have to either reimplement some parts of these libraries or use hacks to hide them completely within Boost.Log binary to speed up the compilation. BTW, have you tried if precompiled headers help?
participants (10)
-
Andrey Semashev -
Christian Holmquist -
Darryl Green -
edouard＠fausse.info -
Roland Bock -
Rutger ter Borg -
Steven Watanabe -
Stewart, Robert -
vicente.botet -
Vladimir Prus