Subject: bug#12463: 24.2; pos-visible-in-window-p gets slower over time

From: Chong Yidong <[email-masked]>
Date: Fri, 21 Sep 2012 18:01:19 +0800

Juanma Barranquero <[email-masked]> writes:

> Additionally to Eli's comments, the functions init_*_functions are
> defined after being used to initialize the struct, so it fails:
>
> You'll have to move them around.

Thanks.  Fixed in the attached patch, which also addresses Eli's other
comments.  Please take a look again.

> Anyway, what I was doing to move Vlibrary_cache out of w32 is affected
> by your patch, so I'd be happy if you could also fix that. The main
> issue is making sure that dynamic loading can be used before dumping,
> but the dumped Vlibrary_cache and image_types do not get dumped with
> entries that would not be relevant afterwards (that's could be an
> issue on Windows).

As far as I can tell, my patch should not change anything in this
regard, except that when you move Vlibrary_cache out of w32, you can
also get rid of the #ifdef NTGUI in define_image_type.

Eli Zaretskii <[email-masked]> writes:

> Since LIBRARIES could now be Qnil, but this function cannot tolerate
> that, there should be an assertion to that effect, either here or in
> every type->init function.

Passing a nil LIBRARIES argument causes nil to be passed to
w32_delayed_load, which AFAICT is safe; w32_delayed_load deals with nil
just fine, as a no-op.

=== modified file 'src/dispextern.h'
*** src/dispextern.h	2012-09-13 02:21:28 +0000
--- src/dispextern.h	2012-09-21 09:29:26 +0000
***************
*** 2766,2771 ****
--- 2766,2775 ----
    /* Free resources of image IMG which is used on frame F.  */
    void (* free) (struct frame *f, struct image *img);
  
+   /* Initialization function (used for dynamic loading of image
+      libraries on Windows), 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 09:47:34 +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 *);
***************
*** 579,632 ****
    do { Vimage_types = Fcons (type, Vimage_types); } while (0)
  
  /* 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;
  }

--- 579,629 ----
    do { Vimage_types = Fcons (type, Vimage_types); } while (0)
  
  /* Define a new image type from TYPE.  This adds a copy of TYPE to
!    image_types and caches the loading status of TYPE.
  
!    LIBRARIES is an alist associating dynamic libraries to external
!    files implementing them, which is passed to the image library
!    initialization function if necessary.  A nil value defaults to
!    Vdynamic_library_alist.  */
! 
! 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);
  	      }
--- 650,656 ----
  	    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;
--- 983,989 ----
  
    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 ****
--- 2259,2265 ----
    xbm_image_p,
    xbm_load,
    x_clear_image,
+   NULL,
    NULL
  };
  
***************
*** 3051,3056 ****
--- 3049,3060 ----
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_xpm_functions (Lisp_Object);
+ #else
+ #define init_xpm_functions NULL
+ #endif
+ 
  /* Structure describing the image type XPM.  */
  
  static struct image_type xpm_type =
***************
*** 3059,3064 ****
--- 3063,3069 ----
    xpm_image_p,
    xpm_load,
    x_clear_image,
+   init_xpm_functions,
    NULL
  };
  
***************
*** 4981,4986 ****
--- 4986,4992 ----
    pbm_image_p,
    pbm_load,
    x_clear_image,
+   NULL,
    NULL
  };
  
***************
*** 5387,5392 ****
--- 5393,5404 ----
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_png_functions (Lisp_Object);
+ #else
+ #define init_png_functions NULL
+ #endif
+ 
  /* Structure describing the image type `png'.  */
  
  static struct image_type png_type =
***************
*** 5395,5400 ****
--- 5407,5413 ----
    png_image_p,
    png_load,
    x_clear_image,
+   init_png_functions,
    NULL
  };
  
***************
*** 6039,6044 ****
--- 6052,6063 ----
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_jpeg_functions (Lisp_Object);
+ #else
+ #define init_jpeg_functions NULL
+ #endif
+ 
  /* Structure describing the image type `jpeg'.  */
  
  static struct image_type jpeg_type =
***************
*** 6047,6052 ****
--- 6066,6072 ----
    jpeg_image_p,
    jpeg_load,
    x_clear_image,
+   init_jpeg_functions,
    NULL
  };
  
***************
*** 6624,6629 ****
--- 6644,6655 ----
    {":index",		IMAGE_NON_NEGATIVE_INTEGER_VALUE,	0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_tiff_functions (Lisp_Object);
+ #else
+ #define init_tiff_functions NULL
+ #endif
+ 
  /* Structure describing the image type `tiff'.  */
  
  static struct image_type tiff_type =
***************
*** 6632,6637 ****
--- 6658,6664 ----
    tiff_image_p,
    tiff_load,
    x_clear_image,
+   init_tiff_functions,
    NULL
  };
  
***************
*** 7072,7077 ****
--- 7099,7110 ----
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_gif_functions (Lisp_Object);
+ #else
+ #define init_gif_functions NULL
+ #endif
+ 
  /* Structure describing the image type `gif'.  */
  
  static struct image_type gif_type =
***************
*** 7080,7085 ****
--- 7113,7119 ----
    gif_image_p,
    gif_load,
    gif_clear_image,
+   init_gif_functions,
    NULL
  };
  
***************
*** 7562,7567 ****
--- 7596,7607 ----
      {":crop",		IMAGE_DONT_CHECK_VALUE_TYPE,		0}
    };
  
+ #ifdef HAVE_NTGUI
+ static int init_imagemagick_functions (Lisp_Object);
+ #else
+ #define init_imagemagick_functions NULL
+ #endif
+ 
  /* Structure describing the image type for any image handled via
     ImageMagick.  */
  
***************
*** 7571,7576 ****
--- 7611,7617 ----
      imagemagick_image_p,
      imagemagick_load,
      imagemagick_clear_image,
+     init_imagemagick_functions,
      NULL
    };
  
***************
*** 8109,8130 ****
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
  /* Structure describing the image type `svg'.  Its the same type of
     structure defined for all image formats, handled by emacs image
     functions.  See struct image_type in dispextern.h.  */
  
  static struct image_type svg_type =
  {
-   /* An identifier showing that this is an image structure for the SVG format.  */
    &Qsvg,
-   /* Handle to a function that can be used to identify a SVG file.  */
    svg_image_p,
-   /* Handle to function used to load a SVG file.  */
    svg_load,
-   /* Handle to function to free sresources for SVG.  */
    x_clear_image,
!   /* 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
  };
  
--- 8150,8172 ----
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_svg_functions (Lisp_Object);
+ #else
+ #define init_svg_functions NULL
+ #endif
+ 
  /* Structure describing the image type `svg'.  Its the same type of
     structure defined for all image formats, handled by emacs image
     functions.  See struct image_type in dispextern.h.  */
  
  static struct image_type svg_type =
  {
    &Qsvg,
    svg_image_p,
    svg_load,
    x_clear_image,
!   init_svg_functions,
    NULL
  };
  
***************
*** 8504,8509 ****
--- 8546,8557 ----
    {":background",	IMAGE_STRING_OR_NIL_VALUE,		0}
  };
  
+ #ifdef HAVE_NTGUI
+ static int init_gs_functions (Lisp_Object);
+ #else
+ #define init_gs_functions NULL
+ #endif
+ 
  /* Structure describing the image type `ghostscript'.  */
  
  static struct image_type gs_type =
***************
*** 8512,8517 ****
--- 8560,8566 ----
    gs_image_p,
    gs_load,
    gs_clear_image,
+   init_gs_functions,
    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.
--- 8823,8828 ----
***************
*** 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
--- 8832,8902 ----
  of `dynamic-library-alist', which see).  */)
    (Lisp_Object type, Lisp_Object libraries)
  {
!   return lookup_image_type (type, libraries) ? Qt : Qnil;
! }
! 
! /* Look up image type TYPE, and return a pointer to its image_type
!    structure.  Return 0 if TYPE is not a known image type.
! 
!    LIBRARIES is an alist associating dynamic libraries to external
!    files implementing them, which is passed to the image library
!    initialization function if necessary.  A nil value defaults to
!    Vdynamic_library_alist.  */
! 
! 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