meven added a comment. Restricted Application added a subscriber: kde-frameworks-devel.
In D10702#242143 <https://phabricator.kde.org/D10702#242143>, @dfaure wrote: > s/thread/process/. It's not about threads it's about two processes: the (GUI) application, and the kioslave. > > But you present all this in a rather convoluted way, it's much simpler. The SimpleJob for asking a kioslave to delete a file is KIO::file_delete which exactly what this patch ends up calling. > > The issue is choosing when to call unlink directly and when to ask the slave to do it. Or indeed markg's idea of sending the full list of files to the slave in one go, with a new MultiDeleteJob (including progress information). However for many small files this might be slower (that long list needs to be sent over...). > > Since the bug report is about the "one big file" case, I'd say let's not fix what ain't broken (the many small files case), and let's just do "if local and small, delete in-process, otherwise use kioslave", i.e. just adding a size check for the fast path. > > The optimized way to do it would be, rather than a stat() in this method, to change DeleteJobPrivate::slotEntries so that it puts big files into a separate list, for instance. I have tried it and well DeleteJobPrivate::slotEntries is not sufficient, this function is used only when a directory is removed and not when files are removed directly. So this would require to add more complexity and an added stat call to the chain of calls. I plan to do some benchmarking to evaluate the impact of the current patch. I still feel the current patch is good because it is consistent with other Jobs (copyJob for instance) and keeps the code clean. So before trying adding some special cases adding more complexity to preserve marginaly performance in corner-use cases (load of files). As a side note all other jobs have the same issue regarding load of calls to kio but are not blocking the calling thread (except the rename job apparently). To me the bug looks more like a usability concern : is kio asynchrounous and usable for UIs or not ? I'd rather study the current patch. I am working on a benchmark script, with and without the current patch using kioclient contained in kde-cli-tools. I will share the script and results, of course. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10702 To: meven, #frameworks, dfaure, ngraham, #dolphin, jtamate Cc: kde-frameworks-devel, jtamate, markg, ngraham, #frameworks, michaelh, bruns
