GNOME Bugzilla – Bug 642814
Big Memory use
Last modified: 2014-01-04 04:43:38 UTC
gvfsd-gphoto2 consume too much memory to copy data from digital camera : (when select copy) For exemple, if a video take 1.5Go on a file on my camera, gvfsd-gphoto2 will use 1.5Go off memory ... Version used : gvfs-1.6.6-1.fc14.x86_64 gvfs-gphoto2-1.6.6-1.fc14.x86_64 nautilus-2.32.2.1-2.fc14.x86_64 cat /proc/PID/smaps : ... ... 7f40d8e0a000-7f4123450000 rw-p 00000000 00:00 0 Size: 1218840 kB Rss: 1218840 kB Pss: 1218840 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 1218840 kB Referenced: 1172844 kB Swap: 0 kB KernelPageSize: 4 kB MMUPageSize: 4 kB 7f4148000000-7f414bffe000 rw-p 00000000 00:00 0 Size: 65528 kB Rss: 8612 kB Pss: 8612 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 8612 kB Referenced: 264 kB Swap: 0 kB KernelPageSize: 4 kB MMUPageSize: 4 kB ... ... Tested another time with another video : PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 4193 roumano 20 0 1543m 1.3g 2268 R 14.6 68.7 0:21.02 gvfsd-gphoto2 cat /proc/4193/smaps ... ... 7f6b3dc09000-7f6b8f400000 rw-p 00000000 00:00 0 Size: 1335260 kB Rss: 1264988 kB Pss: 1264988 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 15164 kB Private_Dirty: 1249824 kB Referenced: 1252320 kB Swap: 70272 kB KernelPageSize: 4 kB MMUPageSize: 4 kB 7f6bb8000000-7f6bb9550000 rw-p 00000000 00:00 0 Size: 21824 kB Rss: 7616 kB Pss: 7616 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 8 kB Private_Dirty: 7608 kB Referenced: 468 kB Swap: 1968 kB KernelPageSize: 4 kB MMUPageSize: 4 kB 7f6bb9550000-7f6bbc000000 ---p 00000000 00:00 0 Size: 43712 kB Rss: 0 kB Pss: 0 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 0 kB Referenced: 0 kB Swap: 0 kB KernelPageSize: 4 kB MMUPageSize: 4 kB ... ... Can send full smaps trace Tested to download data with gphoto2 command line (gphoto2 --get-all-files) : no problem ...
I can confirm this issue on Ubuntu 12.10 and all older versions. It is in fact not even necessary to copy (or paste to be specific) a large file from a camera somewhere, the huge memory load starts immediately after entering the folder with such big (video) file while generating thumbnail preview (even if the file size of video files for generating thumbnails is limited to e. g. 100MB) and the increasing memory usage doesn't stop until it consumes all available memory (regardless of the video file size) or until the Nautilus window is closed (leaving the directory doesn't help. After closing the Nautilus window, all memory used by gvfsd-gphoto2 is freed and Nautilus shows the default video icon for the file (with no preview). Copying the file makes gnome-gphoto2 read whole file into memory and ONLY AFTER it is whole stored in memory, it begins writing onto HDD (or where you have pasted it). With new Ubuntu 12.10 I am not even able to copy the file as gvfsd-gphoto2 loads the file for a long time and probably doesn't even send any reply to Nautilus within 30 seconds after it has made a copy request (and popped up the copy dialog) - the progress bar stays at 0B copied and after 30 seconds of "inactivity" (the file is actually being read to memory) Nautilus stops the copying with a timeout error. The bug has originally been filled in Launchpad, the developers advised it could be a memory leak in gphoto2 backend and told me to report it upstream: https://bugs.launchpad.net/ubuntu/+source/gvfs/+bug/673151 this bug has been marked as a duplicate of the following bug: https://bugs.launchpad.net/ubuntu/+source/gvfs/+bug/348522 Thanks in advance, this years-old bug is really a pain for me and surely for many others.
The one workaround I've found for this is to spin up a Windows XP VM in Virtualbox and pass the USB device through to windows, then copy the files from a Virtualbox shared folder, using windows, onto the device. This bypasses gvfs entirely. Such a shame that stuff like this sits broken for years making it impossible to recommend that normal users use Linux on the desktop. Unfortunately, fixing this bug is beyond my abilities.
Created attachment 260111 [details] [review] gphoto2: Implement pull support Make use of gp_file_new_from_fd() to copy the file from the device straight to the local file without copying it into memory first. This fixes memory usage issues with copying large videos off devices. A positive side effect is that the progress bar now behaves correctly when copying a large file.
The above patch should fix the memory usage problem. Some points to note: * You may get timeouts when coping a large file with Nautilus (see https://bugzilla.gnome.org/show_bug.cgi?id=712585). This will hopefully be fixed soon. * Nautilus generating video thumbnails may cause the entire video to be loaded into memory. A workaround is to disable previews.
(In reply to comment #4) > The above patch should fix the memory usage problem. <snip> > * Nautilus generating video thumbnails may cause the entire video to be loaded > into memory. A workaround is to disable previews. totem-video-thumbnailer wouldn't load the full video (or rather, wouldn't even be called) if the gphoto backend provided a thumbnail for it already. I imagine that this metadata is probably already on the device. See G_FILE_ATTRIBUTE_PREVIEW_ICON.
(In reply to comment #5) > (In reply to comment #4) > > The above patch should fix the memory usage problem. > <snip> > > * Nautilus generating video thumbnails may cause the entire video to be loaded > > into memory. A workaround is to disable previews. > > totem-video-thumbnailer wouldn't load the full video (or rather, wouldn't even > be called) if the gphoto backend provided a thumbnail for it already. I imagine > that this metadata is probably already on the device. See > G_FILE_ATTRIBUTE_PREVIEW_ICON. I played around and found that on my Android device, it only has thumbnails of photos and returns NOT_SUPPORTED when retrieving the video thumbnail which causes Nautilus to invoke totem-video-thumbnailer and load the whole video into memory (and it only generates a thumbnail with the patch from #712601).
Review of attachment 260111 [details] [review]: Thanks for your patch, however there is several small things to fix: ::: daemon/gvfsbackendgphoto2.c @@ +3464,3 @@ + GFileProgressCallback progress_callback; + gpointer progress_callback_data; +} PullContext; Would be nice to put this at the start of file to others handles. @@ +3492,3 @@ +ctx_progress_stop_func (GPContext *context, + unsigned int id, + void *data) Please remove the tabs. @@ +3533,3 @@ + /* Fallback to the default implementation unless we have a regular file */ + if (!file_get_info (gphoto2_backend, dir, name, info, &error, FALSE) || + g_file_info_get_file_type (info) != G_FILE_TYPE_REGULAR) Would be better to use: is_regular (GVfsBackendGphoto2 *gphoto2_backend, const char *dir, const char *name) @@ +3548,3 @@ + flags & G_FILE_COPY_BACKUP ? TRUE : FALSE, + G_FILE_CREATE_REPLACE_DESTINATION, + NULL, &error)); Please use brackets if there is else section. Would be good to use G_VFS_JOB (job)->cancellable instead of NULL (also for g_file_create). @@ +3578,3 @@ + ctx_progress_update_func, + ctx_progress_stop_func, + &pc); Please remove the tabs.
Created attachment 264389 [details] [review] gphoto2: Implement pull support Make use of gp_file_new_from_fd() to copy the file from the device straight to the local file without copying it into memory first. This fixes memory usage issues with copying large videos off devices. A positive side effect is that the progress bar now behaves correctly when copying a large file.
Thanks for the review. (In reply to comment #7) > Review of attachment 260111 [details] [review]: > > Thanks for your patch, however there is several small things to fix: > > ::: daemon/gvfsbackendgphoto2.c > @@ +3464,3 @@ > + GFileProgressCallback progress_callback; > + gpointer progress_callback_data; > +} PullContext; > > Would be nice to put this at the start of file to others handles. Done. > > @@ +3492,3 @@ > +ctx_progress_stop_func (GPContext *context, > + unsigned int id, > + void *data) > > Please remove the tabs. Done. > > @@ +3533,3 @@ > + /* Fallback to the default implementation unless we have a regular file */ > + if (!file_get_info (gphoto2_backend, dir, name, info, &error, FALSE) || > + g_file_info_get_file_type (info) != G_FILE_TYPE_REGULAR) > > Would be better to use: > is_regular (GVfsBackendGphoto2 *gphoto2_backend, const char *dir, const char > *name) It is necessary to call file_get_info() to get the file size anyway, so I think it is probably easier (and marginally more efficient) to just do it like this. > > @@ +3548,3 @@ > + flags & G_FILE_COPY_BACKUP ? TRUE : FALSE, > + G_FILE_CREATE_REPLACE_DESTINATION, > + NULL, &error)); > > Please use brackets if there is else section. Done. > Would be good to use G_VFS_JOB (job)->cancellable instead of NULL (also for > g_file_create). Done. > > @@ +3578,3 @@ > + ctx_progress_update_func, > + ctx_progress_stop_func, > + &pc); > > Please remove the tabs. Done.
Review of attachment 264389 [details] [review]: Looks good!
Pushed to master as 86162bbe4b09f517b551ff1c9207a119e91ab733. Thanks! Note that this fixes the issue for the common use case, pulling files off a device to a local disk. Arbitrary reading of a file still requires it to be read into memory with obvious consequences. If there are still problems with memory usage, please open another bug.