bug#9610: 24.0.90; org-mode: sluggish response and high CPU utilization with large .org files

[Eli Zaretskii <eliz@gnu.org>]

> Date: Tue, 27 Sep 2011 09:47:45 -0400
> From: Steve Revilak <steve@srevilak.net>
> Cc: 9610@debbugs.gnu.org, Bastien Guerry <bzg@gnu.org>, steve@srevilak.net
> 
> My bug report contained two scenarios to produce sluggish response and
> high CPU utilization.  These scenarios were
> 
>   (1) move point to the end of buffer, and start typing
> 
>   (2) Put point in line 1, column zero; press and hold the down (then up)
>   arrows to move point down (then up) the org-mode buffer.

Sorry, I missed the second one.

> Setting bidi-paragraph-direction: left-to-right eliminates the problem
> with scenario (1).  However, it has no effect on scenario (2).

Yes, there was another reason for the slowdown.  I found an
optimization of the new display that solves the problem for me with
your sample Org file.  The fix is committed as revision 105941 on the
trunk.  If you cannot or don't want to build the trunk, the patch is
reproduced below; please try it.  On my machine, the cursor motion is
now as fast as in Emacs 23 with the same file.

I'm closing the bug.  Feel free to reopen if there are any leftovers.

Thanks.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-09-27 08:37:07 +0000
+++ src/ChangeLog	2011-09-27 17:18:31 +0000
@@ -1,3 +1,11 @@
+2011-09-27  Eli Zaretskii  <eliz@gnu.org>
+
+	* xdisp.c (handle_invisible_prop): If invisible text ends on a
+	newline, reseat the iterator instead of bidi-iterating there one
+	character at a time.  (Bug#9610)
+	(BUFFER_POS_REACHED_P, move_it_in_display_line_to): Bail when past
+	TO_CHARPOS if the bidi iterator is at base embedding level.
+
 2011-09-27  Andreas Schwab  <schwab@linux-m68k.org>
 
 	* lread.c (readevalloop): Use correct code for NBSP.

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2011-09-24 16:28:25 +0000
+++ src/xdisp.c	2011-09-27 17:18:31 +0000
@@ -4056,40 +4056,67 @@ handle_invisible_prop (struct it *it)
 	  /* The position newpos is now either ZV or on visible text.  */
 	  if (it->bidi_p && newpos < ZV)
 	    {
-	      /* With bidi iteration, the region of invisible text
-		 could start and/or end in the middle of a non-base
-		 embedding level.  Therefore, we need to skip
-		 invisible text using the bidi iterator, starting at
-		 IT's current position, until we find ourselves
-		 outside the invisible text.  Skipping invisible text
-		 _after_ bidi iteration avoids affecting the visual
-		 order of the displayed text when invisible properties
-		 are added or removed.  */
-	      if (it->bidi_it.first_elt && it->bidi_it.charpos < ZV)
-		{
-		  /* If we were `reseat'ed to a new paragraph,
-		     determine the paragraph base direction.  We need
-		     to do it now because next_element_from_buffer may
-		     not have a chance to do it, if we are going to
-		     skip any text at the beginning, which resets the
-		     FIRST_ELT flag.  */
-		  bidi_paragraph_init (it->paragraph_embedding,
-				       &it->bidi_it, 1);
-		}
-	      do
+	      EMACS_INT bpos = CHAR_TO_BYTE (newpos);
+
+	      if (FETCH_BYTE (bpos) == '\n'
+		  || (newpos > BEGV && FETCH_BYTE (bpos - 1) == '\n'))
 		{
-		  bidi_move_to_visually_next (&it->bidi_it);
+		  /* If the invisible text ends on a newline or the
+		     character after a newline, we can avoid the
+		     costly, character by character, bidi iteration to
+		     newpos, and instead simply reseat the iterator
+		     there.  That's because all bidi reordering
+		     information is tossed at the newline.  This is a
+		     big win for modes that hide complete lines, like
+		     Outline, Org, etc.  (Implementation note: the
+		     call to reseat_1 is necessary, because it signals
+		     to the bidi iterator that it needs to reinit its
+		     internal information when the next element for
+		     display is requested.  */
+		  struct text_pos tpos;
+
+		  SET_TEXT_POS (tpos, newpos, bpos);
+		  reseat_1 (it, tpos, 0);
+		}
+	      else	/* Must use the slow method.  */
+		{
+		  /* With bidi iteration, the region of invisible text
+		     could start and/or end in the middle of a
+		     non-base embedding level.  Therefore, we need to
+		     skip invisible text using the bidi iterator,
+		     starting at IT's current position, until we find
+		     ourselves outside the invisible text.  Skipping
+		     invisible text _after_ bidi iteration avoids
+		     affecting the visual order of the displayed text
+		     when invisible properties are added or
+		     removed.  */
+		  if (it->bidi_it.first_elt && it->bidi_it.charpos < ZV)
+		    {
+		      /* If we were `reseat'ed to a new paragraph,
+			 determine the paragraph base direction.  We
+			 need to do it now because
+			 next_element_from_buffer may not have a
+			 chance to do it, if we are going to skip any
+			 text at the beginning, which resets the
+			 FIRST_ELT flag.  */
+		      bidi_paragraph_init (it->paragraph_embedding,
+					   &it->bidi_it, 1);
+		    }
+		  do
+		    {
+		      bidi_move_to_visually_next (&it->bidi_it);
+		    }
+		  while (it->stop_charpos <= it->bidi_it.charpos
+			 && it->bidi_it.charpos < newpos);
+		  IT_CHARPOS (*it) = it->bidi_it.charpos;
+		  IT_BYTEPOS (*it) = it->bidi_it.bytepos;
+		  /* If we overstepped NEWPOS, record its position in
+		     the iterator, so that we skip invisible text if
+		     later the bidi iteration lands us in the
+		     invisible region again. */
+		  if (IT_CHARPOS (*it) >= newpos)
+		    it->prev_stop = newpos;
 		}
-	      while (it->stop_charpos <= it->bidi_it.charpos
-		     && it->bidi_it.charpos < newpos);
-	      IT_CHARPOS (*it) = it->bidi_it.charpos;
-	      IT_BYTEPOS (*it) = it->bidi_it.bytepos;
-	      /* If we overstepped NEWPOS, record its position in the
-		 iterator, so that we skip invisible text if later the
-		 bidi iteration lands us in the invisible region
-		 again. */
-	      if (IT_CHARPOS (*it) >= newpos)
-		it->prev_stop = newpos;
 	    }
 	  else
 	    {
@@ -7880,7 +7907,9 @@ move_it_in_display_line_to (struct it *i
   ((op & MOVE_TO_POS) != 0					\
    && BUFFERP (it->object)					\
    && (IT_CHARPOS (*it) == to_charpos				\
-       || (!it->bidi_p && IT_CHARPOS (*it) > to_charpos)	\
+       || ((!it->bidi_p						\
+	    || BIDI_AT_BASE_LEVEL (it->bidi_it))		\
+	   && IT_CHARPOS (*it) > to_charpos)			\
        || (it->what == IT_COMPOSITION				\
 	   && ((IT_CHARPOS (*it) > to_charpos			\
 		&& to_charpos >= it->cmp_it.charpos)		\
@@ -7912,7 +7941,13 @@ move_it_in_display_line_to (struct it *i
       if ((op & MOVE_TO_POS) != 0
 	  && BUFFERP (it->object)
 	  && it->method == GET_FROM_BUFFER
-	  && ((!it->bidi_p && IT_CHARPOS (*it) > to_charpos)
+	  && (((!it->bidi_p
+		/* When the iterator is at base embedding level, we
+		   are guaranteed that characters are delivered for
+		   display in strictly increasing order of their
+		   buffer positions.  */
+		|| BIDI_AT_BASE_LEVEL (it->bidi_it))
+	       && IT_CHARPOS (*it) > to_charpos)
 	      || (it->bidi_p
 		  && (prev_method == GET_FROM_IMAGE
 		      || prev_method == GET_FROM_STRETCH