Skip to content
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

Give our threads names #4356

Merged
merged 2 commits into from
May 25, 2018
Merged

Conversation

Wallacoloo
Copy link
Member

Previously all our threads were named QThread. The thread name shows up when listing the processes with ps, and also when debugging in gdb. Providing a real thread name makes stack dumps & such that much easier to read.

Example before/after:

(gdb) info threads
  Id   Target Id         Frame 
* 1    Thread 0x7ffff7f90800 (LWP 10519) "lmms" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  2    Thread 0x7fffe8d5b700 (LWP 10523) "QXcbEventReader" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  3    Thread 0x7fffdec30700 (LWP 10524) "gmain" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  4    Thread 0x7fffde42f700 (LWP 10525) "gdbus" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  5    Thread 0x7fffdcfd5700 (LWP 10526) "QDBusConnection" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  6    Thread 0x7fffcb06a700 (LWP 10528) "QThread" 0x00007ffff7bc707c in pthread_cond_wait@@GLIBC_2.3.2 () from /usr/lib/libpthread.so.0
  7    Thread 0x7fffca869700 (LWP 10529) "QThread" 0x00007ffff7bc707c in pthread_cond_wait@@GLIBC_2.3.2 () from /usr/lib/libpthread.so.0
  8    Thread 0x7fffca068700 (LWP 10530) "QThread" 0x00007ffff7bc707c in pthread_cond_wait@@GLIBC_2.3.2 () from /usr/lib/libpthread.so.0
  9    Thread 0x7ffff7ef0700 (LWP 10531) "PulseHotplug" 0x00007ffff367edd6 in ppoll () from /usr/lib/libc.so.6
  10   Thread 0x7fff9693e700 (LWP 10532) "SDLAudioP1" 0x00007ffff367edd6 in ppoll () from /usr/lib/libc.so.6
  11   Thread 0x7fff9613d700 (LWP 10533) "MidiAlsaSeq" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  12   Thread 0x7fff9593c700 (LWP 10534) "QThread" 0x00007ffff7bc707c in pthread_cond_wait@@GLIBC_2.3.2 () from /usr/lib/libpthread.so.0
  17   Thread 0x7fff75850700 (LWP 10540) "dconf worker" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6

after:

(gdb) info threads
  Id   Target Id         Frame 
* 1    Thread 0x7ffff7f90800 (LWP 12501) "lmms" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  2    Thread 0x7fffe8d5b700 (LWP 12505) "QXcbEventReader" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  3    Thread 0x7fffdec30700 (LWP 12506) "gmain" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  4    Thread 0x7fffde42f700 (LWP 12507) "gdbus" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  5    Thread 0x7fffdcfd5700 (LWP 12508) "QDBusConnection" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  6    Thread 0x7fffcb06a700 (LWP 12510) "MixerWorkerThre" 0x00007fffb1b93e19 in runDysonCompress () from /home/colin/src/lmms/build/plugins/ladspa/dyson_compress_1403.so
  7    Thread 0x7fffc2869700 (LWP 12511) "MixerWorkerThre" 0x00007ffff7bca3ac in __lll_lock_wait () from /usr/lib/libpthread.so.0
  8    Thread 0x7fffca869700 (LWP 12512) "MixerWorkerThre" 0x00007ffff7bc707c in pthread_cond_wait@@GLIBC_2.3.2 () from /usr/lib/libpthread.so.0
  9    Thread 0x7ffff7ef0700 (LWP 12515) "PulseHotplug" 0x00007ffff367edd6 in ppoll () from /usr/lib/libc.so.6
  10   Thread 0x7fff96794700 (LWP 12516) "SDLAudioP1" 0x00007ffff367edd6 in ppoll () from /usr/lib/libc.so.6
  11   Thread 0x7fff95f93700 (LWP 12517) "MidiAlsaSeq" 0x00007ffff367ecd9 in poll () from /usr/lib/libc.so.6
  12   Thread 0x7fff95792700 (LWP 12518) "QThread" 0x00005555556b8e32 in MixerWorkerThread::JobQueue::wait() ()

Part of the text gets cut off for longer thread names, but it still gives one something to grep for, at least.

It helps with debugging.
@Wallacoloo
Copy link
Member Author

I should note that I didn't name the different audio drivers, like AudioAlsa and AudioOss. Might push another commit here for that if I get a few minutes this week.

@Wallacoloo
Copy link
Member Author

Hmm. So it turns out the other solution is to add Q_OBJECT at the top of each class that derives from QThread, and Qt's Meta Object Compiler (moc) will stamp out implementations for some virtual functions, including one that returns the class name as a string. Then, QThread automatically calls that virtual function and gets the appropriate name to apply to each thread.

I suspect there might be some reasons to not begin sprinkling Q_OBJECT around inside core classes though. But on the other hand, MixerWorkerThread & such already derive from QThread, which means all those virtual functions are already there - just not overriden.

Advice would be welcome - which approach should I use: call setObjectName("NAME_OF_CLASS") in the constructor of each type that derives from QThread, or add Q_OBJECT to the top of each type that derives from QThread?

@lukas-w
Copy link
Member

lukas-w commented May 16, 2018

@Wallacoloo I think using Q_OBJECT is the best approach. It avoids duplicating the class name and I've yet to see a proof for any considerable overhead, especially for classes that already inherit from QObject. AFAIK, All it does in this case is add a QMetaObject instance to the class.

By default, QThread sets its name based on the Qt meta class. To get an
accurate metaclass, the class which inherits QThread must declare
Q_OBJECT in its header. Futhermore, Qt's MOC requires that a Qt type be
the primary base class when declaring Q_OBJECT, hence the order of
base classes has been rearranged for some classes.
@Wallacoloo
Copy link
Member Author

Okay, I've implemented this for the Midi and Audio drivers. I've also changed to use Q_OBJECT instead of setObjectName, except for Mixer::fifoWriter because Q_OBJECT declarations don't work for nested classes.

I had to reorder some base classes, because the Q_OBJECT macro assumes the primary base class derives from QObject.

Would like for somebody to give this a quick once-over before merging to make sure I didn't do something dumb. :-)

@Wallacoloo Wallacoloo merged commit a3cdda0 into LMMS:master May 25, 2018
@Wallacoloo Wallacoloo deleted the feature/name-threads branch May 25, 2018 01:01
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
* Give our threads names

It helps with debugging.

* Use Q_OBJECT macro to automatically name threads.

By default, QThread sets its name based on the Qt meta class. To get an
accurate metaclass, the class which inherits QThread must declare
Q_OBJECT in its header. Futhermore, Qt's MOC requires that a Qt type be
the primary base class when declaring Q_OBJECT, hence the order of
base classes has been rearranged for some classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants