bug#38407: 27.0.50; infinite loop with display of large file without newlines

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: psainty@orcon.net.nz,  pieter@vanoostrum.org,  38407@debbugs.gnu.org
> Date: Wed, 04 Dec 2019 10:15:55 +0100
> 
>     Eli> First, which part of SAVE_IT causes this?  I'm guessing it's this
>     Eli> part:
> 
>     Eli>   #define SAVE_IT(ITCOPY, ITORIG, CACHE)     \
>     Eli>     do {                                     \
>     Eli>       if (CACHE)                             \
>     Eli>      bidi_unshelve_cache (CACHE, true);      \
>     Eli>       ITCOPY = ITORIG;                               \
>     Eli>       CACHE = bidi_shelve_cache ();          \  <<<<<<<<<<<<
>     Eli>     } while (false)
> 
> Yes, itʼs bidi_shelve_cache

Thanks for verifying.

>     Eli> And if this guess is also true, then I think the problem is that
>     Eli> databuf + sizeof (bidi_cache_idx) is unaligned on 64-bit systems,
>     Eli> since bidi_cache_idx is an int.
> 
> The '_unaligned_' bit of that memmove function name does not mean
> thatʼs itʼs doing unoptimized unaligned copies: it means it accepts
> unaligned pointers, and aligns them as necessary to enable fast
> copying. Anyway, I made bidi_cache_idx an intptr_t, and it made no
> difference.

OK, thanks.

>     Thread 1 "emacs" hit Breakpoint 3, bidi_shelve_cache () at bidi.c:981
>     981         alloc = (bidi_shelve_header_size
>     $25 = 30860
>     $26 = 71842080
> 
> which means Emacs is copying 70MB of data every time bidi_shelve_cache
> is called, and itʼs called *a lot* in this scenario. Could we not do
> this shelving by pointer-swapping or similar rather than copying?

Not sure I understand what kind of pointer-swapping you had in mind.
We don't swap between 2 buffers here, we save away a snapshot of the
iterator state each time we see a character where a line break can be
made, so that we could restore that state when we exhaust the window's
width.  We must restore the iterator state to continue to the next
visual line, and the bidi cache is an integral part of that state.

We could perhaps lower the cache size limit (see
BIDI_CACHE_MAX_ELTS_PER_SLOT in bidi.c), which would then
proportionally decrease the time for making a copy of the cache.  Or
we could make some non-trivial changes in the logic of
move_it_in_display_line_to (and similar changes in display_line) to
detect when the cache becomes too large, and use a backup algorithm
that doesn't copy it.  But I question the utility of such changes:
they will never get us a speedup like bidi-inhibit-bpa does, and for
the relatively rare use case like this one (extremely long lines in a
JSON file, with some bracketed parts containing R2L text, and the user
activating visual-line-mode on top of that) inhibiting the BPA,
whether via so-long or by the user or some other Lisp, sounds like an
okay solution to me.  If the JSON file has long lines, but no R2L
text, we already have an optimization in bidi.c to avoid having a
large cache; and if visual-line-mode is off, the cache doesn't need to
be copied so frequently.  So only the combination of the two causes
this tremendous slowdown, and bidi-inhibit-bpa solves it better than
any alternative.  WDYT?