On 2013-03-24 07:28:03, Marwan Tanager wrote:
> This patch tries to solve a serious problem with zathura leaking a horrendous 
> amount of memory during scrolling through a document.
> The problem is that the current way of periodically reclaiming the cairo 
> surfaces of the pages widgets is not working at all. The only case in which 
> it 
> seems to work is the first time (and "only" the first time) the timeout 
> function purge_pages is triggered, and even this first time sometimes doesn't 
> free the memory if it happened while scrolling through the document.
> To see it in action, try opening a large document (e.g. 500+ pages), and 
> scroll through it by dragging the scrollbar or continuously pressing 
> <shift-j>, 
> or <shift-k>. Meanwhile, watch the output of 'top $(pidof zathura)' until you 
> end up with a throttled system riddled with continuous swapping. Zathura will 
> fiercely eat all the physical system memory while scrolling through a 
> relatively big document.
> I've spent a while trying to figure out why the memory isn't freed after the 
> first time the timeout function is triggered, but quite frankly, I couldn't 
> manage to get to any reasonable conclusion. Injecting debug messages in 
> zathura_page_widget_update_surface shows that all the pages, but the current 
> visible one, are eventually reclaimed. This means that cairo_surface_destroy 
> must have been called on every surface of a page that exceeded it's 
> end-of-life 
> threshold, but without actually doing it's job of freeing the surface memory.
> So, this patch bypasses this mechanism by freeing a surface as soon as it's 
> associated page becomes invisible. Try repeating the same experiment as above 
> with this patch, and watch the rate of memory consumption drops dramatically. 
> I 
> scrolled the entire length of a 1300-pages document and the end result was 
> 500 
> MB. And I don't think this is leaking memory due to cairo surfaces pixel 
> buffers, because it never increases after reaching this limit. Maybe it's 
> some 
> internal allocations done by GTK or GDK. This doesn't seem very far-fetched 
> for 
> an application with 1300+ GTK widgets in this case.

No, please don't. There is reason why we decided to keep the pages in
memory: consider a document with same pages, but one of them takes ages
to render ([1] has an extreme example of such a file, but any document
with larger images should do). In 0.0.8.x scrolling was a PITA with such
documents: you had to wait until the page was fully rendered to do
anything else.

Keeping the pages in memory for some time is our attempt to improve the
scrolling experience in that case. Scrolling back and forth between two
pages becomes a much better experience since you don't have to wait
until the page is re-rendered to see it. So it's a compromise between
memory usage and time spent to re-render pages.

The code to destroy the cairo surfaces periodically is not optimal, but
if it all, we should fix this portion of the code. Maybe a timing-based
solution is not a good idea and there are better ways to approach this,
but I really don't want to ruin the above use case again.

Ideas and patches to improve the page reclaiming code are very welcome.

Sebastian Ramacher
zathura mailing list

Reply via email to