[PATCH 2/3] Playlist performance: finally start using the infrastructure we've built up.

Nanno Langstraat langstr at gmail.com
Mon Feb 1 00:44:14 CET 2010


This commit is part of the fix for Bug 210345. (slow "play next track" when the
playlist is large)

Each 'play next track' event causes a 'metadataChanged( track )' call,
because the "# of times played" metadata of the old track is incremented.

The 'metadataChanged( track )' function did a O(n) scan of the whole playlist.

Now changed to 2 O(log n) lookups:
  TrackPtr -> Item*
  Item* -> row number

As a side effect, the *ForTrack() functions are now also quicker.
---
 src/playlist/PlaylistModel.cpp |   62 ++++++++++++++++++++++------------------
 src/playlist/PlaylistModel.h   |    5 ++-
 2 files changed, 38 insertions(+), 29 deletions(-)

diff --git a/src/playlist/PlaylistModel.cpp b/src/playlist/PlaylistModel.cpp
index c000295..9652b58 100644
--- a/src/playlist/PlaylistModel.cpp
+++ b/src/playlist/PlaylistModel.cpp
@@ -557,25 +557,18 @@ Playlist::Model::stateOfRow( int row ) const
 bool
 Playlist::Model::containsTrack( const Meta::TrackPtr track ) const
 {
-    foreach( Item* i, m_items )
-    {
-        if ( i->track() == track )
-            return true;
-    }
-    return false;
+    return m_TrackToItems.contains( track );
 }
 
 int
 Playlist::Model::firstRowForTrack( const Meta::TrackPtr track ) const
 {
-    int row = 0;
-    foreach( Item* i, m_items )
-    {
-        if ( i->track() == track )
-            return row;
-        row++;
-    }
-    return -1;
+    // QMultiHash::value() will pick a random matching playlist item.
+    Item* item = m_TrackToItems.value( track, NULL );
+    if ( item )
+        return rowForItem( item );
+    else
+        return -1;
 }
 
 QSet<int>
@@ -583,13 +576,14 @@ Playlist::Model::allRowsForTrack( const Meta::TrackPtr track ) const
 {
     QSet<int> trackRows;
 
-    int row = 0;
-    foreach( Item* i, m_items )
+    QMultiHash<Meta::TrackPtr, Item*>::const_iterator item_iterator = m_TrackToItems.find( track );
+    while ((item_iterator != m_TrackToItems.end()) && (item_iterator.key() == track))
     {
-        if ( i->track() == track )
-            trackRows.insert( row );
-        row++;
+        Item* item = item_iterator.value();
+        trackRows.insert( rowForItem( item ) );
+        ++item_iterator;
     }
+
     return trackRows;
 }
 
@@ -651,19 +645,31 @@ Playlist::Model::stateOfId( quint64 id ) const
 }
 
 void
+Playlist::Model::metadataChanged( Item* item )
+{
+    DEBUG_BLOCK
+
+    int row = rowForItem( item );
+    emit dataChanged( index( row, 0 ), index( row, columnCount() - 1 ) );
+
+    emit metadataUpdated();
+
+    debug()<<"Metadata updated for playlist item"<<item->id()<<"at row"<<row;
+}
+
+void
 Playlist::Model::metadataChanged( Meta::TrackPtr track )
 {
-    const int size = m_items.size();
-    for ( int i = 0; i < size; i++ )
+    QMultiHash<Meta::TrackPtr, Item*>::const_iterator item_iterator = m_TrackToItems.find( track );
+    while ((item_iterator != m_TrackToItems.end()) && (item_iterator.key() == track))
     {
-        if ( m_items.at( i )->track() == track )
-        {
-            emit dataChanged( createIndex( i, 0 ), createIndex( i, columnCount() - 1 ) );
-            emit metadataUpdated();
-            debug()<<"Metadata updated for track"<<track->prettyName();
-            break;
-        }
+        Item* item = item_iterator.value();
+        metadataChanged( item );
+
+        ++item_iterator;
     }
+
+    debug()<<"Metadata updated for track"<<track->prettyName();
 }
 
 void
diff --git a/src/playlist/PlaylistModel.h b/src/playlist/PlaylistModel.h
index d919d41..a0912a2 100644
--- a/src/playlist/PlaylistModel.h
+++ b/src/playlist/PlaylistModel.h
@@ -165,8 +165,11 @@ class AMAROK_EXPORT Model : public QAbstractListModel, public Meta::Observer, pu
         void clearCommand();
         void setStateOfRow( int row, Item::State state ) { m_items.at( row )->setState( state ); }
 
-        int rowForItem( Item* & item ) { return m_items.indexOf( item ); }
+        // miscellaneous
+        void metadataChanged( Item* item );
+        int rowForItem( Item* item ) const { return m_items.indexOf( item ); }
 
+        // variables
         PlaylistItems m_items; //! list of playlist items.
             // 'm_items' keeps the "manual" order; sorting happens in the proxy models.
         QHash<quint64, Item*> m_itemIds; //! maps track id's to items
-- 
1.7.0.4


--------------050205060600030001000806
Content-Type: text/x-patch;
 name="0003-Rename-firstRowForTrack-to-anyRowForTrack-to-emphasi.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0003-Rename-firstRowForTrack-to-anyRowForTrack-to-emphasi.pa";
 filename*1="tch"



More information about the Amarok-devel mailing list