bug#15876: 24.3.50; Highly degraded performance between rev 114715 and 115006

[Dmitry Antipov <dmantipov@yandex.ru>]

[-- Attachment #1: Type: text/plain, Size: 3756 bytes --]

On 12/05/2013 09:36 PM, Eli Zaretskii wrote:

> Yes, I see the same on Windows.  However, I'm not sure we are looking
> at objects we should be looking at.  The ones your code traces are
> font objects.  By contrast, the entries in the font caches are cons
> cells like this:
>
>     (font-spec . [font-entity1 font-entity2 ...])
>
> IOW, I think we should be tracing font-specs and font-entities, not
> font objects.

IMO the real problem is that redisplay sometimes isn't clever enough about requesting
font for the particular character (0x25b7 in our case):

#0  font_make_entity () at ../../trunk/src/font.c:169
#1  0x00000000005a8a3f in xfont_list_pattern (display=display@entry=0x1379260,
     pattern=pattern@entry=0x7fffffff7730 "-adobe-courier-*-*-*--*-*-*-*-*-*-iso10646-1", registry=registry@entry=12260082,
     script=script@entry=12084946) at ../../trunk/src/xfont.c:403
#2  0x00000000005a9059 in xfont_list (f=<optimized out>, spec=12183525) at ../../trunk/src/xfont.c:515
#3  0x0000000000563193 in font_list_entities (f=f@entry=0x111fd58, spec=spec@entry=15986301) at ../../trunk/src/font.c:2735
#4  0x0000000000563b42 in font_find_for_lface (f=f@entry=0x111fd58, attrs=attrs@entry=0x16b9510, spec=<optimized out>,
     c=c@entry=9655) at ../../trunk/src/font.c:3206
#5  0x00000000005a9d44 in fontset_find_font (fontset=18338269, c=c@entry=9655, face=face@entry=0x16b9510, id=id@entry=-1,
     fallback=fallback@entry=false) at ../../trunk/src/fontset.c:681
#6  0x00000000005aa210 in fontset_font (fontset=fontset@entry=17988749, c=c@entry=9655, face=face@entry=0x16b9510, id=-1)
     at ../../trunk/src/fontset.c:754
#7  0x00000000005aacb2 in face_for_char (f=0x111fd58, face=face@entry=0x16b9510, c=9655, pos=<optimized out>, object=<optimized out>)
     at ../../trunk/src/fontset.c:978
#8  0x000000000043cb45 in get_next_display_element (it=it@entry=0x7fffffff97f0) at ../../trunk/src/xdisp.c:6997
#9  0x0000000000441bc3 in display_line (it=it@entry=0x7fffffff97f0) at ../../trunk/src/xdisp.c:19593
#10 0x000000000044506a in try_window (window=window@entry=17964077, pos=..., flags=flags@entry=1) at ../../trunk/src/xdisp.c:16505
#11 0x000000000045ab57 in redisplay_window (window=17964077, just_this_one_p=just_this_one_p@entry=false)
     at ../../trunk/src/xdisp.c:16022
#12 0x000000000045cbf3 in redisplay_window_0 (window=window@entry=17964077) at ../../trunk/src/xdisp.c:14023
#13 0x000000000054d216 in internal_condition_case_1 (bfun=bfun@entry=0x45cbc0 <redisplay_window_0>, arg=17964077,
     handlers=<optimized out>, hfun=hfun@entry=0x4290a0 <redisplay_window_error>) at ../../trunk/src/eval.c:1368
#14 0x000000000042d8ce in redisplay_windows (window=17964077) at ../../trunk/src/xdisp.c:14003
#15 0x000000000044a121 in redisplay_internal () at ../../trunk/src/xdisp.c:13602

Such a request produces a lot of font-entity objects, which are not needed (and becomes
reachable only via font cache) after the "best match" font is selected for the particular
character. Next, if font loading is too memory-intensive and gc_cons_threshold was hit, GC
clears font cache. Next cursor movement triggers redisplay, which in turn asks for the "best
match" for 0x25b7 again. If font cache is never cleared, this is acceptable because all possible
matches (represented as a vector of font-entities) are cached. But, if font cache is cleared,
font_list_entities calls to driver->list function, which creates a lot of font-entities again,
etc., etc.

Until we have somewhat smarter redisplay, possible solution is to clear font cache when it
becomes larger than some specified size rather than at each GC. Now I have the virtual machine
with Windows 7 installed, and this fix looks reasonable for me. Could you also please try it?

Dmitry


[-- Attachment #2: font_cache_size_bug15876.patch --]
[-- Type: text/x-patch, Size: 5247 bytes --]

=== modified file 'src/alloc.c'
--- src/alloc.c	2013-12-09 08:23:01 +0000
+++ src/alloc.c	2013-12-11 05:38:41 +0000
@@ -5299,17 +5299,22 @@
 
 #ifdef HAVE_WINDOW_SYSTEM
 
+/* Rather arbitrary but willing to fix Bug#15876.  */
+
+#define FONT_CACHE_THRESHOLD 4096
+
 /* Remove unmarked font-spec and font-entity objects from ENTRY, which is
    (DRIVER-TYPE NUM-FRAMES FONT-CACHE-DATA ...), and return changed entry.  */
 
 static Lisp_Object
-compact_font_cache_entry (Lisp_Object entry)
+compact_font_cache_entry (struct terminal *t, Lisp_Object entry)
 {
   Lisp_Object tail, *prev = &entry;
 
   for (tail = entry; CONSP (tail); tail = XCDR (tail))
     {
       bool drop = 0;
+      ptrdiff_t size = 0;
       Lisp_Object obj = XCAR (tail);
 
       /* Consider OBJ if it is (font-spec . [font-entity font-entity ...]).  */
@@ -5317,8 +5322,9 @@
 	  && !VECTOR_MARKED_P (XFONT_SPEC (XCAR (obj)))
 	  && VECTORP (XCDR (obj)))
 	{
-	  ptrdiff_t i, size = ASIZE (XCDR (obj)) & ~ARRAY_MARK_FLAG;
+	  ptrdiff_t i;
 
+	  size = ASIZE (XCDR (obj)) & ~ARRAY_MARK_FLAG;
 	  /* If font-spec is not marked, most likely all font-entities
 	     are not marked too.  But we must be sure that nothing is
 	     marked within OBJ before we really drop it.  */
@@ -5330,7 +5336,11 @@
 	    drop = 1;
 	}
       if (drop)
-	*prev = XCDR (tail);
+	{
+	  *prev = XCDR (tail);
+	  /* Count font-spec and vector of font-entities.  */
+	  FONT_CACHE_SIZE (t) -= (size + 1);
+	}
       else
 	prev = xcdr_addr (tail);
     }
@@ -5351,10 +5361,20 @@
 
       if (CONSP (cache))
 	{
-	  Lisp_Object entry;
+	  eassert (FONT_CACHE_SIZE (t) >= 0);
+	  if (FONT_CACHE_SIZE (t) > FONT_CACHE_THRESHOLD)
+	    {
+	      Lisp_Object entry;
 
-	  for (entry = XCDR (cache); CONSP (entry); entry = XCDR (entry))
-	    XSETCAR (entry, compact_font_cache_entry (XCAR (entry)));
+	      fprintf (stderr, "GC%ld: font cache compaction %ld -> ",
+		       gcs_done, FONT_CACHE_SIZE (t));
+	      for (entry = XCDR (cache); CONSP (entry); entry = XCDR (entry))
+		XSETCAR (entry, compact_font_cache_entry (t, XCAR (entry)));
+	      fprintf (stderr, "%ld\n", FONT_CACHE_SIZE (t));
+	    }
+	  else
+	    fprintf (stderr, "GC%ld: font cache too small (%d <= %d)\n",
+		     gcs_done, FONT_CACHE_SIZE (t), FONT_CACHE_THRESHOLD);
 	}
       mark_object (cache);
     }

=== modified file 'src/font.c'
--- src/font.c	2013-12-10 03:36:36 +0000
+++ src/font.c	2013-12-11 05:35:25 +0000
@@ -2740,6 +2740,8 @@
 	    copy = copy_font_spec (scratch_font_spec);
 	    ASET (copy, FONT_TYPE_INDEX, driver_list->driver->type);
 	    XSETCDR (cache, Fcons (Fcons (copy, val), XCDR (cache)));
+	    /* Count font-spec and a vector of font-entities.  */
+	    FONT_CACHE_SIZE (f->terminal) += ASIZE (val) + 1;
 	  }
 	if (ASIZE (val) > 0
 	    && (need_filtering
@@ -2793,6 +2795,8 @@
 	    copy = copy_font_spec (work);
 	    ASET (copy, FONT_TYPE_INDEX, driver_list->driver->type);
 	    XSETCDR (cache, Fcons (Fcons (copy, entity), XCDR (cache)));
+	    /* Count font-spec and entity.  */
+	    FONT_CACHE_SIZE (f->terminal) += 2;
 	  }
 	if (! NILP (entity))
 	  break;

=== modified file 'src/nsterm.h'
--- src/nsterm.h	2013-12-07 16:48:12 +0000
+++ src/nsterm.h	2013-12-11 05:35:25 +0000
@@ -560,6 +560,9 @@
   /* This is a cons cell of the form (NAME . FONT-LIST-CACHE).  */
   Lisp_Object name_list_element;
 
+  /* Amount of font-entities and font-specs objects in the cache above.  */
+  ptrdiff_t font_cache_size;
+
   /* The number of fonts loaded. */
   int n_fonts;
 

=== modified file 'src/termhooks.h'
--- src/termhooks.h	2013-10-18 12:57:44 +0000
+++ src/termhooks.h	2013-12-11 05:35:25 +0000
@@ -628,12 +628,15 @@
 #if defined (HAVE_X_WINDOWS)
 #define TERMINAL_FONT_CACHE(t)						\
   (t->type == output_x_window ? t->display_info.x->name_list_element : Qnil)
+#define FONT_CACHE_SIZE(t) (t)->display_info.x->font_cache_size
 #elif defined (HAVE_NTGUI)
 #define TERMINAL_FONT_CACHE(t)						\
   (t->type == output_w32 ? t->display_info.w32->name_list_element : Qnil)
+#define FONT_CACHE_SIZE(t) (t)->display_info.w32->font_cache_size
 #elif defined (HAVE_NS)
 #define TERMINAL_FONT_CACHE(t)						\
   (t->type == output_ns ? t->display_info.ns->name_list_element : Qnil)
+#define FONT_CACHE_SIZE(t) (t)->display_info.ns->font_cache_size
 #endif
 
 extern struct terminal *get_terminal (Lisp_Object terminal, bool);

=== modified file 'src/w32term.h'
--- src/w32term.h	2013-12-02 13:35:53 +0000
+++ src/w32term.h	2013-12-11 05:35:25 +0000
@@ -74,6 +74,9 @@
   /* This is a cons cell of the form (NAME . FONT-LIST-CACHE).  */
   Lisp_Object name_list_element;
 
+  /* Amount of font-entities and font-specs objects in the cache above.  */
+  ptrdiff_t font_cache_size;
+
   /* Number of frames that are on this display.  */
   int reference_count;
 

=== modified file 'src/xterm.h'
--- src/xterm.h	2013-12-07 23:04:10 +0000
+++ src/xterm.h	2013-12-11 05:35:25 +0000
@@ -140,6 +140,9 @@
   /* This is a cons cell of the form (NAME . FONT-LIST-CACHE).  */
   Lisp_Object name_list_element;
 
+  /* Amount of font-entities and font-specs objects in the cache above.  */
+  ptrdiff_t font_cache_size;
+
   /* Number of frames that are on this display.  */
   int reference_count;