Re: Display slowness that is painful

[storm@cua.dk (Kim F. Storm)]

Chong Yidong <cyd@stupidchicken.com> writes:

> "Richard M. Stallman" <rms@gnu.org> writes:
>
>>     This is *really* not the time to make changes in redisplay.
>>
>> Yes it is.  Even if we had started the pretest already,
>> we would fix this bug.  Or what are pretests for?

I don't know -- it's been so long since I participated in one that I
have forgot :-)

> Managers of successful software projects often make the conscious
> choice to postphone resolving bugs whose "fixes" can introduce even
> more serious bugs while conferring limited benefits.  (And they seem
> to be able to actually make releases...)

Yes, that is what "management" is all about -- making decisions when
"enough is enough".

> In this particular case, having looked through the relevant redisplay
> code, I don't see any apparent code stupidity going on.  It's simply
> the case that, in this file, the first "newline" takes place 340,000
> characters in.

Good catch!!

That can definitely cause problems for redisplay as it does search for
line beginnings/ends quite a lot, but usually the penalty is neglible.

>                 The normally negligible delay from displaying glyphs
> in octal format (v.s. unibyte-display-via-language-environment) is
> magnified by this amount.  The redisplay engine has to scroll through
> the entire "line" to check for overlays, text properties, etc., and
> this seems to be unavoidable.

I tried to create two files like this:

emacs -q

C-x C-f file1.txt RET RET C-p ESC 3 4 0 0 0 0 x

C-x C-f file2.txt RET RET C-p ESC 3 4 0 0 0 0 C-q 2 1 2 RET

and indeed doing C-p C-n (or C-a C-e) in the second buffer is
significantly slower than in the first buffer.

Ok, one might expect it to be 4 times slower, but it is (feels) much
slower than that (perhaps 8 times slower).

It seems to be because it needs to merge_face with the escape glyph
for each character -- and that takes extra time.  Below is a patch to
speed up the processing for this specific part of the problem.  With
the patch, redisplay time for file2 seems to be approx 4 x redisplay time
for file1.



*** xdisp.c	24 Jan 2006 09:30:15 +0100	1.1073
--- xdisp.c	02 Feb 2006 11:33:04 +0100	
***************
*** 5329,5334 ****
--- 5329,5338 ----
     display element from the current position of IT.  Value is zero if
     end of buffer (or C string) is reached.  */
  
+ static struct frame *last_escape_glyph_frame = NULL;
+ static unsigned last_escape_glyph_face_id = (1 << FACE_ID_BITS);
+ static int last_escape_glyph_merged_face_id = 0;
+ 
  int
  get_next_display_element (it)
       struct it *it;
***************
*** 5445,5455 ****
--- 5449,5467 ----
  		       face_id = merge_faces (it->f, Qt, lface_id,
  					      it->face_id);
  		    }
+ 		  else if (it->f == last_escape_glyph_frame
+ 			   && it->face_id == last_escape_glyph_face_id)
+ 		    {
+ 		      face_id = last_escape_glyph_merged_face_id;
+ 		    }
  		  else
  		    {
  		      /* Merge the escape-glyph face into the current face.  */
  		      face_id = merge_faces (it->f, Qescape_glyph, 0,
  					     it->face_id);
+ 		      last_escape_glyph_frame = it->f;
+ 		      last_escape_glyph_face_id = it->face_id;
+ 		      last_escape_glyph_merged_face_id = face_id;
  		    }
  
  		  XSETINT (it->ctl_chars[0], g);
***************
*** 5496,5506 ****
--- 5508,5526 ----
  		  face_id = merge_faces (it->f, Qt, lface_id,
  					 it->face_id);
  		}
+ 	      else if (it->f == last_escape_glyph_frame
+ 		       && it->face_id == last_escape_glyph_face_id)
+ 		{
+ 		  face_id = last_escape_glyph_merged_face_id;
+ 		}
  	      else
  		{
  		  /* Merge the escape-glyph face into the current face.  */
  		  face_id = merge_faces (it->f, Qescape_glyph, 0,
  					 it->face_id);
+ 		  last_escape_glyph_frame = it->f;
+ 		  last_escape_glyph_face_id = it->face_id;
+ 		  last_escape_glyph_merged_face_id = face_id;
  		}
  
  	      /* Handle soft hyphens in the mode where they only get
***************
*** 10545,10550 ****
--- 10565,10572 ----
   retry:
    pause = 0;
    reconsider_clip_changes (w, current_buffer);
+   last_escape_glyph_frame = NULL;
+   last_escape_glyph_face_id = (1 << FACE_ID_BITS);
  
    /* If new fonts have been loaded that make a glyph matrix adjustment
       necessary, do it.  */



--
Kim F. Storm <storm@cua.dk> http://www.cua.dk