-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remove itt_notify
module
#1290
Remove itt_notify
module
#1290
Conversation
afa12a0
to
4117d89
Compare
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Quick questions: Unless the pool name is too long it fits in 15 chars |
They're the global (for the whole runtime) and local (to the pool) thread indices. Nothing to do with distributed, though I see how that might be confusing... E.g. the MPI pool might say Isn't it possible to abbreviate the pthread name instead of simply cropping it? e.g:
Definitely possible. This was the simplest way of getting them to fit without having to do a lot of custom logic. If I can find a simple way of shortening it I'll consider it for this PR, otherwise I'll probably leave that for "sometime". Not sure it's worth a lot of effort, especially since one can still print the full name manually. |
4117d89
to
156bdb7
Compare
@rasolca I've updated the short thread names to be What do you think about the change? |
156bdb7
to
87205f5
Compare
Remove `itt_notify` module
6faa0ff
to
6db9077
Compare
Abbreviate thread name to "pika/<first char of context>/<first char of pool name>/<local thread index>", e.g. "pika/w/d/3" for the fourth worker thread on the [d]efault pool, which is a [w]orker thread.
…s are backed by static strings
6db9077
to
8cd2180
Compare
I believe we haven't attempted to use the ITT support in a long time and I don't see ourselves using it in the near future, so I'm proposing to remove it.
I've also updated
set_thread_name
to set the pthread name. This name shows up in e.g. gdb when doinginfo threads
:The only quirk is that a pthread name can be maximum 15 characters (excluding a null terminator). For this reason I set a "short" thread name for pthreads, such as
pika/w/d/2
, which denotes aw
orker thread on thed
efault pool with thread index2
. This shortened name can be ambiguous since it only uses the first character of the thread pool name, but for most purposes this should be unique enough (the default pool is calleddefault
, and if an MPI polling pool is used it's calledpolling
). The full thread name can still be accessed withprint pika::detail::get_thread_name()
on some thread in gdb (in somewhat obscured form, since it's stored in astd::string
), e.g.