bug#11822: 24.1; emacsclient terminal mode captures escape characters as text

[Ken Raeburn <raeburn@permabit.com>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ken Raeburn <raeburn@permabit.com>
>> Cc: 11822@debbugs.gnu.org
>> Date: Tue, 08 Sep 2015 15:54:49 -0400
>> 
>> > I'm still not following you: what do input events have to do with the
>> > need to redisplay or not redisplay some frame(s)?
>> 
>> If we don't need to update a frame, that's great. But I'm also thinking
>> about cases where a frame does need updating but isn't on the
>> currently-used display. Though for it to be important in this context,
>> updating text probably don't matter much(?), just cases where we
>> actually need to stop and wait for a reply, which probably means changes
>> to face definitions like updating the foreground color.
>
> I don't follow: what replies did you have in mind?  Replies from whom
> or what?

Replies from the X server to LookupColor or AllocColor requests.

That's when the round-trip time and the number of round trips become
important. If we're just sending text-drawing requests, and if there's
enough bandwidth or socket buffer space (neither of which is actually
guaranteed), we shouldn't need to wait, and it shouldn't cause any
significant delay for updates to other frames. So once the
global-variable aspects are addressed, the sort of delay I'm dealing
with should only come up for things like changes to colors in faces that
are used on multiple displays.

> Emacs redraws windows on frames that require redrawing because
> something happened that affects how the visible portion of the windows
> look on the glass.  Emacs doesn't consider some changes more important
> than others, nor waits for any replies, when it decides that changes
> to buffers or strings require redisplay of some window.
>
> I don't think the idea of holding off some display updates for
> whatever reasons will fly, because users rightfully expect the display
> to be up to date, unless Emacs is busy computing something.

I didn't mean having them seconds or minutes out of date. What I'm
thinking of -- assuming for the sake of argument that we're still
concerned about types of changes that'll still require changes on
multiple frames even after the global-variable issues are addressed,
like changing the default face's foreground color -- would be something
like updating faces and then content on one frame, then going on to the
next and updating faces and then content, but preempting the rest ASAP
if user input is received. If recompute_basic_faces is called, since it
triggers a lot of round trips, at least one or two possible preemption
points in the middle of that sequence.

Plus more intelligent ordering of frames for updating (see below). And
image handling probably fits in there too somewhere.

If there's no input received during the process, or after the input is
dealt with, all the face computations and display updates should proceed
normally and all frames should become up-to-date.

I was thinking in terms of delays or idle timers (e.g., wait for "idle"
time of 0.0001s then do this next update increment) as a possible simple
and stupid way to implement that, driven by the input-handling part of
the program, but reviewing what I'm learning about Emacs redisplay, it
doesn't really make sense.

>> Would changing sizes for a face cause the face to be recomputed from
>> scratch?
>
> It doesn't in my testing (I tried "C-x C-+").  You can easily try that
> yourself: put a breakpoint on recompute_basic_faces, and see if it
> breaks when you change the face size.

I tried it in the scratch buffer in a new Emacs process. It doesn't call
recompute_basic_faces, but it did call realize_face twice, and
XParseColor and x_alloc_nearest_color_1 each four times. So that's eight
round trips that seem unnecessary as we should already have the color
definitions and allocated color cells.

>> Giving updates to a remote display that's not the currently-active one
>> lower priority than dealing with input on the active display could let
>> us be more responsive on the active display, but at the risk of leaving
>> the other display slightly out of date in occasional cases where it
>> might matter (a second person working at the second display at the same
>> time, both displays mapping to the same physical display via multiple
>> ssh connections, etc).
>
> I'm not sure I understand the practical meaning of "lower priority".
> When the display engine decides that more than one window needs to be
> redisplayed, it iterates through all the frames, one by one, and on
> each frame iterates through all of its windows.  The order of the
> frame traversal is independent of their displays, and it is
> sequential.
>
> Given this general description, what would "lower priority" mean in
> practice?

Reorder the frame traversal. If in some circumstances a full redisplay
process updating multiple frames can be slow (fewer such circumstances
after the work Stefan outlined and has started gets done, but I expect
some cases will remain), and if user input can preempt completion of
redisplay (I see comments indicating it can but don't know precisely how
or how well it works with X), which frames do we want updated first or
more often? Near as I can tell, it's done by the age of the frame, from
newest to oldest, because newer ones are added to Vframe_list at the
front. My comments about the current display vs other displays vs tty
frames are just ideas of what a better heuristic might look like.

If we effectively do face recomputation across all frames needing it as
a separate pass before doing screen updates, though, it might not help.
(For face updates, at least... I haven't traced through the image
handling.)

> And anyway, I don't think we should do anything that could produce
> frames that are not up to date, because we will be crucified by users.
> I can easily envision a use case where frames on the
> currently-inactive display are actively watched by a user, perhaps
> even the same user who types at the currently-active display.

Yep, that's why they should all get updated ASAP if some input doesn't
preempt redisplay. It doesn't argue for order-of-creation processing
though.

> We can justify partially outdated display when Emacs has something to
> do, but we cannot justify that when Emacs is idle.
>
> However, Emacs should refrain from redrawing iconified frames, so one
> possible method of saving some time should be to leave frames on the
> inactive display iconified.  Did you try that, and if so, did that
> help?

I haven't. I tried remotely iconifying it via Lisp code, but Emacs still
thought it was visible. Now that I'm at home, I'll try to leave it
iconified before heading back to work.

>> > Anyway, I think preventing frames from being unnecessarily redisplayed
>> > will bring larger benefits than just avoiding realization of some
>> > faces.
>> 
>> Perhaps so. I think we've got inefficiencies at multiple levels:
>> updating frames that don't need it; updating faces that don't need it
>> (on frames where something does or may need updating); redundant color
>> queries to a display; probably some issues around image handling. Fixing
>> any of them would be an improvement, but addressing more than one is
>> probably better still.
>
> My reading of the discussion and your backtraces indicate that all of
> that stems from a single problem: when we create or update faces, we
> set a global flag that causes faces to be recomputed on all frames.
> This then snowballs into the need to reload colors and redraw all
> frames.

I think that's the worst part for my new-frame-with-multiple-displays
case, but I don't think it's the only area of the code that could be
improved.

I took a look at the calls to x_alloc_nearest_color_1. In creating an
initial frame with "emacs -Q", I get over two hundred calls, and every
one requires a round-trip to the server. But there were only 13 distinct
RGB color values passed. The most popular values passed were these:

     88 x_alloc_nearest_color_1(0000/0000/0000) (white)
     71 x_alloc_nearest_color_1(ffff/ffff/ffff) (black)
     15 x_alloc_nearest_color_1(bfbf/bfbf/bfbf) (grey75)

Then there's XParseColor; over 2300 calls, but only about 9% require
round-trips to the server, the rest using the "#RRGGBB" syntax that gets
parsed locally. I haven't traced which part of the program accounts for
what fraction of the calls.

Eliminating unnecessary cache clearing might reduce these, maybe by a
factor of two or more, but that's still excessive; and 100 round trips
with a 30ms RTT would still contribute 3 seconds to the time needed to
finish setting up the initial frame. Now, if instead we had just *one*
call to XAllocNamedColor for "white", and one for "black", etc....

Ken