[zathura] [Zathura PATCH] RFC

[Marwan Tanager]

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.

While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

So, what's your suggestions, ideas (or maybe patches :-))?

And what's the final destiny of the periodic page reclaiming code?


Marwan Tanager (1):
Fix a horrible memory leak.

callbacks.c | 3 +++
page-widget.c | 9 +++++++++
page-widget.h | 8 ++++++++
3 files changed, 20 insertions(+)

[Marwan Tanager]

---
callbacks.c | 3 +++
page-widget.c | 9 +++++++++
page-widget.h | 8 ++++++++
3 files changed, 20 insertions(+)

diff --git a/callbacks.c b/callbacks.c
index 2c63945..89eacaf 100644
--- a/callbacks.c
+++ b/callbacks.c
@@ -102,6 +102,9 @@ cb_view_vadjustment_value_changed(GtkAdjustment* GIRARA_UNUSED(adjustment), gpoi
}
} else {
zathura_page_set_visibility(page, false);
+ if (zathura_page_widget_get_surface(ZATHURA_PAGE(page_widget)) != NULL) {
+ zathura_page_widget_update_surface(ZATHURA_PAGE(page_widget), NULL);
+ }
}
zathura_page_widget_update_view_time(ZATHURA_PAGE(page_widget));
}
diff --git a/page-widget.c b/page-widget.c
index bf94ba3..42ce131 100644
--- a/page-widget.c
+++ b/page-widget.c
@@ -501,6 +501,15 @@ zathura_page_widget_update_surface(ZathuraPage* widget, cairo_surface_t* surface
zathura_page_widget_redraw_canvas(widget);
}

+cairo_surface_t*
+zathura_page_widget_get_surface(ZathuraPage* widget)
+{
+ g_return_val_if_fail(ZATHURA_IS_PAGE(widget) == TRUE, NULL);
+ zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
+
+ return priv->surface;
+}
+
static void
zathura_page_widget_size_allocate(GtkWidget* widget, GdkRectangle* allocation)
{
diff --git a/page-widget.h b/page-widget.h
index fe0b67a..b2aa5ec 100644
--- a/page-widget.h
+++ b/page-widget.h
@@ -59,6 +59,14 @@ GtkWidget* zathura_page_widget_new(zathura_t* zathura, zathura_page_t* page);
* @param surface the new surface
*/
void zathura_page_widget_update_surface(ZathuraPage* widget, cairo_surface_t* surface);
+
+/**
+ * Retunrs a pointer to the cairo surface object associated with the given page widget.
+ * @param widget the page widget
+ * @return a pointer to a cairo surface object
+ */
+cairo_surface_t* zathura_page_widget_get_surface(ZathuraPage* widget);
+
/**
* Draw a rectangle to mark links or search results
* @param widget the widget

[Sebastian Ramacher]

Post by Marwan Tanager
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.

Cheers

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager
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.

Personally, I haven't had an encounter with such extreme cases, so I hadn't
thought of this, but of course this is something that should be considered.

Post by Sebastian Ramacher
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.

Of course, it's an obvious trade-off, but when it is very unbalanced like in
this case (I don't want to sacrifice all of my system memory and crash running
applications in order to have a good experience scrolling around a pdf
document), that's where we should reconsider doing things.

Post by Sebastian Ramacher
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.

That's why I sent the patch in the first place. I didn't actually intend it to
be applied, but just to shed some light on the source of the problem.



Marwan

[Ignas Anikevičius]

Hello,

Post by Sebastian Ramacher
...
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.
...

I was thinking about this and thought, that maybe a combination of two
approaches could be possible. I'll try to depict what I have in mind:

- Load zathura and render the pages (i.e. keep them in memory),
which are visible.

- When scrolling, remember all the pages seen in the last, say, 5
minutes.

- If we scroll through many pages, then it means that we are not
interested in viewing all of the pages, which we have seen in the
last 5 minutes. Then remember only the last 10 pages.

So in essence, what I propose, is to keep a page buffer which would
remember all pages from last 5 minutes or last 10 pages whichever is
smaller. Of course the quantities here are taken only for illustrative
purposes and I have no idea whether they make sense or not.

Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

Let me know if I made the explanation clear enough and if something
similar would be desired in zathura.

All the best,

Ignas

[Marwan Tanager]

Post by Ignas Anikevičius
Hello,

Post by Sebastian Ramacher
...
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.
...

I was thinking about this and thought, that maybe a combination of two
- Load zathura and render the pages (i.e. keep them in memory),
which are visible.
- When scrolling, remember all the pages seen in the last, say, 5
minutes.
- If we scroll through many pages, then it means that we are not
interested in viewing all of the pages, which we have seen in the
last 5 minutes. Then remember only the last 10 pages.
So in essence, what I propose, is to keep a page buffer which would
remember all pages from last 5 minutes or last 10 pages whichever is
smaller. Of course the quantities here are taken only for illustrative
purposes and I have no idea whether they make sense or not.

As far as I understand from this, you're talking about a cache of configurable
size (in number of pages), that are invalidated using a LRU algorithm. If
that's the case, then I like your idea, and think that it would be effective
here.

Post by Ignas Anikevičius
Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Yes, this is an actual bug that is reported at http://bugs.pwmt.org/issue274
and it should be fixed. Maybe you're the one who reported it!

Post by Ignas Anikevičius
Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

That's exactly what I've just been thinking about, simply a sliding window of a
configurable number of rows centered at the current visible one and moving with
us as we scroll around.

Combined with the above caching, then pages outside of this window shouldn't be
freed unless they 1) aren't in the cache, or 2) are in the cache, but need to
be invalidated in order to make room for the currently visible ones if they
aren't already in the cache.

Post by Ignas Anikevičius
Let me know if I made the explanation clear enough and if something
similar would be desired in zathura.

I think we're yet to see what Sebastian (and maybe others) think of this. I was
just expressing my opinion.



Marwan

[Sebastian Ramacher]

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Yes, this is an actual bug that is reported at http://bugs.pwmt.org/issue274
and it should be fixed. Maybe you're the one who reported it!

Everything except the file monitor and the zathura_t instance should be released
and re-created (if I remember that correctly), i.e. the document gets closed and
re-opened again. zathura is just a PITA to run with valgrind. There are so many
GTK related errors, that we can't fix, that valgrind's output is pretty much
useless.

We might have missed a free, g_object_unref, etc. somewhere of course. So
ideas to find memory leaks without valgrind or a use-able suppression file for
valgrind are very welcome.

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

That's exactly what I've just been thinking about, simply a sliding window of a
configurable number of rows centered at the current visible one and moving with
us as we scroll around.
Combined with the above caching, then pages outside of this window shouldn't be
freed unless they 1) aren't in the cache, or 2) are in the cache, but need to
be invalidated in order to make room for the currently visible ones if they
aren't already in the cache.

That'd be nice to have. Instead of a configurable number of rows we'd need
something that also loads pages left and right of the current page if
pages-per-row is larger than 1.

Regards

[Sebastian Ramacher]

Post by Sebastian Ramacher

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

That's exactly what I've just been thinking about, simply a sliding window of a
configurable number of rows centered at the current visible one and moving with
us as we scroll around.
Combined with the above caching, then pages outside of this window shouldn't be
freed unless they 1) aren't in the cache, or 2) are in the cache, but need to
be invalidated in order to make room for the currently visible ones if they
aren't already in the cache.

That'd be nice to have. Instead of a configurable number of rows we'd need
something that also loads pages left and right of the current page if
pages-per-row is larger than 1.

JFTR: that's http://bugs.pwmt.org/issue267.

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Yes, this is an actual bug that is reported at http://bugs.pwmt.org/issue274
and it should be fixed. Maybe you're the one who reported it!

Everything except the file monitor and the zathura_t instance should be released
and re-created (if I remember that correctly), i.e. the document gets closed and
re-opened again. zathura is just a PITA to run with valgrind. There are so many
GTK related errors, that we can't fix, that valgrind's output is pretty much
useless.
We might have missed a free, g_object_unref, etc. somewhere of course. So
ideas to find memory leaks without valgrind or a use-able suppression file for
valgrind are very welcome.

I had actually done a quick scan on the code as far as freeing the memory upon
document closing is concerned, but wasn't able to spot anything missing.
Everything seems to be freed, the poppler pages, the widgets, the cairo
surfaces. But, of course, I'm not omniscient, and I could have missed
something, too.

While thinking about this issue, I got an idea that might be related; could
this memory be allocated to the "target" surfaces of the widgets (i.e., the
targets of the cairo contexts that are passed to zathura_page_widget_draw), as
opposed to the source surfaces stored in zathura_page_widget_private_s that are
used to draw on those targets, and which now seem to be freed when the cache
entries are invalidated?

Now, with a reasonable cache size, the rate of memory consumption is much less
than it was before, which confirms that the source surfaces are actually freed.
But, if memory is allocated only to the source surfaces, then the memory
consumption should stay fixed as soon as ${page-cache-size} pages hit the
screen. But, this is not what actually happens, and the memory consumption
keeps increasing until each page in the document is viewed at least once. At
this point, no more memory is consumed while scrolling.

This makes me think that if the seemingly leaking memory is allocated to the
target surfaces of the widgets, and GTK doesn't free this memory when freeing
the widgets, then this might explain all of the above issues (creeping memory
while scrolling, and after document reloads), and unfortunately, in this case,
I think we wouldn't be able to do much about it. So, I hope I'm wrong ;)


--
Marwan Tanager

[Sebastian Ramacher]

Post by Sebastian Ramacher

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

That's exactly what I've just been thinking about, simply a sliding window of a
configurable number of rows centered at the current visible one and moving with
us as we scroll around.
Combined with the above caching, then pages outside of this window shouldn't be
freed unless they 1) aren't in the cache, or 2) are in the cache, but need to
be invalidated in order to make room for the currently visible ones if they
aren't already in the cache.

That'd be nice to have. Instead of a configurable number of rows we'd need
something that also loads pages left and right of the current page if
pages-per-row is larger than 1.

JFTR: that's http://bugs.pwmt.org/issue267.

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Yes, this is an actual bug that is reported at http://bugs.pwmt.org/issue274
and it should be fixed. Maybe you're the one who reported it!

Everything except the file monitor and the zathura_t instance should be released
and re-created (if I remember that correctly), i.e. the document gets closed and
re-opened again. zathura is just a PITA to run with valgrind. There are so many
GTK related errors, that we can't fix, that valgrind's output is pretty much
useless.
We might have missed a free, g_object_unref, etc. somewhere of course. So
ideas to find memory leaks without valgrind or a use-able suppression file for
valgrind are very welcome.

I had actually done a quick scan on the code as far as freeing the memory upon
document closing is concerned, but wasn't able to spot anything missing.
Everything seems to be freed, the poppler pages, the widgets, the cairo
surfaces. But, of course, I'm not omniscient, and I could have missed
something, too.

While thinking about this issue, I got an idea that might be related; could
this memory be allocated to the "target" surfaces of the widgets (i.e., the
targets of the cairo contexts that are passed to zathura_page_widget_draw), as
opposed to the source surfaces stored in zathura_page_widget_private_s that are
used to draw on those targets, and which now seem to be freed when the cache
entries are invalidated?

Now, with a reasonable cache size, the rate of memory consumption is much less
than it was before, which confirms that the source surfaces are actually freed.
But, if memory is allocated only to the source surfaces, then the memory
consumption should stay fixed as soon as ${page-cache-size} pages hit the
screen. But, this is not what actually happens, and the memory consumption
keeps increasing until each page in the document is viewed at least once. At
this point, no more memory is consumed while scrolling.

This makes me think that if the seemingly leaking memory is allocated to the
target surfaces of the widgets, and GTK doesn't free this memory when freeing
the widgets, then this might explain all of the above issues (creeping memory
while scrolling, and after document reloads), and unfortunately, in this case,
I think we wouldn't be able to do much about it. So, I hope I'm wrong ;)


--
Marwan Tanager

[Sebastian Ramacher]

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Yes, this is an actual bug that is reported at http://bugs.pwmt.org/issue274
and it should be fixed. Maybe you're the one who reported it!

Everything except the file monitor and the zathura_t instance should be released
and re-created (if I remember that correctly), i.e. the document gets closed and
re-opened again. zathura is just a PITA to run with valgrind. There are so many
GTK related errors, that we can't fix, that valgrind's output is pretty much
useless.

We might have missed a free, g_object_unref, etc. somewhere of course. So
ideas to find memory leaks without valgrind or a use-able suppression file for
valgrind are very welcome.

Post by Marwan Tanager

Post by Ignas Anikevičius
Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

That's exactly what I've just been thinking about, simply a sliding window of a
configurable number of rows centered at the current visible one and moving with
us as we scroll around.
Combined with the above caching, then pages outside of this window shouldn't be
freed unless they 1) aren't in the cache, or 2) are in the cache, but need to
be invalidated in order to make room for the currently visible ones if they
aren't already in the cache.

That'd be nice to have. Instead of a configurable number of rows we'd need
something that also loads pages left and right of the current page if
pages-per-row is larger than 1.

Regards

[Marwan Tanager]

Post by Ignas Anikevičius
Hello,

Post by Sebastian Ramacher
...
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.
...

I was thinking about this and thought, that maybe a combination of two
- Load zathura and render the pages (i.e. keep them in memory),
which are visible.
- When scrolling, remember all the pages seen in the last, say, 5
minutes.
- If we scroll through many pages, then it means that we are not
interested in viewing all of the pages, which we have seen in the
last 5 minutes. Then remember only the last 10 pages.
So in essence, what I propose, is to keep a page buffer which would
remember all pages from last 5 minutes or last 10 pages whichever is
smaller. Of course the quantities here are taken only for illustrative
purposes and I have no idea whether they make sense or not.

As far as I understand from this, you're talking about a cache of configurable
size (in number of pages), that are invalidated using a LRU algorithm. If
that's the case, then I like your idea, and think that it would be effective
here.

Post by Ignas Anikevičius
Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Yes, this is an actual bug that is reported at http://bugs.pwmt.org/issue274
and it should be fixed. Maybe you're the one who reported it!

Post by Ignas Anikevičius
Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

That's exactly what I've just been thinking about, simply a sliding window of a
configurable number of rows centered at the current visible one and moving with
us as we scroll around.

Combined with the above caching, then pages outside of this window shouldn't be
freed unless they 1) aren't in the cache, or 2) are in the cache, but need to
be invalidated in order to make room for the currently visible ones if they
aren't already in the cache.

Post by Ignas Anikevičius
Let me know if I made the explanation clear enough and if something
similar would be desired in zathura.

I think we're yet to see what Sebastian (and maybe others) think of this. I was
just expressing my opinion.



Marwan

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager
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.

Personally, I haven't had an encounter with such extreme cases, so I hadn't
thought of this, but of course this is something that should be considered.

Post by Sebastian Ramacher
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.

Of course, it's an obvious trade-off, but when it is very unbalanced like in
this case (I don't want to sacrifice all of my system memory and crash running
applications in order to have a good experience scrolling around a pdf
document), that's where we should reconsider doing things.

Post by Sebastian Ramacher
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.

That's why I sent the patch in the first place. I didn't actually intend it to
be applied, but just to shed some light on the source of the problem.



Marwan

[Ignas Anikevičius]

Hello,

Post by Sebastian Ramacher
...
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.
...

I was thinking about this and thought, that maybe a combination of two
approaches could be possible. I'll try to depict what I have in mind:

- Load zathura and render the pages (i.e. keep them in memory),
which are visible.

- When scrolling, remember all the pages seen in the last, say, 5
minutes.

- If we scroll through many pages, then it means that we are not
interested in viewing all of the pages, which we have seen in the
last 5 minutes. Then remember only the last 10 pages.

So in essence, what I propose, is to keep a page buffer which would
remember all pages from last 5 minutes or last 10 pages whichever is
smaller. Of course the quantities here are taken only for illustrative
purposes and I have no idea whether they make sense or not.

Also, I remember, that I hit a mem-leak recently when working on a
poster made with LaTeX. I was recompiling the same page over and over
again with LaTeX and I the memory consumption went very high. I was
wondering if this is because zathura was remembering the pages from the
previous versions of the recompiled document. Maybe zathura should free
memory on document reloads?

Also, I thought a while ago, that zathura's rendering experience could
be even further enhanced by caching the pages, which have not been seen
yet. For example at one moment we see a row of pages and zathura caches
the row above and below the one we currently view. Then if we scroll in
either direction, the page is already rendered, which might be visually
more pleasing than waiting for a large page to come up. Of course, this
should be implemented in such a way, that we do not recache the pages,
which are already among the ones which have been looked at recently.

Let me know if I made the explanation clear enough and if something
similar would be desired in zathura.

All the best,

Ignas

[Sebastian Ramacher]

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I played with some gdk_threads_{enter,leave} calls around the destroy to
no avail.

I will follow up to the other mails in this thread later this week.

[1] Nothing in the sense of that the first call gives a huge decrease in
RES usage and subsequent calls only a small decrease.

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I've done exactly the same test and made sure the ref count is always 1 before
the call. That's exactly what drove me nuts!



Marwan

[Sebastian Ramacher]

Post by Marwan Tanager

Post by Sebastian Ramacher

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I've done exactly the same test and made sure the ref count is always 1 before
the call. That's exactly what drove me nuts!

Yes, it's totally weird.

One further note: With the ps plugin the behavior is totally different
as far as I can tell.

Regards

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager

Post by Sebastian Ramacher

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I've done exactly the same test and made sure the ref count is always 1 before
the call. That's exactly what drove me nuts!

Yes, it's totally weird.
One further note: With the ps plugin the behavior is totally different
as far as I can tell.

I've just tried with two document, one postscript and the other djvu. On my
side, things seem to work well with djvu, but are still buggy with postscript.



Marwan

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager

Post by Sebastian Ramacher

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I've done exactly the same test and made sure the ref count is always 1 before
the call. That's exactly what drove me nuts!

Yes, it's totally weird.
One further note: With the ps plugin the behavior is totally different
as far as I can tell.

I've just tried with two document, one postscript and the other djvu. On my
side, things seem to work well with djvu, but are still buggy with postscript.



Marwan

[Sebastian Ramacher]

Post by Marwan Tanager

Post by Sebastian Ramacher

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I've done exactly the same test and made sure the ref count is always 1 before
the call. That's exactly what drove me nuts!

Yes, it's totally weird.

One further note: With the ps plugin the behavior is totally different
as far as I can tell.

Regards

[Marwan Tanager]

Post by Sebastian Ramacher

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I've done exactly the same test and made sure the ref count is always 1 before
the call. That's exactly what drove me nuts!



Marwan

[Marwan Tanager]

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.

While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

So, what's your suggestions, ideas (or maybe patches :-))?

And what's the final destiny of the periodic page reclaiming code?


Marwan Tanager (1):
Fix a horrible memory leak.

callbacks.c | 3 +++
page-widget.c | 9 +++++++++
page-widget.h | 8 ++++++++
3 files changed, 20 insertions(+)

[Marwan Tanager]

---
callbacks.c | 3 +++
page-widget.c | 9 +++++++++
page-widget.h | 8 ++++++++
3 files changed, 20 insertions(+)

diff --git a/callbacks.c b/callbacks.c
index 2c63945..89eacaf 100644
--- a/callbacks.c
+++ b/callbacks.c
@@ -102,6 +102,9 @@ cb_view_vadjustment_value_changed(GtkAdjustment* GIRARA_UNUSED(adjustment), gpoi
}
} else {
zathura_page_set_visibility(page, false);
+ if (zathura_page_widget_get_surface(ZATHURA_PAGE(page_widget)) != NULL) {
+ zathura_page_widget_update_surface(ZATHURA_PAGE(page_widget), NULL);
+ }
}
zathura_page_widget_update_view_time(ZATHURA_PAGE(page_widget));
}
diff --git a/page-widget.c b/page-widget.c
index bf94ba3..42ce131 100644
--- a/page-widget.c
+++ b/page-widget.c
@@ -501,6 +501,15 @@ zathura_page_widget_update_surface(ZathuraPage* widget, cairo_surface_t* surface
zathura_page_widget_redraw_canvas(widget);
}

+cairo_surface_t*
+zathura_page_widget_get_surface(ZathuraPage* widget)
+{
+ g_return_val_if_fail(ZATHURA_IS_PAGE(widget) == TRUE, NULL);
+ zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
+
+ return priv->surface;
+}
+
static void
zathura_page_widget_size_allocate(GtkWidget* widget, GdkRectangle* allocation)
{
diff --git a/page-widget.h b/page-widget.h
index fe0b67a..b2aa5ec 100644
--- a/page-widget.h
+++ b/page-widget.h
@@ -59,6 +59,14 @@ GtkWidget* zathura_page_widget_new(zathura_t* zathura, zathura_page_t* page);
* @param surface the new surface
*/
void zathura_page_widget_update_surface(ZathuraPage* widget, cairo_surface_t* surface);
+
+/**
+ * Retunrs a pointer to the cairo surface object associated with the given page widget.
+ * @param widget the page widget
+ * @return a pointer to a cairo surface object
+ */
+cairo_surface_t* zathura_page_widget_get_surface(ZathuraPage* widget);
+
/**
* Draw a rectangle to mark links or search results
* @param widget the widget

[Sebastian Ramacher]

Post by Marwan Tanager
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.

Cheers

[Sebastian Ramacher]

Post by Marwan Tanager
While browsing the bug tracker for related issues, I found the one at
http://bugs.pwmt.org/issue95 which points out the 0.0.8.5 version as not
suffering from this problem. I built it and experimented. The result was that
the total amount of memory consumed by scrolling through the length of the
1300-pages document was comparable to the one with this patch. So, there must
be something messy happening with cairo_surface_destroy when it's get called
from the timeout function while scrolling. Maybe it's non-reentrant and some
mess happens when it's called concurrently from both the timeout function and
the render thread. But if that's the case, then why doesn't it free the memory
after the first time, even when there is no scrolling? Also, I tried to
experiment with mutexes around the cairo_surface_destroy calls on the render
thread and zathura_page_widget_update_surface but nothing surprising has
happened.

I played a bit with the code tonight. What puzzles me is that it works
for the first time but not for any subsequent purge operations. As far
as I can tell, the ref counters of the surface are 1 and thus
cairo_surface_destroy should free the image. cairo_surface_destroy is
definitely called, but I really don't know why nothing happens [1].

I played with some gdk_threads_{enter,leave} calls around the destroy to
no avail.

I will follow up to the other mails in this thread later this week.

[1] Nothing in the sense of that the first call gives a huge decrease in
RES usage and subsequent calls only a small decrease.