nicolasfella marked an inline comment as done. nicolasfella added inline comments.
INLINE COMMENTS > dfaure wrote in krecentfilesmenu.cpp:150 > Depending on QtConcurrent is just fine. However QtConcurrent::filtered is for > CPU-intensive operations, not for I/O operations. 1) you don't want to put > blocking runnables in the global thread pool [can be solved by assigning to a > different threadpool], 2) I don't think you really want to parallelize 16 > calls to QFile::exists, for the case of a local physical harddisk? Not sure > it would really harm (we're not reading 16 files, at least), but for sure the > overhead of dispatching runnables to 16 threads would make it slower... One > solution here might be just a dedicated thread iterating over the list. > > However: note that the usual KIO way to do file operations async isn't > multithreading, it's rather KIO jobs. > This would move the "5 minutes waiting for an NFS server" horror case into a > kioslave rather than a thread, both achieve the desired outcome which is to > not block the GUI thread. > > Keeping the order of the entries is going to be interesting. I guess we need > a temporary data structure which stores "exists | does not exist" for each > entry, and once all jobs are done, we go through that and fill the menu. Note > that remote URLs should just be assumed to exist, we don't want to actually > check (slow; might prompt for password; might error on different networks; > etc.). > > Unlike Kai-Uwe, I think this should be a separate merge request though, it's > all quite orthogonal to your work. As long as you confirm that filling the > menu "later" has no negative impact on the API (i.e. the user of the class > cannot assume that the menu is filled in immediately). > As long as you confirm that filling the menu "later" has no negative impact > on the API (i.e. the user of the class cannot assume that the menu is filled > in immediately). Should be fine REVISION DETAIL https://phabricator.kde.org/D26448 To: nicolasfella, #frameworks, dfaure Cc: broulik, elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
