Bug 316242 - Removing subset of a large playlist results in Amarok freeze, 100% CPU usage (on one core)
Summary: Removing subset of a large playlist results in Amarok freeze, 100% CPU usage ...
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Playlist (show other bugs)
Version: 2.7-git
Platform: Ubuntu Linux
: NOR major
Target Milestone: 2.8
Assignee: Amarok Developers
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2013-03-06 19:06 UTC by Patrick Callahan
Modified: 2013-05-24 09:18 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 2.8


Attachments
screenshot of amarok debug output during playlist track removal/freeze (353.78 KB, image/png)
2013-03-06 20:04 UTC, Patrick Callahan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Callahan 2013-03-06 19:06:42 UTC
Amarok, incredibly, can handle adding my entire collection (tens of thousands of songs) into a playlist in about 1.5 seconds. It can also handle removing everything from the playlist very quickly, so long as it's removed at once.

However, removing only a portion of the playlist is much slower. There's a delay in even removing a single track from a 30,000+ size playlist. Removing more, (e.g., all of the tracks but one) basically results in a freeze.

Reproducible: Always

Steps to Reproduce:
1. Add many (10,000, let's say) tracks to the playlist.
2. Remove all of them but one.
Actual Results:  
CPU usage shoots up to 100% on one core, Amarok becomes unresponsive.

Expected Results:  
Amarok removes the tracks from the playlist in about the same amount of time it took to add them.

I originally thought this had something to do with writing to the current.xspf file when tracks are removed, maybe waiting on disk access or something. It would appear now that this is not the case, as when Amarok has become unresponsive, the file isn't being written and Amarok doesn't present as writing to disk in ksysguard.
Comment 1 Patrick Callahan 2013-03-06 20:04:12 UTC
Created attachment 77812 [details]
screenshot of amarok debug output during playlist track removal/freeze

It gives nothing but messages exactly like the following pair:
‘amarok:       BEGIN: void PlaylistInfoWidget::updateTotalPlaylistLength() 
amarok:       END__: void PlaylistInfoWidget::updateTotalPlaylistLength() [Took: 0.012s]’
Comment 2 Myriam Schweingruber 2013-03-11 16:39:31 UTC
Confirmed, the same happens when triggering bug 316551 and then trying to remove sections of the added podcasts. This is the Konsole output when doing it:


amarok: BEGIN: void Playlist::Controller::removeRows(QList<int>&) 
amarok:   BEGIN: virtual void Playlist::RemoveTracksCmd::redo() 
amarok:     BEGIN: void Playlist::Model::removeTracksCommand(const RemoveCmdList&) 
amarok:       MPRIS2: Queueing up a PropertiesChanged signal 
amarok:     END__: void Playlist::Model::removeTracksCommand(const RemoveCmdList&) [Took: 0.87s] 
amarok:   END__: virtual void Playlist::RemoveTracksCmd::redo() [Took: 0.87s] 
amarok: END__: void Playlist::Controller::removeRows(QList<int>&) [Took: 0.88s] 
amarok: BEGIN: void DBusAbstractAdaptor::_m_emitPropertiesChanged() 
amarok: END__: void DBusAbstractAdaptor::_m_emitPropertiesChanged() [Took: 0s] 
amarok: BEGIN: void ScanManager::checkForDirectoryChanges() 
amarok:   BEGIN: void DirWatchJob::setPaused(bool) 
amarok:   END__: void DirWatchJob::setPaused(bool) [Took: 0s] 
amarok: END__: void ScanManager::checkForDirectoryChanges() [Took: 0s] 
amarok: BEGIN: void Playlist::Controller::removeRows(QList<int>&) 
amarok:   BEGIN: virtual void Playlist::RemoveTracksCmd::redo() 
amarok:     BEGIN: void Playlist::Model::removeTracksCommand(const RemoveCmdList&) 
amarok:       MPRIS2: Queueing up a PropertiesChanged signal 
amarok:     END__: void Playlist::Model::removeTracksCommand(const RemoveCmdList&) [Took: 0.38s] 
amarok:   END__: virtual void Playlist::RemoveTracksCmd::redo() [Took: 0.38s] 
amarok: END__: void Playlist::Controller::removeRows(QList<int>&) [Took: 0.38s] 
amarok: BEGIN: void DBusAbstractAdaptor::_m_emitPropertiesChanged() 
amarok: END__: void DBusAbstractAdaptor::_m_emitPropertiesChanged() [Took: 0s] 
amarok: BEGIN: void Playlist::Controller::removeRows(QList<int>&) 
amarok:   BEGIN: virtual void Playlist::RemoveTracksCmd::redo() 
amarok:     BEGIN: void Playlist::Model::removeTracksCommand(const RemoveCmdList&) 
amarok:       MPRIS2: Queueing up a PropertiesChanged signal 
amarok:     END__: void Playlist::Model::removeTracksCommand(const RemoveCmdList&) [Took: 4.7s] 
amarok:   END__: virtual void Playlist::RemoveTracksCmd::redo() [Took: 4.7s] 
amarok: END__: void Playlist::Controller::removeRows(QList<int>&) [Took: 4.7s] 
amarok: BEGIN: void DBusAbstractAdaptor::_m_emitPropertiesChanged() 
amarok: END__: void DBusAbstractAdaptor::_m_emitPropertiesChanged() [Took: 0s]
Comment 3 Matěj Laitl 2013-05-24 09:18:06 UTC
Git commit 00ea1116c554aeb8505554085b35ddb3d0abffce by Matěj Laitl.
Committed on 23/05/2013 at 18:41.
Pushed by laitl into branch 'master'.

Optimize and simplify Playlist::Model::removeTracksCommand()

We don't need to keep track of multiple command lists when removing,
just sort rows to remove first and then keep track of how many rows
have been already removed and subtract it at appropriate places.

The optimization is finding consecutive runs of rows to remove and then
grouping begin/endRemoveRows() for them, which was the main CPU hog as
it updates above models and view.

Removing 24.658 tracks from 24.660-track-long playlist now takes about
half a second, which is hopefully acceptable.

Also note that even greater optimization was done by Ralf Engels
earlier after 2.7, commit 861143c02dc1a1, where he cleverly used
references where appropriate, and much more.

@Patrick, please retest with Amarok git master, it should be much
better.
FIXED-IN: 2.8
CCMAIL: Ralf Engels <ralf-engels@gmx.de>
DIGEST: Optimization: removing tens of thousands of tracks from Amarok
playlist is now much faster.

M  +1    -0    ChangeLog
M  +70   -81   src/playlist/PlaylistModel.cpp

http://commits.kde.org/amarok/00ea1116c554aeb8505554085b35ddb3d0abffce