Thread name by Zelnes · Pull Request #1 · Lbardoux/log

Zelnes · 2017-04-01T11:48:39Z

I've added a feature that binds a thread id to a string.
This allows a user to set a name to several threads.
It gives thins kind of output for this code :

#include "logs.hpp"
#include <thread>

void printNameTest()
{
	mlog::Options::setFormat("[{TYPE} {DATE} {TIME} {THREAD}] ");
	mlog::info("Before binding the TEST value");
	mlog::Options::bindThreadName(std::this_thread::get_id(), "TEST");
	mlog::info("After binding the TEST value");
}

int main(int argc, char const *argv[])
{
	std::thread test(printNameTest);
	test.join();
	return 0;
}

Gives :

user$ g++  test.cpp -o main.out -pthread -std=c++11
user$ ./main.out 
[INFO    04/01/2017 13:42:56 0x7fec67a3e700] Before binding the TEST value
[INFO    04/01/2017 13:42:56 TEST] After binding the TEST value
user$

Assuming we have ./test.cpp and ./logs.hpp

…g header, to use a string instead of the thread id. Though, thread id will be use as default value until you reach the directive to add a name to a thread id. You can after change the value, or delete the id-string association

…unbindThreadName

…EAD_NAME, mlog::Options::bindThreadName and mlog::Options::unbindThreadName

Lbardoux · 2017-04-01T17:19:59Z

+ * @param[in]  name  Name that should be displayed instead of an id
+ * @see mlog::Options::unbidThreadName
+ * 
+ * @fn void mlog::Options::unbidThreadName(const std::thread::id& id)


A n is missing right here, (unbid instead of unbind), please add it in order to get consistent documentation.

Lbardoux · 2017-04-01T17:20:40Z

+ *
+ * @param[in]  id    The identifier returned by std::thread::get_id()
+ * @param[in]  name  Name that should be displayed instead of an id
+ * @see mlog::Options::unbidThreadName


Same story here, unbid instead of unbind.

Lbardoux · 2017-04-01T17:21:03Z

+ * @var mlog::__details::__Static_declarer::THREAD_NAME
+ * The container for bounds between thread id and a string value
+ * @see mlog::Options::bindThreadName
+ * @see mlog::Options::unbidThreadName


Here again.

Lbardoux · 2017-04-01T17:22:30Z

                            case -3:
-#                               ifdef MTL_LOG_WITH_THREADS
-                                out << "0x" << std::hex << std::this_thread::get_id() << std::dec;
+#                               ifdef _REENTRANT


Please use MTL_LOG_WITH_THREADS macro instead of _REENTRANT.

Lbardoux · 2017-04-01T17:22:59Z

                static bool                                   ENABLE_ALPHA_BOOL;
                static MTL_LOG_NAMESPACE::__details::__Header FORMAT;

+#               ifdef _REENTRANT


MTL_LOG_WITH_THREADS instead of _REENTRANT

Lbardoux · 2017-04-01T17:23:29Z

                MTL_LOG_LOCK;
                return MTL_LOG_NAMESPACE::Options::FORMAT;
            }
+#           ifdef _REENTRANT


MTL_LOG_WITH_THREADS

Lbardoux · 2017-04-01T17:23:48Z

+            {
+                MTL_LOG_NAMESPACE::Options::THREAD_NAME.insert(std::make_pair(id, name));
+            }
+            static void unbidThreadName(const std::thread::id& id)


unbid instead of unbind

Lbardoux · 2017-04-01T17:24:02Z

                                                                   MTL_LOG_NAMESPACE::Options::C_BLANK,
-                                                                   MTL_LOG_NAMESPACE::Options::isColorEnabled());
+                                                                   MTL_LOG_NAMESPACE::Options::isColorEnabled(),
+#                                                                  ifdef _REENTRANT


MTL_LOG_WITH_THREADS

Lbardoux · 2017-04-01T17:25:18Z

+ * tag will print the thread id
+ *
+ * @param[in]  id    The identifier returned by std::thread::get_id()
+ * @see mlog::Options::bidThreadName


bid instead of bind

Lbardoux · 2017-04-01T17:26:32Z

-                             const char *const nocolor, bool colorEnabled)
+                             const char *const nocolor, bool colorEnabled, const void* threads_names)
                {
+#                   ifdef _REENTRANT


MTL_LOG_WITh_THREADS

Lbardoux · 2017-04-01T17:51:22Z

I don't think this is a good idea to provide functions if and only if a special flag or macro was provided.
It causes trouble if this flag or macro suddenly disappears, with undefined symbols.
I do prefer a function that does nothing, easily remove by the compiler.

Also, you could skip the first argument (std::thread::id) by adding it on the function body directly, assuming the function does something only if we're working with threads. Then users won't have to write it by themselve.
In addition, MTL_LOG_LOCK to perform mutual exclusion is a good idea and will save time for me later with python and java wrapping.

The functions or now always visible, and they don't need the thread id any more. If MTL_LOG_WITH_THREADS is defined, then the function will have a job to do, otherwise it returns immediately.

Zelnes · 2017-04-01T18:37:30Z

I suppose that the fixes added are good, if you mind check one more time, and I think we'll be good !

Lbardoux · 2017-04-02T10:46:15Z

+#               endif
            }
-            static void unbidThreadName(const std::thread::id& id)
+            static void unbidThreadName()


unbid is still here, and be explicit about functions that doesn't take parameter (consistence with the entire file) with void keyword.

Lbardoux · 2017-04-02T10:53:41Z

It looks well, but one more complaint, why void* ?
I mean you could easily use a typedef on __details scope, and then use a pointer to that specific type. It also avoid this reinterpret cast.

Lbardoux · 2017-04-02T13:02:39Z

Tested with openMP with a simple pragma omp parallel and it seems to work

…an be changed when we find a way to use thread with mingw.Addition of MTL_THREAD_ID that is thread id type when MTL_LOG_WITH_THREADS is defined, otherwise it's int type. This macro avoid errors. The display function contains the THREAD parameter only if MTL_LOG_WITH_THREADS is defined. Modification of bindThreadName, now there are two functions. The first one binds the given name to the current thread id, while the other one binds a specific thread id to the given name.

Zelnes added 4 commits April 1, 2017 12:20

Adding latest master version to thread feature

e389145

Modification of addThreadName to bindThreadName and delThreadName to …

7cea2a3

…unbindThreadName

Addition of documentation refering to MTL_LOG_NAMESPACE::Options::THR…

e9a5790

…EAD_NAME, mlog::Options::bindThreadName and mlog::Options::unbindThreadName

Lbardoux reviewed Apr 1, 2017

View reviewed changes

Zelnes added 3 commits April 1, 2017 20:16

Spell checking of the documentation

c006aee

Modification of _REENTRANT to MTL_LOG_WITH_THREADS

4171594

Update of functions

dbc86b8

The functions or now always visible, and they don't need the thread id any more. If MTL_LOG_WITH_THREADS is defined, then the function will have a job to do, otherwise it returns immediately.

Lbardoux reviewed Apr 2, 2017

View reviewed changes

Conversation

Zelnes commented Apr 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lbardoux commented Apr 1, 2017

Uh oh!

Zelnes commented Apr 1, 2017

Uh oh!

Lbardoux Apr 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lbardoux commented Apr 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lbardoux commented Apr 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Zelnes commented Apr 1, 2017 •

edited

Loading

Lbardoux Apr 2, 2017 •

edited

Loading

Lbardoux commented Apr 2, 2017 •

edited

Loading