Merge pull request #265 from tomahawk-player/playableproxyfilter-performance

Playableproxyfilter performance

src/libtomahawk/playlist/PlayableProxyModel.cpp

@@ -145,28 +145,34 @@ PlayableProxyModel::setSourcePlayableModel( PlayableModel* sourceModel )
 bool
 PlayableProxyModel::filterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const
 {
-    bool dupeFilter = true;
+    PlayableProxyModelFilterMemo memo;
-    bool visibilityFilter = true;
-
-    if ( m_hideDupeItems )
-        dupeFilter = dupeFilterAcceptsRow( sourceRow, sourceParent );
-    if ( m_maxVisibleItems > 0 )
-        visibilityFilter = visibilityFilterAcceptsRow( sourceRow, sourceParent );
-
-    return ( dupeFilter && visibilityFilter && nameFilterAcceptsRow( sourceRow, sourceParent ) );
-}
-
-
-bool
-PlayableProxyModel::dupeFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const
-{
-    if ( !m_hideDupeItems )
-        return true;
 
     PlayableItem* pi = itemFromIndex( sourceModel()->index( sourceRow, 0, sourceParent ) );
     if ( !pi )
         return false;
 
+    return filterAcceptsRowInternal( sourceRow, pi, sourceParent, memo );
+}
+
+
+bool
+PlayableProxyModel::filterAcceptsRowInternal( int sourceRow, PlayableItem* pi, const QModelIndex& sourceParent, PlayableProxyModelFilterMemo& memo ) const
+{
+    if ( m_maxVisibleItems > 0 && !visibilityFilterAcceptsRow( sourceRow, sourceParent, memo ) )
+        return false;
+    if ( m_hideDupeItems && !dupeFilterAcceptsRow( sourceRow, pi, sourceParent, memo ) )
+        return false;
+
+    return nameFilterAcceptsRow( sourceRow, pi, sourceParent );
+}
+
+
+bool
+PlayableProxyModel::dupeFilterAcceptsRow( int sourceRow, PlayableItem* pi, const QModelIndex& sourceParent, PlayableProxyModelFilterMemo& memo ) const
+{
+    if ( !m_hideDupeItems )
+        return true;
+
     for ( int i = 0; i < sourceRow; i++ )
     {
         PlayableItem* di = itemFromIndex( sourceModel()->index( i, 0, sourceParent ) );
@@ -177,7 +183,7 @@ PlayableProxyModel::dupeFilterAcceptsRow( int sourceRow, const QModelIndex& sour
                  ( pi->album() && pi->album() == di->album() ) ||
                  ( pi->artist() && pi->artist()->name() == di->artist()->name() );
 
-        if ( b && filterAcceptsRow( i, sourceParent ) )
+        if ( b && filterAcceptsRowInternal( i, di, sourceParent, memo ) )
             return false;
     }
 
@@ -186,17 +192,26 @@ PlayableProxyModel::dupeFilterAcceptsRow( int sourceRow, const QModelIndex& sour
 
 
 bool
-PlayableProxyModel::visibilityFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const
+PlayableProxyModel::visibilityFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent, PlayableProxyModelFilterMemo& memo ) const
 {
     if ( m_maxVisibleItems <= 0 )
         return true;
 
-    int items = 0;
+    if ( static_cast<size_t>( sourceRow ) < memo.visibilty.size() )
-    for ( int i = 0; i < sourceRow; i++ )
     {
-        if ( dupeFilterAcceptsRow( i, sourceParent ) && nameFilterAcceptsRow( i, sourceParent ) )
+        // We have already memoized the return value.
+        return memo.visibilty[sourceRow] < m_maxVisibleItems;
+    }
+
+    int items = memo.visibilty.back();
+    for ( int i = memo.visibilty.size() - 1; ( i < sourceRow ) && ( items < m_maxVisibleItems ) ; i++ )
+    {
+        PlayableItem* pi = itemFromIndex( sourceModel()->index( i, 0, sourceParent ) );
+        // We will not change memo in these calls as all values needed in them are already memoized.
+        if ( pi && dupeFilterAcceptsRow( i, pi, sourceParent, memo ) && nameFilterAcceptsRow( i, pi, sourceParent ) )
         {
             items++;
+            memo.visibilty.push_back( items ); // Sets memo.visibilty[i + 1] to items
         }
     }
 
@@ -205,12 +220,8 @@ PlayableProxyModel::visibilityFilterAcceptsRow( int sourceRow, const QModelIndex
 
 
 bool
-PlayableProxyModel::nameFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const
+PlayableProxyModel::nameFilterAcceptsRow( int sourceRow, PlayableItem* pi, const QModelIndex& sourceParent ) const
 {
-    PlayableItem* pi = itemFromIndex( sourceModel()->index( sourceRow, 0, sourceParent ) );
-    if ( !pi )
-        return false;
-
     if ( m_hideEmptyParents && pi->source() )
     {
         if ( !sourceModel()->rowCount( sourceModel()->index( sourceRow, 0, sourceParent ) ) )
@@ -219,25 +230,27 @@ PlayableProxyModel::nameFilterAcceptsRow( int sourceRow, const QModelIndex& sour
         }
     }
 
-    if ( pi->query() )
+    const Tomahawk::query_ptr& query = pi->query();
+    if ( query )
     {
         Tomahawk::result_ptr r;
-        if ( pi->query()->numResults() )
+        if ( query->numResults() )
-            r = pi->query()->results().first();
+            r = query->results().first();
 
         if ( !m_showOfflineResults && ( r.isNull() || !r->isOnline() ) )
             return false;
 
-        if ( filterRegExp().isEmpty() )
+        const QRegExp regexp = filterRegExp();
+        if ( regexp.isEmpty() )
             return true;
 
-        QStringList sl = filterRegExp().pattern().split( " ", QString::SkipEmptyParts );
+        QStringList sl = regexp.pattern().split( " ", QString::SkipEmptyParts );
-        foreach( QString s, sl )
+        foreach( const QString& s, sl )
         {
-            s = s.toLower();
+            const Tomahawk::track_ptr& track = query->track();
-            if ( !pi->query()->track()->artist().toLower().contains( s ) &&
+            if ( !track->artist().contains( s, Qt::CaseInsensitive ) &&
-                 !pi->query()->track()->album().toLower().contains( s ) &&
+                 !track->album().contains( s, Qt::CaseInsensitive ) &&
-                 !pi->query()->track()->track().toLower().contains( s ) )
+                 !track->track().contains( s, Qt::CaseInsensitive ) )
             {
                 return false;
             }
@@ -252,7 +265,8 @@ PlayableProxyModel::nameFilterAcceptsRow( int sourceRow, const QModelIndex& sour
         bool found = true;
         foreach( const QString& s, sl )
         {
-            if ( !al->name().contains( s, Qt::CaseInsensitive ) && !al->artist()->name().contains( s, Qt::CaseInsensitive ) )
+            if ( !al->name().contains( s, Qt::CaseInsensitive ) &&
+                 !al->artist()->name().contains( s, Qt::CaseInsensitive ) )
             {
                 found = false;
             }
@@ -269,7 +283,8 @@ PlayableProxyModel::nameFilterAcceptsRow( int sourceRow, const QModelIndex& sour
         bool found = true;
         foreach( const QString& s, sl )
         {
-            if ( !ar->name().contains( s, Qt::CaseInsensitive ) && !ar->artist()->name().contains( s, Qt::CaseInsensitive ) )
+            if ( !ar->name().contains( s, Qt::CaseInsensitive ) &&
+                 !ar->artist()->name().contains( s, Qt::CaseInsensitive ) )
             {
                 found = false;
             }

src/libtomahawk/playlist/PlayableProxyModel.h

@@ -27,6 +27,20 @@
 
 #include "DllMacro.h"
 
+class PlayableProxyModelFilterMemo
+{
+public:
+    PlayableProxyModelFilterMemo()
+    {
+        // First element always has no predecessors.
+        // TODO C++11: Make this a constexpr using initializer lists.
+        visibilty.push_back( 0 );
+    }
+
+    virtual ~PlayableProxyModelFilterMemo() {}
+    std::vector<int> visibilty;
+};
+
 class DLLEXPORT PlayableProxyModel : public QSortFilterProxyModel
 {
 Q_OBJECT
@@ -43,7 +57,7 @@ public:
 
     virtual QString guid() const;
 
-    virtual PlayableModel* sourceModel() const { return m_model; }
+    PlayableModel* sourceModel() const { return m_model; }
     virtual void setSourcePlayableModel( PlayableModel* sourceModel );
     virtual void setSourceModel( QAbstractItemModel* model );
 
@@ -59,18 +73,18 @@ public:
     virtual void removeIndexes( const QModelIndexList& indexes );
     virtual void removeIndexes( const QList< QPersistentModelIndex >& indexes );
 
-    virtual bool showOfflineResults() const { return m_showOfflineResults; }
+    bool showOfflineResults() const { return m_showOfflineResults; }
-    virtual void setShowOfflineResults( bool b );
+    void setShowOfflineResults( bool b );
 
-    virtual bool hideDupeItems() const { return m_hideDupeItems; }
+    bool hideDupeItems() const { return m_hideDupeItems; }
-    virtual void setHideDupeItems( bool b );
+    void setHideDupeItems( bool b );
 
-    virtual int maxVisibleItems() const { return m_maxVisibleItems; }
+    int maxVisibleItems() const { return m_maxVisibleItems; }
-    virtual void setMaxVisibleItems( int items );
+    void setMaxVisibleItems( int items );
 
-    virtual PlayableItem* itemFromIndex( const QModelIndex& index ) const { return sourceModel()->itemFromIndex( index ); }
+    PlayableItem* itemFromIndex( const QModelIndex& index ) const { return sourceModel()->itemFromIndex( index ); }
-    virtual PlayableItem* itemFromQuery( const Tomahawk::query_ptr& query ) const { return sourceModel()->itemFromQuery( query ); }
+    PlayableItem* itemFromQuery( const Tomahawk::query_ptr& query ) const { return sourceModel()->itemFromQuery( query ); }
-    virtual PlayableItem* itemFromResult( const Tomahawk::result_ptr& result ) const { return sourceModel()->itemFromResult( result ); }
+    PlayableItem* itemFromResult( const Tomahawk::result_ptr& result ) const { return sourceModel()->itemFromResult( result ); }
 
     virtual Tomahawk::playlistinterface_ptr playlistInterface() const;
     void setPlaylistInterface( const Tomahawk::playlistinterface_ptr& playlistInterface );
@@ -103,7 +117,7 @@ signals:
     void selectRequest( const QPersistentModelIndex& index );
 
 protected:
-    virtual bool filterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const;
+    bool filterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const Q_DECL_OVERRIDE;
     virtual bool lessThan( const QModelIndex& left, const QModelIndex& right ) const;
 
     Tomahawk::playlistinterface_ptr m_playlistInterface;
@@ -117,9 +131,10 @@ private slots:
     void onCurrentIndexChanged( const QModelIndex& newIndex, const QModelIndex& oldIndex );
 
 private:
-    bool nameFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const;
+    bool filterAcceptsRowInternal( int sourceRow, PlayableItem* pi, const QModelIndex& sourceParent, PlayableProxyModelFilterMemo& memo ) const;
-    bool dupeFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const;
+    bool nameFilterAcceptsRow( int sourceRow, PlayableItem* pi, const QModelIndex& sourceParent ) const;
-    bool visibilityFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent ) const;
+    bool dupeFilterAcceptsRow( int sourceRow, PlayableItem* pi, const QModelIndex& sourceParent, PlayableProxyModelFilterMemo& memo ) const;
+    bool visibilityFilterAcceptsRow( int sourceRow, const QModelIndex& sourceParent, PlayableProxyModelFilterMemo& memo ) const;
     bool lessThan( int column, const Tomahawk::query_ptr& left, const Tomahawk::query_ptr& right ) const;
 
     PlayableModel* m_model;