Bug 148229 - FileTreeWidget of fileview part is slow
Summary: FileTreeWidget of fileview part is slow
Status: RESOLVED FIXED
Alias: None
Product: kdevelop
Classification: Applications
Component: general (show other bugs)
Version: 3.4.1
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-26 17:21 UTC by Ondrej Karny
Modified: 2007-07-30 10:16 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Conversion of FileTreeWidget storage from list to set (3.83 KB, patch)
2007-07-26 17:25 UTC, Ondrej Karny
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ondrej Karny 2007-07-26 17:21:48 UTC
Version:           3.4.1 (using KDE KDE 3.5.7)
Installed from:    Debian testing/unstable Packages

FileTreeWidget class (apparently responsible for holding or presenting of project files) uses unfortunate  data structures, causing massive slowdown on projects of non-trivial sizes.

This causes KDevelop startup time in the range of tens of minutes on my modest-sized project with 8000 files.

See the patch below.
Comment 1 Ondrej Karny 2007-07-26 17:25:04 UTC
Created attachment 21255 [details]
Conversion of FileTreeWidget storage from list to set

The patch changes the data structure to hold project's files from a list into a
set. The process of adding a bunch of files to the project is considerably
faster, as we do not have to process the whole list of all the files inserted
in the project previously.
Comment 2 Andreas Pakulat 2007-07-26 18:16:42 UTC
SVN commit 692940 by apaku:

Use QMap instead of QList, I didn't think of that to get a set-like storage structure.
Patch from opal@scssoft.com
BUG: 148229


 M  +9 -4      filetreewidget.cpp  
 M  +12 -2     filetreewidget.h  
 M  +1 -1      stdfiletreewidgetimpl.cpp  
 M  +1 -1      vcsfiletreewidgetimpl.cpp  


--- branches/KDE/3.5/kdevelop/parts/fileview/filetreewidget.cpp #692939:692940
@@ -225,9 +225,14 @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
-QStringList FileTreeWidget::projectFiles()
+/**
+ * @brief Test whether given file (or a directory) is part of this project.
+ *
+ * @param fileName or directory to test for presence.
+ */
+bool FileTreeWidget::isInProject(const QString &fileName) const
 {
-    return m_projectFiles;
+    return m_projectFiles.contains(fileName);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -247,10 +252,10 @@
             while( !paths.isEmpty() )
             {
                 if( !m_projectFiles.contains( paths.join("/") ) )
-                    m_projectFiles.append( projectDirectory() + "/" + paths.join("/") );
+                    m_projectFiles.insert( projectDirectory() + "/" + paths.join("/"), true );
                 paths.pop_back();
             }
-            m_projectFiles.append( file );
+            m_projectFiles.insert( file, false );
 //            kdDebug(9017) << "file added: " << file << endl;
         }
 
--- branches/KDE/3.5/kdevelop/parts/fileview/filetreewidget.h #692939:692940
@@ -55,7 +55,7 @@
     bool shouldBeShown( KFileTreeViewItem* item );
 
     QString projectDirectory();
-    QStringList projectFiles();
+    bool isInProject(const QString &fileName) const;
 
     FileViewPart *part() const { return m_part; }
 
@@ -87,7 +87,17 @@
     KDevVersionControl *versionControl() const;
 
     QStringList m_hidePatterns;
-    QStringList m_projectFiles;
+    /**
+     * @brief Set of all the files in this project.
+     *
+     * @bug
+     * Well, it is not just a plain set,
+     * but rather a map with next-to-useless element value,
+     * keyed by names of files.
+     * QT3 does not seem to have a set datatype,
+     * thus we use the QMap type instead.
+     */
+    QMap<QString, bool> m_projectFiles;
 
     FileViewPart *m_part;
     KFileTreeBranch *m_rootBranch;
--- branches/KDE/3.5/kdevelop/parts/fileview/stdfiletreewidgetimpl.cpp #692939:692940
@@ -52,7 +52,7 @@
     FileTreeWidget *lv = static_cast<StdFileTreeViewItem*>( parent )->listView();
     const KURL fileURL = fileItem->url();
 
-    bool isDirectory = lv->projectFiles().contains( fileURL.path() ) > 0;
+    const bool isDirectory = lv->isInProject( fileURL.path() );
 
     return new StdFileTreeViewItem( parent, fileItem, this, isDirectory );
 }
--- branches/KDE/3.5/kdevelop/parts/fileview/vcsfiletreewidgetimpl.cpp #692939:692940
@@ -146,7 +146,7 @@
 
     FileTreeWidget *lv = static_cast<filetreeview::FileTreeViewItem*>( parent )->listView();
     const KURL fileURL = fileItem->url();
-    bool isDirectory = lv->projectFiles().contains( fileURL.path() ) > 0;
+    const bool isDirectory = lv->isInProject( fileURL.path() );
 
     VCSFileTreeViewItem *newItem = new VCSFileTreeViewItem( parent, fileItem, this, isDirectory );
 
Comment 3 Andreas Pakulat 2007-07-27 17:36:29 UTC
SVN commit 693236 by apaku:

Revert the change from list to map as it breaks the automatic addition of newly
created files to the filetree. I don't know why that breaks (as it should be
handled by KFileTreeView from kdelibs), but I don't have the time to
investigate.

Instead I changed all contains() calls to findIndex, contains always iterates
the whole list, because it does actually a count(), findIndex should speed
things up considerably.

If it doesn't speed up enough, please re-open the bugreport and provide a patch
that doesn't break the automatic addition of new files to the tree

CCBUG: 148229


 M  +6 -11     filetreewidget.cpp  
 M  +2 -12     filetreewidget.h  
 M  +1 -1      stdfiletreewidgetimpl.cpp  
 M  +1 -1      vcsfiletreewidgetimpl.cpp  


--- branches/KDE/3.5/kdevelop/parts/fileview/filetreewidget.cpp #693235:693236
@@ -226,14 +226,9 @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
-/**
- * @brief Test whether given file (or a directory) is part of this project.
- *
- * @param fileName or directory to test for presence.
- */
-bool FileTreeWidget::isInProject(const QString &fileName) const
+QStringList FileTreeWidget::projectFiles()
 {
-    return m_projectFiles.contains(fileName);
+    return m_projectFiles;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -249,17 +244,17 @@
             continue;
         kdDebug(9017) << "adding file: " << *it << endl;
         QString file = projectDirectory() + "/" + ( *it );
-        if ( !m_projectFiles.contains( file ) )
+        if ( m_projectFiles.findIndex( file ) == -1 )
         {
             QStringList paths = QStringList::split( "/", *it);
             paths.pop_back();
             while( !paths.isEmpty() )
             {
-                if( !m_projectFiles.contains( paths.join("/") ) )
-                    m_projectFiles.insert( projectDirectory() + "/" + paths.join("/"), true );
+                if( m_projectFiles.findIndex( paths.join("/") ) == -1 )
+                    m_projectFiles.append( projectDirectory() + "/" + paths.join("/") );
                 paths.pop_back();
             }
-            m_projectFiles.insert( file, false );
+            m_projectFiles.append( file );
 //            kdDebug(9017) << "file added: " << file << endl;
         }
 
--- branches/KDE/3.5/kdevelop/parts/fileview/filetreewidget.h #693235:693236
@@ -55,7 +55,7 @@
     bool shouldBeShown( KFileTreeViewItem* item );
 
     QString projectDirectory();
-    bool isInProject(const QString &fileName) const;
+    QStringList projectFiles();
 
     FileViewPart *part() const { return m_part; }
 
@@ -87,17 +87,7 @@
     KDevVersionControl *versionControl() const;
 
     QStringList m_hidePatterns;
-    /**
-     * @brief Set of all the files in this project.
-     *
-     * @bug
-     * Well, it is not just a plain set,
-     * but rather a map with next-to-useless element value,
-     * keyed by names of files.
-     * QT3 does not seem to have a set datatype,
-     * thus we use the QMap type instead.
-     */
-    QMap<QString, bool> m_projectFiles;
+    QStringList m_projectFiles;
 
     FileViewPart *m_part;
     KFileTreeBranch *m_rootBranch;
--- branches/KDE/3.5/kdevelop/parts/fileview/stdfiletreewidgetimpl.cpp #693235:693236
@@ -52,7 +52,7 @@
     FileTreeWidget *lv = static_cast<StdFileTreeViewItem*>( parent )->listView();
     const KURL fileURL = fileItem->url();
 
-    const bool isDirectory = lv->isInProject( fileURL.path() );
+    bool isDirectory = lv->projectFiles().contains( fileURL.path() ) > 0;
 
     return new StdFileTreeViewItem( parent, fileItem, this, isDirectory );
 }
--- branches/KDE/3.5/kdevelop/parts/fileview/vcsfiletreewidgetimpl.cpp #693235:693236
@@ -146,7 +146,7 @@
 
     FileTreeWidget *lv = static_cast<filetreeview::FileTreeViewItem*>( parent )->listView();
     const KURL fileURL = fileItem->url();
-    const bool isDirectory = lv->isInProject( fileURL.path() );
+    bool isDirectory = lv->projectFiles().findIndex( fileURL.path() ) != -1;
 
     VCSFileTreeViewItem *newItem = new VCSFileTreeViewItem( parent, fileItem, this, isDirectory );
 
Comment 4 dukju ahn 2007-07-29 21:05:29 UTC
>> Revert the change from list to map as it breaks the 
>> automatic addition of newly created files to the filetree.

Is it true? I compiled up to revision 693226 (which uses QMap) and
it correctly updated any added/deleted files.
Can you check it again? Or did you use "Hide non project files" options?
Comment 5 Andreas Pakulat 2007-07-30 10:16:34 UTC
SVN commit 694176 by apaku:

Re-Apply the patch from 148229, it turns out that the problem I saw here was
only related to me not waiting long enough. It seems there's a timer that
starts the dirwatching in the filetree. After waiting for a bit all touche'd
files show up and new ones show up instantly.
CCBUG:148229


 M  +11 -6     filetreewidget.cpp  
 M  +12 -2     filetreewidget.h  
 M  +1 -1      stdfiletreewidgetimpl.cpp  
 M  +1 -1      vcsfiletreewidgetimpl.cpp  


--- branches/KDE/3.5/kdevelop/parts/fileview/filetreewidget.cpp #694175:694176
@@ -226,9 +226,14 @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
-QStringList FileTreeWidget::projectFiles()
+/**
+ * @brief Test whether given file (or a directory) is part of this project.
+ *
+ * @param fileName or directory to test for presence.
+ */
+bool FileTreeWidget::isInProject(const QString &fileName) const
 {
-    return m_projectFiles;
+    return m_projectFiles.contains(fileName);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -244,17 +249,17 @@
             continue;
         kdDebug(9017) << "adding file: " << *it << endl;
         QString file = projectDirectory() + "/" + ( *it );
-        if ( m_projectFiles.findIndex( file ) == -1 )
+        if ( !m_projectFiles.contains( file ) )
         {
             QStringList paths = QStringList::split( "/", *it);
             paths.pop_back();
             while( !paths.isEmpty() )
             {
-                if( m_projectFiles.findIndex( paths.join("/") ) == -1 )
-                    m_projectFiles.append( projectDirectory() + "/" + paths.join("/") );
+                if( !m_projectFiles.contains( paths.join("/") ) )
+                    m_projectFiles.insert( projectDirectory() + "/" + paths.join("/"), true );
                 paths.pop_back();
             }
-            m_projectFiles.append( file );
+            m_projectFiles.insert( file, false );
 //            kdDebug(9017) << "file added: " << file << endl;
         }
 
--- branches/KDE/3.5/kdevelop/parts/fileview/filetreewidget.h #694175:694176
@@ -55,7 +55,7 @@
     bool shouldBeShown( KFileTreeViewItem* item );
 
     QString projectDirectory();
-    QStringList projectFiles();
+    bool isInProject(const QString &fileName) const;
 
     FileViewPart *part() const { return m_part; }
 
@@ -87,7 +87,17 @@
     KDevVersionControl *versionControl() const;
 
     QStringList m_hidePatterns;
-    QStringList m_projectFiles;
+    /**
+     * @brief Set of all the files in this project.
+     *
+     * @bug
+     * Well, it is not just a plain set,
+     * but rather a map with next-to-useless element value,
+     * keyed by names of files.
+     * QT3 does not seem to have a set datatype,
+     * thus we use the QMap type instead.
+     */
+    QMap<QString, bool> m_projectFiles;
 
     FileViewPart *m_part;
     KFileTreeBranch *m_rootBranch;
--- branches/KDE/3.5/kdevelop/parts/fileview/stdfiletreewidgetimpl.cpp #694175:694176
@@ -52,7 +52,7 @@
     FileTreeWidget *lv = static_cast<StdFileTreeViewItem*>( parent )->listView();
     const KURL fileURL = fileItem->url();
 
-    bool isDirectory = lv->projectFiles().contains( fileURL.path() ) > 0;
+    const bool isDirectory = lv->isInProject( fileURL.path() );
 
     return new StdFileTreeViewItem( parent, fileItem, this, isDirectory );
 }
--- branches/KDE/3.5/kdevelop/parts/fileview/vcsfiletreewidgetimpl.cpp #694175:694176
@@ -146,7 +146,7 @@
 
     FileTreeWidget *lv = static_cast<filetreeview::FileTreeViewItem*>( parent )->listView();
     const KURL fileURL = fileItem->url();
-    bool isDirectory = lv->projectFiles().findIndex( fileURL.path() ) != -1;
+    const bool isDirectory = lv->isInProject( fileURL.path() );
 
     VCSFileTreeViewItem *newItem = new VCSFileTreeViewItem( parent, fileItem, this, isDirectory );