Review Request: Project model speedups for project files with lots of directories and files.

[Andreas Pakulat]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3722/#review5151
-----------------------------------------------------------


re 1): Makes sense, though of course means that we need to take extra care that the hash is always properly up-to-date.

re 2): Is the KUrl() copy constructor that slow? It shouldn't be as its an implicitly shared class, i.e. just a ref-count increase. I'd like to avoid returning const&'s in public API as explained here: http://techbase.kde.org/Policies/Library_Code_Policy#Const_References. I'm not religous on this though, so if it turns out to be a major part of the costs then its ok.

re 3): that one is fine.

re qstandarditemmodel insertion: I hope to rewrite the projectmodel completely one day as a QAbstractItemModel so we have full control over the implementation and can tweak it as needed speedwise.

re mutex contention: IndexedString is AFAIK not replaceable with QString as its much more memory-conserving and conversion between QString and IndexedString is much too slow for language support. And language support is one of the users of the fileset and its using it a lot. So that is probably not an option (though David would know for sure).

- Andreas


On 2010-04-21 16:34:36, Brandon Ehle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3722/
> -----------------------------------------------------------
> 
> (Updated 2010-04-21 16:34:36)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> My test case consists of 3 projects (Linux kernel main tree, DRM tree, and Mesa tree).
> 
> 
> Changes:
> 
> 1) Replace recursion inside itemsForUrl with a QMultiHash lookup to speedup itemsForUrl() on large projects.
> 
> 2) Change the url() function for the ProjectBaseItem to return a constant reference instead of a copy to reduce the time spent inside the KUrl() constructor.
> 
> 3) Replace dynamic_cast with virtual function based upcasting inside filesForUrl() and foldersForUrl().
> 
> 
> Remaining problems:
> 
> 1) QStandardItemModel insertion is still extremely slow, but fixing that is a much larger change.
> 
> 2) There is a lot of mutex contention showing up in oprofile measumerements on a 4 core machine.  I believe this is coming from the IndexedString() constructor, but need to verify yet.  If that is the case the IndexedString() constructor either needs to be optimized or addFileSet() should switch away from IndexedString to QString or similar. 
> 
> 
> Also while creating this patch, I ran into the problem that the existing d->fileSet as used by removeFileSet() could potentially have issues when the same path is added more than once.
> 
> 
> This addresses bug 215968.
>     https://bugs.kde.org/show_bug.cgi?id=215968
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/sdk/kdevplatform/interfaces/iproject.h 1116332 
>   /trunk/extragear/sdk/kdevplatform/project/projectmodel.h 1116332 
>   /trunk/extragear/sdk/kdevplatform/project/projectmodel.cpp 1116332 
>   /trunk/extragear/sdk/kdevplatform/shell/project.h 1116332 
>   /trunk/extragear/sdk/kdevplatform/shell/project.cpp 1116332 
>   /trunk/extragear/sdk/kdevplatform/tests/testproject.h 1116332 
> 
> Diff: http://reviewboard.kde.org/r/3722/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Brandon
> 
>