Package: emacs;
Reported by: jwalt <at> garni.ch (Jörg Walter)
Date: Mon, 17 Sep 2012 23:58:01 UTC
Severity: normal
Merged with 12468
Found in version 24.2
Done: Chong Yidong <cyd <at> gnu.org>
Bug is archived. No further changes may be made.
Message #61 received at 12463 <at> debbugs.gnu.org (full text, mbox):
From: Chong Yidong <cyd <at> gnu.org> To: Juanma Barranquero <lekktu <at> gmail.com> Cc: Jörg Walter <jwalt <at> garni.ch>, 12463 <at> debbugs.gnu.org Subject: Re: bug#12463: 24.2; pos-visible-in-window-p gets slower over time Date: Fri, 21 Sep 2012 13:42:53 +0800
Chong Yidong <cyd <at> gnu.org> writes:
>> What kind of duplicate entries image_types had? Were they mostly (or
>> all) for pbm/xbm types?
>
> No, the duplicates were for other image types other than pbm/xbm (in
> this case svg). The trouble, as Jörg pointed out, is that
> define_image_types did not check for the prior existence of an image
> type before registering it again. An unfortunate oversight; I've
> committed a fix to the trunk.
On further inspection of Finit_image_library and friends, the current
arrangement seems sub-optimal. We have a Lisp-visible function
`init-image-library', which triggers loading of image types. This
function is called by lookup_image_type, which afterwards scans the
image_types linked list again.
Please take a look at the following patch, which moves all the work done
by Finit_image_library into lookup_image_type. This requires adding a
LIBRARIES argument to lookup_image_type, with the same meaning as
Finit_image_library and defaulting to Vdynamic_library_alist. Then
Finit_image_library would be a rather simple wrapper around
lookup_image_type. Other callers to lookup_image_type, such as
redisplay looking up an image, would be unaffected.
This patch also gets rid of the CHECK_LIB_AVAILABLE macro and replaces
it using a new slot in struct image_type, which stores the
initialization function for dynamic loading, if any.
This patch is orthogonal to the issue of moving Vlibrary_cache outside
w32. One difference in behavior is that it makes the Windows code scan
Vlibrary_cache only after it has already failed to look for the image
type in image_types, and is about to run the initialization function. I
think this is a bit more straightforward.
=== modified file 'src/dispextern.h'
*** src/dispextern.h 2012-09-13 02:21:28 +0000
--- src/dispextern.h 2012-09-21 05:15:36 +0000
***************
*** 2766,2771 ****
--- 2766,2774 ----
/* Free resources of image IMG which is used on frame F. */
void (* free) (struct frame *f, struct image *img);
+ /* Initialization function, or NULL if none. */
+ int (* init) (Lisp_Object);
+
/* Next in list of all supported image types. */
struct image_type *next;
};
=== modified file 'src/image.c'
*** src/image.c 2012-09-21 03:52:23 +0000
--- src/image.c 2012-09-21 05:35:31 +0000
***************
*** 561,568 ****
/* Function prototypes. */
! static Lisp_Object define_image_type (struct image_type *type, int loaded);
! static struct image_type *lookup_image_type (Lisp_Object symbol);
static void image_error (const char *format, Lisp_Object, Lisp_Object);
static void x_laplace (struct frame *, struct image *);
static void x_emboss (struct frame *, struct image *);
--- 561,568 ----
/* Function prototypes. */
! static struct image_type *define_image_type (struct image_type *, Lisp_Object);
! static struct image_type *lookup_image_type (Lisp_Object, Lisp_Object);
static void image_error (const char *format, Lisp_Object, Lisp_Object);
static void x_laplace (struct frame *, struct image *);
static void x_emboss (struct frame *, struct image *);
***************
*** 581,632 ****
/* Define a new image type from TYPE. This adds a copy of TYPE to
image_types and caches the loading status of TYPE. */
! static Lisp_Object
! define_image_type (struct image_type *type, int loaded)
{
! Lisp_Object success;
! if (!loaded)
! success = Qnil;
! else
{
! struct image_type *p;
! Lisp_Object target_type = *(type->type);
! for (p = image_types; p; p = p->next)
! if (EQ (*(p->type), target_type))
! return Qt;
/* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
The initialized data segment is read-only. */
p = xmalloc (sizeof *p);
*p = *type;
p->next = image_types;
image_types = p;
- success = Qt;
}
! CACHE_IMAGE_TYPE (*type->type, success);
! return success;
! }
!
!
! /* Look up image type SYMBOL, and return a pointer to its image_type
! structure. Value is null if SYMBOL is not a known image type. */
!
! static inline struct image_type *
! lookup_image_type (Lisp_Object symbol)
! {
! struct image_type *type;
!
! /* We must initialize the image-type if it hasn't been already. */
! if (NILP (Finit_image_library (symbol, Vdynamic_library_alist)))
! return 0; /* unimplemented */
!
! for (type = image_types; type; type = type->next)
! if (EQ (symbol, *type->type))
! break;
!
! return type;
}
--- 581,624 ----
/* Define a new image type from TYPE. This adds a copy of TYPE to
image_types and caches the loading status of TYPE. */
! static struct image_type *
! define_image_type (struct image_type *type, Lisp_Object libraries)
{
! struct image_type *p = NULL;
! Lisp_Object target_type = *type->type;
! int type_valid = 1;
!
! for (p = image_types; p; p = p->next)
! if (EQ (*p->type, target_type))
! return p;
! if (type->init)
{
! #ifdef HAVE_NTGUI
! /* If we failed to load the library before, don't try again. */
! Lisp_Object tested = Fassq (target_type, Vlibrary_cache);
! if (CONSP (tested) && NILP (XCDR (tested)))
! type_valid = 0;
! else
! #endif
! {
! /* If the load failed, avoid trying again. */
! type_valid = (*type->init)(libraries);
! CACHE_IMAGE_TYPE (target_type, type_valid);
! }
! }
+ if (type_valid)
+ {
/* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
The initialized data segment is read-only. */
p = xmalloc (sizeof *p);
*p = *type;
p->next = image_types;
image_types = p;
}
! return p;
}
***************
*** 653,659 ****
if (CONSP (tem) && SYMBOLP (XCAR (tem)))
{
struct image_type *type;
! type = lookup_image_type (XCAR (tem));
if (type)
valid_p = type->valid_p (object);
}
--- 645,651 ----
if (CONSP (tem) && SYMBOLP (XCAR (tem)))
{
struct image_type *type;
! type = lookup_image_type (XCAR (tem), Qnil);
if (type)
valid_p = type->valid_p (object);
}
***************
*** 986,992 ****
eassert (valid_image_p (spec));
img->dependencies = NILP (file) ? Qnil : list1 (file);
! img->type = lookup_image_type (image_spec_value (spec, QCtype, NULL));
eassert (img->type != NULL);
img->spec = spec;
img->lisp_data = Qnil;
--- 978,984 ----
eassert (valid_image_p (spec));
img->dependencies = NILP (file) ? Qnil : list1 (file);
! img->type = lookup_image_type (image_spec_value (spec, QCtype, NULL), Qnil);
eassert (img->type != NULL);
img->spec = spec;
img->lisp_data = Qnil;
***************
*** 2262,2267 ****
--- 2254,2260 ----
xbm_image_p,
xbm_load,
x_clear_image,
+ NULL,
NULL
};
***************
*** 3059,3064 ****
--- 3052,3062 ----
xpm_image_p,
xpm_load,
x_clear_image,
+ #ifdef HAVE_NTGUI
+ init_xpm_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 4981,4986 ****
--- 4979,4985 ----
pbm_image_p,
pbm_load,
x_clear_image,
+ NULL,
NULL
};
***************
*** 5395,5400 ****
--- 5394,5404 ----
png_image_p,
png_load,
x_clear_image,
+ #ifdef HAVE_NTGUI
+ init_png_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 6047,6052 ****
--- 6051,6061 ----
jpeg_image_p,
jpeg_load,
x_clear_image,
+ #ifdef HAVE_NTGUI
+ init_jpeg_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 6632,6637 ****
--- 6641,6651 ----
tiff_image_p,
tiff_load,
x_clear_image,
+ #ifdef HAVE_NTGUI
+ init_tiff_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 7080,7085 ****
--- 7094,7104 ----
gif_image_p,
gif_load,
gif_clear_image,
+ #ifdef HAVE_NTGUI
+ init_gif_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 7571,7576 ****
--- 7590,7600 ----
imagemagick_image_p,
imagemagick_load,
imagemagick_clear_image,
+ #ifdef HAVE_NTGUI
+ init_imagemagick_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 8123,8128 ****
--- 8147,8157 ----
svg_load,
/* Handle to function to free sresources for SVG. */
x_clear_image,
+ #ifdef HAVE_NTGUI
+ init_svg_functions,
+ #else
+ NULL,
+ #endif
/* An internal field to link to the next image type in a list of
image types, will be filled in when registering the format. */
NULL
***************
*** 8512,8517 ****
--- 8541,8551 ----
gs_image_p,
gs_load,
gs_clear_image,
+ #ifdef HAVE_NTGUI
+ init_gs_functions,
+ #else
+ NULL,
+ #endif
NULL
};
***************
*** 8774,8789 ****
Initialization
***********************************************************************/
- #ifdef HAVE_NTGUI
- /* Image types that rely on external libraries are loaded dynamically
- if the library is available. */
- #define CHECK_LIB_AVAILABLE(image_type, init_lib_fn, libraries) \
- define_image_type (image_type, init_lib_fn (libraries))
- #else
- #define CHECK_LIB_AVAILABLE(image_type, init_lib_fn, libraries) \
- define_image_type (image_type, 1)
- #endif /* HAVE_NTGUI */
-
DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 2, 2, 0,
doc: /* Initialize image library implementing image type TYPE.
Return non-nil if TYPE is a supported image type.
--- 8808,8813 ----
***************
*** 8793,8853 ****
of `dynamic-library-alist', which see). */)
(Lisp_Object type, Lisp_Object libraries)
{
! #ifdef HAVE_NTGUI
! /* Don't try to reload the library. */
! Lisp_Object tested = Fassq (type, Vlibrary_cache);
! if (CONSP (tested))
! return XCDR (tested);
! #endif
/* Types pbm and xbm are built-in and always available. */
! if (EQ (type, Qpbm) || EQ (type, Qxbm))
! return Qt;
#if defined (HAVE_XPM) || defined (HAVE_NS)
if (EQ (type, Qxpm))
! return CHECK_LIB_AVAILABLE (&xpm_type, init_xpm_functions, libraries);
#endif
#if defined (HAVE_JPEG) || defined (HAVE_NS)
if (EQ (type, Qjpeg))
! return CHECK_LIB_AVAILABLE (&jpeg_type, init_jpeg_functions, libraries);
#endif
#if defined (HAVE_TIFF) || defined (HAVE_NS)
if (EQ (type, Qtiff))
! return CHECK_LIB_AVAILABLE (&tiff_type, init_tiff_functions, libraries);
#endif
#if defined (HAVE_GIF) || defined (HAVE_NS)
if (EQ (type, Qgif))
! return CHECK_LIB_AVAILABLE (&gif_type, init_gif_functions, libraries);
#endif
#if defined (HAVE_PNG) || defined (HAVE_NS)
if (EQ (type, Qpng))
! return CHECK_LIB_AVAILABLE (&png_type, init_png_functions, libraries);
#endif
#if defined (HAVE_RSVG)
if (EQ (type, Qsvg))
! return CHECK_LIB_AVAILABLE (&svg_type, init_svg_functions, libraries);
#endif
#if defined (HAVE_IMAGEMAGICK)
if (EQ (type, Qimagemagick))
! return CHECK_LIB_AVAILABLE (&imagemagick_type, init_imagemagick_functions,
! libraries);
#endif
#ifdef HAVE_GHOSTSCRIPT
if (EQ (type, Qpostscript))
! return CHECK_LIB_AVAILABLE (&gs_type, init_gs_functions, libraries);
#endif
! /* If the type is not recognized, avoid testing it ever again. */
! CACHE_IMAGE_TYPE (type, Qnil);
! return Qnil;
}
void
--- 8817,8883 ----
of `dynamic-library-alist', which see). */)
(Lisp_Object type, Lisp_Object libraries)
{
! struct image_type *p = lookup_image_type (type, libraries);
! return p ? *p->type : Qnil;
! }
!
! /* Look up image type SYMBOL, and return a pointer to its image_type
! structure. Value is null if SYMBOL is not a known image type. */
!
! static struct image_type *
! lookup_image_type (Lisp_Object type, Lisp_Object libraries)
! {
! if (NILP (libraries))
! libraries = Vdynamic_library_alist;
/* Types pbm and xbm are built-in and always available. */
! if (EQ (type, Qpbm))
! return &pbm_type;
!
! if (EQ (type, Qxbm))
! return &xbm_type;
#if defined (HAVE_XPM) || defined (HAVE_NS)
if (EQ (type, Qxpm))
! return define_image_type (&xpm_type, libraries);
#endif
#if defined (HAVE_JPEG) || defined (HAVE_NS)
if (EQ (type, Qjpeg))
! return define_image_type (&jpeg_type, libraries);
#endif
#if defined (HAVE_TIFF) || defined (HAVE_NS)
if (EQ (type, Qtiff))
! return define_image_type (&tiff_type, libraries);
#endif
#if defined (HAVE_GIF) || defined (HAVE_NS)
if (EQ (type, Qgif))
! return define_image_type (&gif_type, libraries);
#endif
#if defined (HAVE_PNG) || defined (HAVE_NS)
if (EQ (type, Qpng))
! return define_image_type (&png_type, libraries);
#endif
#if defined (HAVE_RSVG)
if (EQ (type, Qsvg))
! return define_image_type (&svg_type, libraries);
#endif
#if defined (HAVE_IMAGEMAGICK)
if (EQ (type, Qimagemagick))
! return define_image_type (&imagemagick_type, libraries);
#endif
#ifdef HAVE_GHOSTSCRIPT
if (EQ (type, Qpostscript))
! return define_image_type (&gs_type, libraries);
#endif
! return NULL;
}
void
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.