Review Request 123654: Improve performance of KFileItem and KDirSortFilterProxyModel

[David Faure]

> On May 10, 2015, 10:43 p.m., David Faure wrote:
> > src/core/kfileitem.cpp, line 1115
> > <https://git.reviewboard.kde.org/r/123654/diff/1/?file=366630#file366630line1115>
> >
> >     I had the same thought as Milian... but I wonder why I wrote that comment. m_strText is the displayName, m_strName is not. Maybe I confused the two when I wrote it. That's my best guess right now.
> >     
> >     Mark: the first commit is about the current directory ("."), the second commit is about urls without a filename, how does it make the first one obsolete? I don't see the relation.
> 
> Mark Gaiser wrote:
>     Well, i could be wrong (likely ;) if so, please correct me) but the return statement is checking if m_strName is > 1 in length and if that string starts with a dot. By having the second commit the first one (the if statement) just doesn't seem to be needed anymore. Which is probably also what Aleix thought and removed the if.

The first commit uses m_strName if the URL has no filename. You see two checks on "is fileName empty", but it's not the same string that we're checking, given the assignment fileName = m_strName between the two if()s.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123654/#review80189
-----------------------------------------------------------


On May 6, 2015, 2:35 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123654/
> -----------------------------------------------------------
> 
> (Updated May 6, 2015, 2:35 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Reduces some operations in KDirSortFilterProxyModel.
> Removes usage of QUrl::fileName() in KFileItem::isHidden(), it comprised 75% of the time spent running the benchmark in here.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 010074d 
>   autotests/kdirsortfilterproxymodel_benchmark.cpp PRE-CREATION 
>   src/core/kfileitem.cpp 344ac67 
>   src/filewidgets/kdirsortfilterproxymodel.cpp 22ac025 
> 
> Diff: https://git.reviewboard.kde.org/r/123654/diff/
> 
> 
> Testing
> -------
> 
> Everything seems to be working still, including tests.
> 
> There was a comment about trash:/ triggering an assert, but I couldn't reproduce.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150511/f6b1db1a/attachment-0001.html>