Bug 296401 - [PATCH] Gwenview leaks memory in fullscreen mode
Summary: [PATCH] Gwenview leaks memory in fullscreen mode
Alias: None
Product: gwenview
Classification: Applications
Component: general (show other bugs)
Version: 2.8.1
Platform: Ubuntu Linux
: NOR critical (vote)
Target Milestone: ---
Assignee: Gwenview Bugs
: 241574 300730 (view as bug list)
Depends on:
Reported: 2012-03-19 23:47 UTC by Benni Hill
Modified: 2012-12-02 00:25 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.8.4

use insertMulti() (574 bytes, patch)
2012-03-21 23:48 UTC, Benni Hill
don't overwrite mLastAccess (2.48 KB, patch)
2012-03-27 22:24 UTC, Benni Hill

Note You need to log in before you can comment on or make changes to this bug.
Description Benni Hill 2012-03-19 23:47:21 UTC
User-Agent:       Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Build Identifier: 

Gwenview leaks memory in fullscreen mode. I'm using Gwenview 2.8.1 from Kubuntu Backports ppa.

Reproducible: Always

Steps to Reproduce:
1. Open the first image of a directory with many (~500) high res (~12 mega pixels) images and switch to full screen
2. Switch through the images. Memory consumption will go up constantly.
3. You can speed up memory increase if you zoom every image to 100% ("F" button)
Actual Results:  
Gwenview demands more memory than available. System is swapping.

Expected Results:  
Gwenview should not use more memory than available.
Comment 1 Benni Hill 2012-03-20 21:10:26 UTC
I just confirmed this bug with latest git version. 
Disabling preloading (with exporting GWENVIEW_MAX_UNREFERENCED_IMAGES=0) seems to work around this bug.
Comment 2 Benni Hill 2012-03-21 23:48:44 UTC
Created attachment 69795 [details]
use insertMulti()

I think I fixed it.
The problem is that in fullscreen mode the thumbnails at the top often get loaded at the same time (< 1msec difference), so the garbageCollector sees the same mLastAccess for all images and therefore only one image is inserted into the QMap UnreferencedImages. I changed the garbageCollector to use insertMulti(). Please see attached patch and test.

(IMHO overwriting the mLastAccess value by the thumbnail bar won't remove the oldest image from memory?)
Comment 3 Benni Hill 2012-03-27 22:24:22 UTC
Created attachment 69952 [details]
don't overwrite mLastAccess

I found out that the mLastAccess variable is constantly overwritten in DocumentInfoProvider::isModified and DocumentInfoProvider::isBusy.
I changed
DocumentFactory::load(const KUrl& url) to
DocumentFactory::load(const KUrl& url, bool updateLastAccess)
with updateLastAccess=true as default and changed the factory->load(url) call in isModified and isBusy to factory->load(url, false). This seems to solve the problem (in an inelegant way).
Comment 4 Benni Hill 2012-03-28 14:54:22 UTC
As it seems to me that isModified and isBusy just need a pointer to an already loaded image, wouldn't it be better to implement an DocumentFactory::find(const KUrl& url) function which returns a pointer if the url has already been loaded (and hasUrl could be deprecated)?
Comment 5 Aurelien Gateau 2012-05-04 15:24:46 UTC
Hi, thanks for the patch! and sorry for the late answer :(.

I am looking at your work right now.
Comment 6 Aurelien Gateau 2012-05-04 16:22:11 UTC
Git commit c2439f20fcabbde514a18389e4718b397bc0bd17 by Aurélien Gâteau.
Committed on 04/05/2012 at 18:21.
Pushed by gateau into branch 'KDE/4.8'.

Fix leaking documents

Documents were never released because DocumentInfoProvider kept loading them.
A big thank you to Benni Hill for discovering the bug and proposing the
initial patch.

M  +6    -11   app/documentinfoprovider.cpp
M  +12   -4    lib/document/documentfactory.cpp
M  +19   -1    lib/document/documentfactory.h

Comment 7 Benni Hill 2012-12-02 00:23:16 UTC
*** Bug 241574 has been marked as a duplicate of this bug. ***
Comment 8 Benni Hill 2012-12-02 00:25:21 UTC
*** Bug 300730 has been marked as a duplicate of this bug. ***