From: Tassilo Horn <tassilo@member.fsf.org>
To: joakim@verona.se
Cc: Miles Bader <miles@gnu.org>, Jashy <nanjunjie@gmail.com>,
Emacs-devel@gnu.org
Subject: Re: Using the ImageMagick backend seems to leak memory
Date: Tue, 11 Jan 2011 20:10:29 +0100 [thread overview]
Message-ID: <8739oz8dei.fsf@member.fsf.org> (raw)
In-Reply-To: <m3r5cjl38b.fsf@verona.se> (joakim@verona.se's message of "Tue, 11 Jan 2011 19:11:48 +0100")
joakim@verona.se writes:
Hi Joakim,
> I've looked at this a little bit further.
>
> The error is that the image read with ReadImage is cloned by
> NewMagickWandFromImage not refered as I believed when I wrote the
> code.
>
> The code was written that way to avoid another memory problem I
> experienced with image bundles. If the current strategy remains,
> DestroyImage(im_image); should be the right fix.
I've tested that, and now memory is freed as expected. Still, when I
used doc-view to view a 30 pages PDF (aka 30 large PNGs), the emacs
process started with ~11 MB when showing the first page and was at 12.2
MB after viewing all pages in sequence, killing the doc-view buffer, and
doing (clear-image-cache) and (garbage-collect). But that might be no
real leak.
> However, the strategy should probably be changed to avoid the cloning.
> This will result in a more complex code path.
Well, you are the ImageMagick master. ;-)
Anyway, should I commit the following patch which includes your
DestroyImage() call that fixes the issue, and I additionally surrounded
image loading with calls to MagickWandGenesis()/MagickWandTerminus(),
because that's the way the API is used throughout the example codes in
the MagickWand API docs?
--8<---------------cut here---------------start------------->8---
=== modified file 'src/image.c'
--- src/image.c 2011-01-07 22:33:32 +0000
+++ src/image.c 2011-01-11 19:02:33 +0000
@@ -7521,6 +7521,9 @@
image. Interface :index is same as for GIF. First we "ping" the
image to see how many sub-images it contains. Pinging is faster
than loading the image to find out things about it. */
+
+ /* MagickWandGenesis() initializes the imagemagick library. */
+ MagickWandGenesis ();
image = image_spec_value (img->spec, QCindex, NULL);
ino = INTEGERP (image) ? XFASTINT (image) : 0;
ping_wand = NewMagickWand ();
@@ -7549,6 +7552,7 @@
img->data.lisp_val));
DestroyMagickWand (ping_wand);
+
/* Now, after pinging, we know how many images are inside the
file. If its not a bundle, just one. */
@@ -7566,6 +7570,7 @@
if (im_image != NULL)
{
image_wand = NewMagickWandFromImage (im_image);
+ DestroyImage(im_image);
status = MagickTrue;
}
else
@@ -7576,7 +7581,7 @@
image_wand = NewMagickWand ();
status = MagickReadImageBlob (image_wand, contents, size);
}
- image_error ("im read failed", Qnil, Qnil);
+
if (status == MagickFalse) goto imagemagick_error;
/* If width and/or height is set in the display spec assume we want
@@ -7805,11 +7810,13 @@
/* Final cleanup. image_wand should be the only resource left. */
DestroyMagickWand (image_wand);
+ MagickWandTerminus ();
return 1;
imagemagick_error:
DestroyMagickWand (image_wand);
+ MagickWandTerminus ();
/* TODO more cleanup. */
image_error ("Error parsing IMAGEMAGICK image `%s'", img->spec, Qnil);
return 0;
@@ -8681,8 +8688,6 @@
#if defined (HAVE_IMAGEMAGICK)
if (EQ (type, Qimagemagick))
{
- /* MagickWandGenesis() initializes the imagemagick library. */
- MagickWandGenesis ();
return CHECK_LIB_AVAILABLE (&imagemagick_type, init_imagemagick_functions,
libraries);
}
--8<---------------cut here---------------end--------------->8---
Bye,
Tassilo
next prev parent reply other threads:[~2011-01-11 19:10 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-06 7:51 Using the ImageMagick backend seems to leak memory Tassilo Horn
2011-01-06 9:02 ` joakim
2011-01-06 10:20 ` Tassilo Horn
2011-01-07 4:17 ` Jashy
2011-01-07 4:59 ` Miles Bader
2011-01-07 6:27 ` Stephen J. Turnbull
2011-01-07 7:25 ` Miles Bader
2011-01-07 8:28 ` Tassilo Horn
2011-01-07 22:39 ` Andreas Schwab
2011-01-10 10:34 ` Tassilo Horn
2011-01-10 13:28 ` Jashy
2011-01-10 13:56 ` Tassilo Horn
2011-01-10 14:36 ` Jashy
2011-01-11 10:14 ` Jashy
2011-01-11 10:59 ` joakim
2011-01-11 14:17 ` joakim
2011-01-11 14:38 ` joakim
2011-01-11 17:43 ` Tassilo Horn
2011-01-11 18:11 ` joakim
2011-01-11 19:10 ` Tassilo Horn [this message]
2011-01-11 19:35 ` joakim
2011-01-11 19:55 ` Tassilo Horn
2011-01-12 1:47 ` Stefan Monnier
2011-01-12 7:37 ` Tassilo Horn
2011-01-10 21:47 ` Tassilo Horn
2011-01-11 8:47 ` Tassilo Horn
2011-01-11 9:35 ` joakim
2011-01-11 9:53 ` Tassilo Horn
2011-01-11 9:54 ` joakim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8739oz8dei.fsf@member.fsf.org \
--to=tassilo@member.fsf.org \
--cc=Emacs-devel@gnu.org \
--cc=joakim@verona.se \
--cc=miles@gnu.org \
--cc=nanjunjie@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.