GNOME Bugzilla – Bug 711459
Only enumerate calls trash_watcher_rescan
Last modified: 2016-03-31 14:37:09 UTC
Because only enumerate() calls trash_watcher_rescan(), any operation invoked before enumerate() is first called on a file in the trash backend fails. Eg: $ pkill gvfs $ gvfs-cat trash:///afile.txt gvfs-cat: trash:///afile.txt: error opening file: No such file or directory $ gvfs-info trash:/// display name: Trash name: / type: directory uri: trash:/// attributes: standard::type: 2 standard::name: / standard::display-name: Trash standard::icon: user-trash standard::content-type: inode/directory standard::symbolic-icon: user-trash-symbolic id::filesystem: trash: trash::item-count: 0 <--- note $ gvfs-ls trash:/// afile.txt $ gvfs-cat trash:///afile.txt some text $ gvfs-info trash:/// display name: Trash name: / type: directory uri: trash:/// attributes: standard::type: 2 standard::name: / standard::display-name: Trash standard::icon: user-trash standard::content-type: inode/directory standard::symbolic-icon: user-trash-symbolic id::filesystem: trash: trash::item-count: 1
Created attachment 323964 [details] [review] trash: Rescan trash dirs before operations with files Patches to use G_FILE_ATTRIBUTE_TRASH_ITEM_COUNT were recently proposed for Nautilus and GTK+. We have to be sure that the item count is valid and therefore we have to rescan the trash dirs before. Otherwise the count is 0 if enumeration isn't called before, which is obviously wrong. Similarly, trash dirs have to be rescanned before other operations (i.e. read, delete, pull), otherwise the operations may fail with G_IO_ERROR_NOT_FOUND if enumeration wasn't called before.
Created attachment 323965 [details] [review] trash: Check modification time to avoid rereading Rereading of trash dirs is pretty expensive and unfortunately it is performed more often with recent changes. Check modification time of each trash dir in order to avoid needless rereading, because simple query_info call is much faster than enumeration.
This is not probably the best way how to test performance, but it shows some important values: Query info for 5000 files in the trash without any additional patches (dir monitoring is enabled): $ time gvfs-info $(gvfs-ls -c trash:///) > /dev/null real 0m4.285s user 0m2.206s sys 0m0.284s Query info for 5000 files in the trash without any additional patches (dir monitoring is not enabled): $ time gvfs-info $(gvfs-ls -c trash:///) > /dev/null real 0m4.383s user 0m2.240s sys 0m0.260s Query info for 5000 files in the trash with the first patch only (dir monitoring is enabled): $ time gvfs-info $(gvfs-ls -c trash:///) > /dev/null real 0m5.521s user 0m2.339s sys 0m0.265s Query info for 5000 files in the trash with the first patch only (dir monitoring is not enabled): $ time gvfs-info $(gvfs-ls -c trash:///) > /dev/null real 4m24.525s user 0m3.157s sys 0m0.352s Query info for 5000 files in the trash with the both patches (dir monitoring is enabled): $ time gvfs-info $(gvfs-ls -c trash:///) > /dev/null real 0m5.400s user 0m2.303s sys 0m0.262s Query info for 5000 files in the trash with the both patches (dir monitoring is not enabled): $ time gvfs-info $(gvfs-ls -c trash:///) > /dev/null real 0m5.572s user 0m2.332s sys 0m0.256s You can see a significant slowdown with first patch only, when dir monitoring is not enabled. It is caused by additional rescan of trash directories. You can also see that this slowdown is almost reduced by the second patch which avoids rereading if modification time is not changed...
Ryan, can you please take a look at the patches?
(In reply to Ondrej Holy from comment #1) > Created attachment 323964 [details] [review] [review] > trash: Rescan trash dirs before operations with files > > Patches to use G_FILE_ATTRIBUTE_TRASH_ITEM_COUNT were recently proposed for > Nautilus and GTK+. We have to be sure that the item count is valid and > therefore we have to rescan the trash dirs before. Another solution might be to call enumeration once before trash monitor is established (e.g. from Nautilus and GTK+), but this patch fixes more problems than just wrong item-count...
Review of attachment 323964 [details] [review]: Do we really need all the ops here? I think my original reasoning was that one cannot know the name of a file in the trash without first having enumerated it... I'm definitely okay with adding this for query-info on the root object if we have not yet established a watch...
You are right that nobody can't know the name of file without enumeration, but e.g. some applications might want to reopen files from last time and it may fail in such case... maybe recent files should not include files from trash... but still we can't be sure that some application doesn't remember the files itself...
Created attachment 325089 [details] [review] trash: Rescan trash dirs before operations with files I improved the patch to call trash_watcher_rescan only if watch is not established yet as suggested in Comment 6. However it is called from all the operations as before due to reasons mentioned in Comment 7.
Pushed as commit be29e48 and commit 7d0ef03 since the gtk+ patch is already pushed...