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

[Ken Raeburn]

Eli Zaretskii <eliz@gnu.org> writes:

>> [...preempting redisplay...]
> We had this ability in the past, but we all but deleted it, as it
> seemed not to make redisplay more responsive. [...]

> But it could be that we didn't look at the effect of this when frames
> are displayed via X over slow networks.  So please try experimenting
> with an Emacs before the deletion (I think 24.4 is old enough), and
> see if setting redisplay-dont-pause to nil helps in your case.  If it
> doesn't, then what you suggest above is probably not an idea that will
> yield tangible benefits.

Interesting.

Hmm... looking at the code (24.3.93 is what I have handy), it looks like
update_frame is the interesting point where this is checked, which is
called from redisplay_internal after the point where prepare_menu_bars
currently triggers recompute_basic_faces calls across the frames.

So if I understand this right, enabling this preemption (setting
redisplay-dont-pause to nil) would let new user input preempt redisplay
of some frames, but only after prepare_menu_bars has already caused the
color-related round-trips to happen. That seems to be the slow part, so
I'm not sure what I should expect to see happen differently.

>> >> 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.
>
> For how many frames were XParseColor and x_alloc_nearest_color_1
> called?  If only for the single frame where you've changed the face,
> then it's expected.

In that test I had only one frame.

If I create a second frame also showing *scratch* and shrink the text
size, both frames need updating, and I do see twice as many calls.

If I create a second frame, load a text file into one frame, with no
font-lock highlighting, and shrink the text size for the text file, I
see two pairs of calls (one for white, one for black).

Those are as I'd expect -- once per frame showing that buffer, and only
for the faces affected. So it's only a smaller waste of time, but the
calls are still redundant with information we've already retrieved.

I've read that a tenth of a second is a good rough threshold on response
time between the user feeling that the program is responding immediately
and feeling that it isn't. With a RTT of 30ms, four round trips will
exceed that.

>
> If you are suggesting to be more selective wrt what exactly needs to
> be recomputed during face update, then this will need some analysis
> regarding which parts are more expensive than others, and introduction
> of corresponding flags to signal which face aspects need to be
> recomputed.  Assuming this is even possible without a more or less
> complete rewrite of face-related code (which currently just throws
> away a face and realizes it anew), the relative cost (in terms of
> time) of recomputing each aspects will most probably be different for
> different display back-ends, perhaps even for different network
> bandwidths.  Someone™ should do this research and publish the results,
> before we could start designing a good solution.

I don't think I'd try anything that fancy. Realizing faces from scratch
is probably fine as long as that can be made fast enough in most
reasonable cases.

>> > Given this general description, what would "lower priority" mean in
>> > practice?
>> 
>> Reorder the frame traversal.
>
> Since the goal is to limit redisplay to a single frame, the current
> one, I think this is a moot point.

Limiting redisplay to one frame when possible will go a long way. There
will still be cases that need to update multiple frames (like changing
faces used on multiple frames), but they may be infrequent enough that
we don't need to worry about it.

>> > 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.
>
> We should see about that once the global face recalculation is gone.
>
>> 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.
>
> Once again, it is necessary to figure out which portions of these
> calls is on behalf of frames other than the one being created.  Those
> calls should all but go away after the global effect of face updating
> is eliminated.  Only after that we will see how much of this problem
> remains.

In case I wasn't clear, the numbers above were for creating the initial
frame; there was no other to act on behalf of. Emacs startup and
handling of the initial frame may be special in some ways compared to
creating additional frames once Emacs is up and running, but the startup
speed contributes to the user experience too. I know I'm sort of jumping
around between use cases here, but different use cases are impacted
differently by different aspects of the network handling here.

I've hit other cases that don't involve multiple frames. Tooltip window
popups involves way too many round trips, and highlighting and
un-highlighting parts of the mode line as the mouse is moved through it
can sometimes (not sure what the circumstances are) trigger color
queries on every change.

>> Eliminating unnecessary cache clearing might reduce these, maybe by a
>> factor of two or more, but that's still excessive
>
> What is the estimation of "factor of two or more" is based on?  Why
> not by an order of magnitude, for example?

My email a few days ago with the gdb stack traces showed three expensive
calls to recompute_basic_faces (plus three very cheap ones) in setting
up the initial frame. If that's caused by needlessly deciding twice that
we have to recompute faces we've already computed for the one and only
frame, and that those cases can be identified and fixed, that would
presumably eliminate two of the three sets of LookupColor and AllocColor
requests being issued because of recompute_basic_faces. There are
additional requests coming via other paths (e.g., setting mouse color),
so I don't think we'd get a full factor of three unless they're affected
in exactly the same way. Though, it's possible they could be worse
offenders, too. Three might be a better estimate than two, but I would
be very surprised (but pleased) to get much more than that this way.

There's no other frame at this point for there to be queries generated
for, so any code changes from "recompute on all frames" to "recompute on
the current frame" probably won't change this.

I was also thinking of the round-trips involved in the image (tool-bar
icon) handling, where x_disable_image caused a lot of round trips, but
that's not actually XParseColor and XAllocColor, it's XQueryColors, and
I haven't looked at whether there's redundant work there. Given that
there were nearly 100 round trips, I hope there is, since then we might
be able to eliminate some of them.

>
>> Now, if instead we had just *one* call to XAllocNamedColor for
>> "white", and one for "black", etc....
>
> Why do you think we will have more than that?

The color lookups are currently done for each face independently. If
multiple faces use the same colors, we'll have multiple requests to the
X server for those colors.

Consider: 15 faces times at least 2 colors per face (foreground and
background, plus maybe box, underline, overline, and strike-through),
times 2 round trips per color (LookupColor, assuming not a hex RGB
value, and AllocColor), is at least 60 round trips. At 30 ms per round
trip that's 1.8 seconds. Using XAllocNamedColor instead of
XParseColor+XAllocColor would cut that in half, but it would still be
very noticeable.

Ken