-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show thread name on threads list, if available. #60
Conversation
Find if GetThreadDescription() is available in Kernel32.dll (it's available from the Win10 Creators update onwards). If it is, add another column "Thread Name" to the list of threads shown when selecting which thread(s) to profile. If this function is unavailable, don't show this extra column at all. If it is available, we rely on the application being profiled to have called SetThreadDescription(), but if it has not, we just show "-" instead.
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.
LGTM aside the Fn
identifier.
Worth noting that there is another, older way to "set" the thread name (by raising and catching an exception, which would be caught by a debugger).
src/profiler/threadinfo.cpp
Outdated
@@ -35,18 +35,37 @@ static __int64 getTotal(FILETIME time) | |||
return (__int64(time.dwHighDateTime) << 32) + time.dwLowDateTime; | |||
} | |||
|
|||
typedef HRESULT( WINAPI *Fn )(HANDLE, PWSTR*); | |||
static Fn GetThreadDescription = reinterpret_cast<Fn>(GetProcAddress( GetModuleHandle( TEXT( "Kernel32.dll" ) ), "GetThreadDescription" )); |
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.
Please pick a more unique global-scope identifier than Fn
, e.g. GetThreadDescriptionFunc
.
src/wxProfilerGUI/threadlist.cpp
Outdated
@@ -362,3 +390,4 @@ std::wstring ThreadList::getLocation(HANDLE thread_handle) { | |||
|
|||
return L"-"; | |||
} | |||
|
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.
Spurious whitespace change.
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.
Oops. Fixed!
We used that older way to set a thread name before the new API in our code but as far as I could figure out, it was only available inside the debugger. I had a quick search there but can't see any way that we could add code to read it. Otherwise I would be happy to add something in for this too. |
Yes, that's right. Theoretically, Very Sleepy could capture it if the program would set it while it was being profiled. There is some code for "debugging", for the purpose of profiling new threads as they are created, however it is not currently finished, so it would take some work to get it that far. |
Thank you for submitting this improvement. |
Find if GetThreadDescription() is available in Kernel32.dll (it's available from the Win10 Creators update onwards). If it is, add another column "Thread Name" to the list of threads shown when selecting which thread(s) to profile. If this function is unavailable, don't show this extra column at all. If it is available, we rely on the application being profiled to have called SetThreadDescription(), but if it has not, we just show "-" instead.