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

[Eli Zaretskii]

> From: Ken Raeburn <raeburn@permabit.com>
> Cc: 11822@debbugs.gnu.org
> Date: Fri, 11 Sep 2015 02:54:06 -0400
> 
> 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.

It should avoid redisplaying other frames when input is available
after redisplaying the first one.  Your original complaint was (and
still is, as far as I'm concerned) that Emacs was unnecessarily trying
to redisplay frames on another display, which caused undue network
round-trips for communicating with X.  Setting that variable to nil
should at most only redraw the selected frame when user input is
available.  If you perform this experiment when the frame(s) in need
of network communications are on the inactive display, you should see
whether the effect is tangible.  If it isn't, trying to refrain from
displaying frames on other displays, per one of your suggestions, will
not be very effective, perhaps not at all, and we need to look for
other ways to optimize this use case.

> >> >> 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.

Then these calls cannot be avoided with the current design of face
realization.  IOW, creating a single frame where communications with X
are over a slow network will always be slow, unless we radically
change the design and implementation of faces.

> > 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.

??? Doesn't this contradict with your measurements above, where X
calls for even a single frame take too long?  How can we make this
faster without changing how faces are realized?  What am I missing?

> >> > 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.

I learned the hard way to solve problems one at a time, starting with
the one that gives the most benefit.

> >> 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

Then please help me understand why creating a single frame needs so
many color-related calls on X.  I know very little about X GUI
performance, and the person who was our X expert is no longer on
board, sadly.

> 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.

Please tell which calls take the lion's share of time in these
scenarios, and please show the backtraces for those calls.

> >> 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.

I asked back then why there are multiple calls, since the first call
resets the face_change flag.  I asked you to try to figure out which
code sets the flag again.  I'd still appreciate the answer to that.

> 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.

You forget that setting the face_change flag also causes a complete
redrawing of a frame, so avoiding that might produce more benefits.

IOW, until we really measure the difference, we will never know what
is the factor.  It could even be (a disappointing) 1.1.

> 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.

Please show the relevant data.  Images are also cached, AFAIR, so I'd
expect not to see unnecessary calls.

> 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.

That should be part of redesigning how faces are realized.  But you
rejected the idea, so how do you expect this to happen?

> 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.

Out of curiosity: is this all true for the Cairo build as well?  Or
does that build save us these round-trips?