'Re: [Kde-pim] Review Request: Fix ETM memory leak: m_Items hash table was never cleaned up'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] Review Request: Fix ETM memory leak: m_Items hash table was never cleaned up
From:       "Milian Wolff" <mail () milianw ! de>
Date:       2012-10-16 22:21:08
Message-ID: 20121016222108.17392.2822 () vidsolbach ! de
[Download RAW message or body]

> On Oct. 16, 2012, 5:40 p.m., Alex Fiestas wrote:
> > I have applied the patch and modified MAXBUFFERS to 1 (expecting to get \
> > only 1 folder cached). 
> > Then loaded in this order:
> > Inbox (KMail using 70Mb, folder has around 2K of emails)
> > KDE-Commits (KMail going up to 800Mb, folder has roughly  200K of \
> > emails) Inbox
> > Akadmey (this folder has around 20Msg)
> > 
> > KMail stayed using 860Mb of ram.

Don't you have favorite folders? Where they all loaded? Can you compare the \
memory consumption (without changing MAXBUFFERS) before and after applying \
this patch and skimming through the same set of folders? Also please try to \
track whether the memory consumption further increases after you've loaded \
one of these massive folders once or whether it levels out.

Also note how my two memory profiles compare: While the one levels out and \
stays much lower than the other, it doesn't go down either. Something which \
I could so far only explain through memory fragmentation... or other \
unexpected issues which where not apparent in massif.

To sum up: I think this patch is an improvement yet it might be that there \
are more places of undesired implicit sharing and thus "leaking".

- Milian

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106832/#review20460
-----------------------------------------------------------

On Oct. 15, 2012, 12:13 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106832/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2012, 12:13 p.m.)
> 
> 
> Review request for KDEPIM, KDEPIM-Libraries and Stephen Kelly.
> 
> 
> Description
> -------
> 
> As it turned out, the ETM kept filling its QHash<Item::Id, Item> m_items \
> without ever cleaning it up. This resulted in increasing memory \
> consumption of KMail until eventually all collections where fetched at \
> least once by the ETM. Note: It's not a true leak, i.e. the data is still \
> reachable and gets properly cleaned up on close. But during runtime the \
> ever increasing memory consumption is easily noticeable. 
> Generally this patch ensures that the existing cache code is actually \
> working. Maybe the old code was working once, but a regression due to the \
> patch that moved some of that code from ETMPrivate to \
> MonitorPrivate::PurgeBuffer crept in? 
> Anyhow, this patch contains basically of these parts:
> 
> a) Ensure that the SelectionProxyModel properly derefs root indices in \
> its dtor. Should be a non-brainer, compare to the ctor e.g. If this is \
> not done, collections will always stay reffed and thus never cleared up. 
> b) Fix the implementation of the PurgeBuffer. The old code was as far as \
> I could see non functional. The safety calls to ::purge from ::buffer \
> e.g. purged much more than just the one requested ID from the buffer. \
> This in turn resulted in ::buffer never returning something besides "-1". \
>  Instead, the code is now simplified by using a QQueue FIFO. Considering \
> though that this change was mostly based on how I thought the buffer \
> *should* behave like, this might need some reviewing. Note that I didn't \
> find the documentation sufficient. The buffer, as I've written it now, is \
> now working as following: 
> PurgeBuffer::purge(id) -> just remove id from FIFO
> PurgeBuffer::buffer(id) -> 
> - remove id if it already is tracked (i.e. purge(id))
> - ensure the FIFO is not bigger than X items, if it is, dequeue and purge \
>                 first item
> - enqueue id to FIFO
> 
> c) Fix the implementation of ETMPrivate::removeItems and ::purgeItems \
> such that it does not keep iterators around which get invalidated once we \
> call .erase on the parent container. This would yield double deletion \
> runtime errors and the like. 
> d) Fix the memory leak by actually removing items from m_items in \
> ::removeItems. This requires us to remove the purged'd collection id from \
> m_populatedCols. 
> Note: This patch should be the basis for evaluating the caching \
> parameters (MAXITEMS=10000 and MAXBUFFERSIZE=10). Especially I think we \
> should investigate whether the MAXITEMS should not take precedence over \
> MAXBUFFERSIZE, such that e.g. opening 10 10k folders does not result in \
> 100k of items in memory, but instead only 10k items. 
> Note: Favorited folders are always reffed and thus never cleared from the \
> cache. 
> 
> Diffs
> -----
> 
> akonadi/entitytreemodel_p.cpp bc2ca28 
> akonadi/monitor_p.h b5d0984 
> akonadi/monitor_p.cpp b2e85eb 
> akonadi/selectionproxymodel.cpp 72ca71a 
> 
> Diff: http://git.reviewboard.kde.org/r/106832/diff/
> 
> 
> Testing
> -------
> 
> Ran it against my local setup. The memory consumption stays roughly \
> constant over time now when reading all my folders, instead of increasing \
> over time until all collections have been seen. 
> To reproduce: Open KMail, go from one collection to the next until you \
> have visited all. Note the memory consumption intermittently. Before: \
> Memory consumption increases until the end. After: Memory consumption \
> will reach a peak and go up and down as you open your collectoins. 
> 
> Screenshots
> -----------
> 
> memory consumption before patch
> http://git.reviewboard.kde.org/r/106832/s/774/
> memory consumption after patch
> http://git.reviewboard.kde.org/r/106832/s/775/
> 
> 
> Thanks,
> 
> Milian Wolff
> 
> 

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread] 