Logging improvements: options & custom log record handling #167
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| #ifndef NETWORK_LOGGING_HPP_20121112 | ||
| #define NETWORK_LOGGING_HPP_20121112 | ||
|
|
||
| #include <sstream> | ||
| #include <functional> | ||
|
|
||
| namespace network { namespace logging { | ||
|
|
||
| class log_record; | ||
|
|
||
| //using log_record_handler = std::function< void (const std::string&) >; // use this when VS can compile it... | ||
| typedef std::function< void (const log_record&) > log_record_handler; | ||
|
|
||
| void set_log_record_handler( log_record_handler handler ); | ||
| void log( const log_record& message ); | ||
|
|
||
| namespace handler | ||
| { | ||
| log_record_handler get_std_log_handler(); | ||
| log_record_handler get_default_log_handler(); | ||
| } | ||
|
|
||
| /** Helper to build a log record as a stream. */ | ||
| class log_record | ||
| { | ||
| public: | ||
| log_record(){} // = default; | ||
|
|
||
| // Implicit construction from string | ||
| log_record( const std::string& message ) | ||
| { | ||
| write( message ); | ||
| } | ||
|
|
||
| ~log_record() | ||
| { | ||
| log( *this ); | ||
| } | ||
|
|
||
| template< typename TypeOfSomething > | ||
| log_record& write( const TypeOfSomething& something ) // THINK: use universal references? | ||
|
There was a problem hiding this comment. Yes, use TypeOfSomething&& here. |
||
| { | ||
| m_text_stream << something; | ||
| return *this; | ||
| } | ||
|
|
||
| std::string full_message() const { return m_text_stream.str(); } | ||
|
|
||
| private: | ||
| // disable copy | ||
| log_record( const log_record& ); // = delete; | ||
| log_record& operator=( const log_record& ); // = delete; | ||
|
There was a problem hiding this comment. Just use =delete as we're C++11 only anyway. |
||
|
|
||
| std::ostringstream m_text_stream; // stream in which we build the message | ||
|
|
||
| }; | ||
|
|
||
| template< typename TypeOfSomething > | ||
| inline log_record& operator<<( log_record& log, const TypeOfSomething& something ) // THINK: use universal references? | ||
|
There was a problem hiding this comment. Yes use const TypeOfSomething&& here. There was a problem hiding this comment. You mean non-const then? It's not universal reference if it's const if I understand correctly. |
||
| { | ||
| return log.write( something ); | ||
| } | ||
|
|
||
| }} | ||
|
|
||
|
|
||
| #endif /* end of include guard: NETWORK_LOGGING_HPP_20121112 */ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
|
|
||
| #ifdef NETWORK_NO_LIB | ||
| #undef NETWORK_NO_LIB | ||
| #endif | ||
|
|
||
| #include <iostream> | ||
| #include <boost/thread/shared_mutex.hpp> | ||
| #include <boost/thread/shared_lock_guard.hpp> | ||
|
There was a problem hiding this comment. Have you looked at There was a problem hiding this comment. You mean at standard mutexes? The problem is that there is no shared mutex (multiple readers, single writer) in the standard... or I could replace with std::mutex but then the reading lock would be expensive. There was a problem hiding this comment. Why would you need a reader? This is a logging sink right, why do you need to synchronize on reads? There was a problem hiding this comment. This mutex is used only to lock on changing the log handler. The log handler itself should have it's own mechanism to handle thread-safe logging (as boost::log do for example). So here we shouldn't lock (I think) when only calling the handler, just let it manage thread safety. The implementation will be able to make the good choices for efficiency itself, like using some kind of concurrent log record stack (one of the configuration of boost::log does that for example). I think it's a problem that should be solved by the logging implementation, not by the log user, that is cppnetlib. There was a problem hiding this comment. I see. So you're trying to make readers wait on writers if there are any. That sounds reasonable. However, why not just use a shared_ptr and copy that around instead? That way you don't have to lock when you're changing the shared_ptr as .reset() is guaranteed to be race-free. There was a problem hiding this comment. That's right! I didn't think about this way of doing. The shared_ptr would lock on assignation/reset. However, am I correct in thinking that if I test the shared_ptr then dereference it, I have no guarantee that I'm not derefenrencing nullptr? I mean, the only correct thread-safe way I see it working is if I copy the shared_ptr in an other one, which would be almost the same as locking it (and releasing it)? (on the performance side I mean) There was a problem hiding this comment. Yes, make a copy then use the copy of the shared_ptr. That's how shared_ptr was designed to be used IIUC. 😃 Compare the cost of this with the reader-writer lock. I don't think you're going to see much of a difference as both of them will be incrementing an atomic count anyway. 😀 |
||
| #include <network/logging/logging.hpp> | ||
|
|
||
| namespace network { namespace logging { | ||
|
|
||
| namespace handler | ||
| { | ||
| namespace | ||
| { | ||
| void std_log_handler( const log_record& log ) | ||
| { | ||
| std::cerr << log.full_message() << std::endl; | ||
| } | ||
| } | ||
|
|
||
| log_record_handler get_std_log_handler() { return &std_log_handler; } | ||
| log_record_handler get_default_log_handler() { return &std_log_handler; } | ||
| } | ||
|
|
||
|
|
||
| namespace | ||
| { | ||
| // the log handler have to manage itself the thread safety on call | ||
| log_record_handler current_log_record_handler = handler::std_log_handler; | ||
| boost::upgrade_mutex mutex_log_handler; // we still need to not change the log handler concurrently | ||
|
There was a problem hiding this comment. Correct me if I'm wrong, but objects in anonymous namespace are implicitly static (and their symbol isn't exposed). Am I wrong? There was a problem hiding this comment. I can't cite the standard at the moment, but this suggest what I believe: http://stackoverflow.com/questions/8549659/c-namespace-and-static-variables There was a problem hiding this comment. They're not implicitly static, they're implicitly with internal linkage. Marking this static ensures that it's initialised as a static object and if it's a POD then it's static value-initialized, meaning no runtime overhead. Also its lifetime is ensured to be valid before main() is called and until std::at_exit is called (and when other threads exit). There was a problem hiding this comment. Ok, I'll add static. I'll need to check in the standard to clarify my knowledge about this later. |
||
| } | ||
|
|
||
|
|
||
| void set_log_record_handler( log_record_handler handler ) | ||
| { | ||
| boost::lock_guard<boost::upgrade_mutex> write_lock( mutex_log_handler ); | ||
| current_log_record_handler = handler; | ||
| } | ||
|
|
||
| void log( const log_record& log ) | ||
| { | ||
| boost::shared_lock<boost::upgrade_mutex> read_lock( mutex_log_handler ); | ||
| if( current_log_record_handler ) | ||
| { | ||
| current_log_record_handler( log ); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| }} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use =default now that we're C++11 only anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I just cannot compile this at this time as I'm using VS2012 and don't have easy access to other compiler for some time. I'll do it anyway and try to compile it on my server.