degeron / qmmp

Automatically exported from code.google.com/p/qmmp
0 stars 0 forks source link

I found the cause of high CPU usage. #685

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
It's a known problem that qmmp had higher CPU resource usage than other similar 
players.
After some hacking, I found two of the possible causes.
The bugs are in several files:
1. volumecontrol.cpp:
The QTimer signals fire too frequently. Signals/slots are heavier than ordinary 
function calls. Besides, you do it every 150 ms. This too-frequent polling of 
volume makes the program very busy. I'd suggest that you re-design this part 
and avoid polling if possible.
If you try to add a "return;" before "m_timer = new QTimer(this);" in the 
contructor of VolumeControl, you can see the CPU usage lowered partially.

2. qsuianalyzer.cpp:
The same problem, a QTimer which is trigerred too frequently. The timer even 
runs unconditionally when the songs are not being played.

3. qmmp-plugin-pack/src/UI/qsui/mainwindow.cpp:
After commenting out these lines, CPU usage got lowered.
#if 0
    connect(m_volumeAction, SIGNAL(triggered(bool)), m_core, SLOT(setMuted(bool)));
    connect(m_volumeSlider, SIGNAL(valueChanged(int)), m_core, SLOT(setVolume(int)));
    connect(m_core, SIGNAL(volumeChanged(int)), m_volumeSlider, SLOT(setValue(int)));
    connect(m_core, SIGNAL(volumeChanged(int)), SLOT(updateVolumeIcon()));
    connect(m_core, SIGNAL(mutedChanged(bool)), SLOT(updateVolumeIcon()));
    connect(m_core, SIGNAL(mutedChanged(bool)), m_volumeAction, SLOT(setChecked(bool)));

    m_volumeSlider->setValue(m_core->volume());
    updateVolumeIcon();
    //visualization
    m_analyzer = new QSUiAnalyzer(this);
    m_ui.analyzerDockWidget->setWidget(m_analyzer);
    Visual::add(m_analyzer);
    //filesystem browser
    m_ui.fileSystemDockWidget->setWidget(new FileSystemBrowser(this));
    //cover
    m_ui.coverDockWidget->setWidget(new CoverWidget(this));
    //playlists
    m_ui.playlistsDockWidget->setWidget(new PlayListBrowser(m_pl_manager, this));
#endif

I guess the slots are called too frequently since the volume events and the 
analyzer all suffered from QTimer problems.

I don't have a patch for these problems at the moment, but I tried to comment 
out different parts of qmmp to see if the CPU usage is lowered. Then I spotted 
these potential causes.
Hope that this helps fix the issue. Qmmp will be perfect if the bug is fixed.
Thank you.

Original issue reported on code.google.com by pcman...@gmail.com on 21 Apr 2014 at 7:18

GoogleCodeExporter commented 9 years ago
Forgot to provide more information.
I use Ubuntu 14.04.
Besides, I'm using pulse audio, but alsa plugin is in use.
I guess this will affect the result.
If you need some more info, I'll do my best to see if I can help.
Thank you very much.

Original comment by pcman...@gmail.com on 21 Apr 2014 at 7:25

GoogleCodeExporter commented 9 years ago
In addition, I'm not using soft volume. So in theory, it's OutputALSA that's 
working, which in turns is redirected to pulse audio.
To monitor the change of volume, it might be better to utilize  
snd_mixer_poll_descriptors() to get some fd. Then the changes of the alsa mixer 
can be monitored via poll() system call and this event-driven design will be 
more efficient than continuous polling using a timer.

Original comment by pcman...@gmail.com on 21 Apr 2014 at 7:34

GoogleCodeExporter commented 9 years ago
Also, it might be related to a known bug in Qt4.
https://bugreports.qt-project.org/browse/QTBUG-7618
This seems to be related to glib integration, too.
Qt on ubuntu is compiled with glib integration turned on.

They seems to fix it for Qt 5 only.
https://codereview.qt-project.org/#change,29063

For Qt4, there is no fix at the moment.
So I'd say in Qt4 it's better to avoid these timers if possible.

Original comment by pcman...@gmail.com on 22 Apr 2014 at 4:24

GoogleCodeExporter commented 9 years ago
Some more tests:
1. The main cause of the higher CPU usage is the frequent repaint of analyzer. 
When playing the music, frequent update is inevitable so it's normal, but when 
the player is not playing anything, the analyzer is still repainted.  This can 
caused about 1-2% cpu usage on my system even when qmmp is not playing any 
music. The timer and repainting should be disabled when idle. This part needs 
some optimization.
2. The timer with 150ms interval in VolumeControl only contributes to a small 
(but still significant) fraction of cpu usage. When qmmp is idle, this 
generates 0.5% cpu usage on my system. Setting useSoftVolume to true can 
eliminate the cpu usage since frequent query of a variable is much cheaper than 
polling the status of a real audio device.
I'll try to see if I can provide any patch.

Original comment by pcman...@gmail.com on 22 Apr 2014 at 11:45

GoogleCodeExporter commented 9 years ago
OK, I made a patch for the cpu usage caused by polling the mixer every 150ms.
The patch made the Volume class derive from QObject and adds a changed() signal 
for change notification.
A new virtual function Volume::hasChangeNotify() is added. The changed() signal 
is only supported when the backend returns true in hasChangeNotify().
Then, I implemented change notification for OutputAlsa with QSocketNotifier.
I haven't touch oss and oss4 backends, but they can be changed to support 
change notification later if needed.
The final result is, when change notification is available, VolumeControl uses 
it and avoid the QTimer with 150 ms interval completely.
Please review and see if the patch can be applied.
Thanks!

Original comment by pcman...@gmail.com on 22 Apr 2014 at 2:24

Attachments:

GoogleCodeExporter commented 9 years ago
Yet another patch to decrease CPU usage when qmmp is idle.
The analyzer widget is being repainted in 25 fps even when qmmp is idle, which 
results the cpu usage.
This fixes the high cpu usage of simple UI when qmmp is not playing music.

Original comment by pcman...@gmail.com on 22 Apr 2014 at 3:11

Attachments:

GoogleCodeExporter commented 9 years ago
The previous patch for analyzer is incorrect. Here is the new one.

Original comment by pcman...@gmail.com on 22 Apr 2014 at 3:54

Attachments:

GoogleCodeExporter commented 9 years ago
A slightly improved version.

Original comment by pcman...@gmail.com on 22 Apr 2014 at 5:10

Attachments:

GoogleCodeExporter commented 9 years ago
Thank you for your patches :) I'll try to add them little later. I know about 
problem with glib event loop. I'm using QT_NO_GLIB environment variable to 
disable it. Unfortunately, cpu usage is not only one side effect of the glib 
events. Sometimes QTimer suddenly stops emitting timeout() signal. But, without 
glib event loop Qt has problem with standard gtk dialogs. So, you should use 
qmmp file dialog in this case.

Original comment by trialuser02 on 22 Apr 2014 at 6:19

GoogleCodeExporter commented 9 years ago
Since we're using LXDE-Qt, there is no gtk+ file dialog. :-)
The glib mainloop indeed is weird sometimes. Is the bug you encountered related 
to Qt bug 32895?
https://bugreports.qt-project.org/browse/QTBUG-32859
The glib integration seems to be problematic. Maybe it will be improved in Qt 5?

Original comment by pcman...@gmail.com on 23 Apr 2014 at 2:31