Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Re-adding getThreadName() and setThreadName() to Log.cpp. #73

Merged
merged 1 commit into from
May 3, 2016
Merged

Re-adding getThreadName() and setThreadName() to Log.cpp. #73

merged 1 commit into from
May 3, 2016

Conversation

bobsummerwill
Copy link
Contributor

@bobsummerwill bobsummerwill commented May 2, 2016

This is a reworking of #72, @rainbeam.
I think it would probably be safer to retain the original code for GLIBC platforms (ie. nearly all Linux platforms).

As I'm typing this, I am wondering why we have these direct calls at all, though, rather than using Boost or C++11 threading API?
Anybody know?

@@ -175,7 +175,7 @@ extern "C" __declspec(dllimport) void __stdcall OutputDebugStringA(const char* l

string dev::getThreadName()
{
#if defined(__APPLE__)
if defined(__GLIBC__) || defined(__APPLE__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the # at the start

@rainbreak
Copy link
Contributor

Yep, seems safer this way.

@bobsummerwill
Copy link
Contributor Author

Typo fixed, thanks, @rainbeam.

@bobsummerwill
Copy link
Contributor Author

There is a build break for Ubuntu (http://52.28.164.97/job/project-build/label=master/4541/console) which I will investigate. I am also looking into whether there is some means of doing this in a cross-platform way with Boost threads or C++ threads.

If there isn't I will add comments to all this code with links to API, articles, etc. explaining why we need this wrapping.

@@ -187,7 +187,7 @@ string dev::getThreadName()

void dev::setThreadName(string const& _n)
{
#if defined(__APPLE__)
#if defined(__GLIBC__) || defined(__APPLE__)
pthread_setname_np(_n.c_str());
Copy link
Contributor

@rainbreak rainbreak May 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously there were different pathways for linux and apple. Linux used

pthread_setname_np(pthread_self(), _n.c_str());

getname was the same usage. I guess apple provides implicit thread_self. Edit: yep, apple can only set the name of the current thread.

@rainbreak
Copy link
Contributor

This is a reworking of #72.
I think it would probably be safer to retain the original code for GLIBC platforms (ie. nearly all Linux platforms).

Added lots of comments and URLs to explain why this code is the way it is.   I will also follow up on the Boost mailing lists to ask why they don't provide API for thread names, which must be a very common need.
@bobsummerwill
Copy link
Contributor Author

Thanks, @rainbeam. Obviously only had my brain half on. Restore that code, and added a bunch of comments detailing our learnings.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants