* Possible redisplay performance enhancements (revisited)
@ 2006-04-21 9:47 YAMAMOTO Mitsuharu
2006-04-21 12:37 ` Eli Zaretskii
2006-04-21 13:28 ` David Reitter
0 siblings, 2 replies; 6+ messages in thread
From: YAMAMOTO Mitsuharu @ 2006-04-21 9:47 UTC (permalink / raw
Before pretesting, I'd like to revisit some of redisplay performance
issues I've mentioned in this list:
1. Excessive redraw of overlapping/overlapped rows?
http://lists.gnu.org/archive/html/emacs-devel/2005-09/msg00705.html
Possibly-positive response from Kim F. Storm, and
possibly-negative response from David Kastrup.
Now I don't think mode-line/header-line updates affect redraw of
overlapped/overlapping rows, because mode-line/header-line does
not overlap with the other parts.
2. Need to check face_id < BASIC_FACE_ID_SENTINEL in get_*_face_and_encoding?
http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00238.html
No response, but I couldn't find any reason to distinguish
known faces from others with respect to encode_char.
I've been using the following patch for a few months, but couldn't
find any problems. So I'd like to propose installing it to CVS and
see if any problems can be found (if no objections, of course).
They are modifications to the platform-independent part, and some of
you may not notice the difference in performace. But text drawing
with ATSUI on the Mac Carbon port is sensitive to these changes. I
think ATSUI support is ready to be enabled by default once these
issues are solved.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
Index: src/dispnew.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/dispnew.c,v
retrieving revision 1.362
diff -c -r1.362 dispnew.c
*** src/dispnew.c 12 Apr 2006 06:23:11 -0000 1.362
--- src/dispnew.c 21 Apr 2006 08:47:15 -0000
***************
*** 4156,4162 ****
update_window_line (w, MATRIX_ROW_VPOS (mode_line_row,
desired_matrix),
&mouse_face_overwritten_p);
- changed_p = 1;
}
/* Find first enabled row. Optimizations in redisplay_internal
--- 4156,4161 ----
***************
*** 4226,4232 ****
{
header_line_row->y = 0;
update_window_line (w, 0, &mouse_face_overwritten_p);
- changed_p = 1;
}
/* Fix the appearance of overlapping/overlapped rows. */
--- 4225,4230 ----
Index: src/xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1089
diff -c -r1.1089 xdisp.c
*** src/xdisp.c 20 Apr 2006 23:03:03 -0000 1.1089
--- src/xdisp.c 21 Apr 2006 08:47:17 -0000
***************
*** 18495,18502 ****
sure to use a face suitable for unibyte. */
STORE_XCHAR2B (char2b, 0, glyph->u.ch);
}
! else if (glyph->u.ch < 128
! && glyph->face_id < BASIC_FACE_ID_SENTINEL)
{
/* Case of ASCII in a face known to fit ASCII. */
STORE_XCHAR2B (char2b, 0, glyph->u.ch);
--- 18495,18501 ----
sure to use a face suitable for unibyte. */
STORE_XCHAR2B (char2b, 0, glyph->u.ch);
}
! else if (glyph->u.ch < 128)
{
/* Case of ASCII in a face known to fit ASCII. */
STORE_XCHAR2B (char2b, 0, glyph->u.ch);
***************
*** 18897,18903 ****
face_id = FACE_FOR_CHAR (f, face, c);
face = FACE_FROM_ID (f, face_id);
}
! else if (c < 128 && face_id < BASIC_FACE_ID_SENTINEL)
{
/* Case of ASCII in a face known to fit ASCII. */
STORE_XCHAR2B (char2b, 0, c);
--- 18896,18902 ----
face_id = FACE_FOR_CHAR (f, face, c);
face = FACE_FROM_ID (f, face_id);
}
! else if (c < 128)
{
/* Case of ASCII in a face known to fit ASCII. */
STORE_XCHAR2B (char2b, 0, c);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible redisplay performance enhancements (revisited)
2006-04-21 9:47 Possible redisplay performance enhancements (revisited) YAMAMOTO Mitsuharu
@ 2006-04-21 12:37 ` Eli Zaretskii
2006-04-24 7:35 ` Kenichi Handa
2006-04-21 13:28 ` David Reitter
1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2006-04-21 12:37 UTC (permalink / raw
Cc: YAMAMOTO Mitsuharu, emacs-devel
> Date: Fri, 21 Apr 2006 18:47:40 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
>
> 2. Need to check face_id < BASIC_FACE_ID_SENTINEL in get_*_face_and_encoding?
> http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00238.html
>
> No response, but I couldn't find any reason to distinguish
> known faces from others with respect to encode_char.
I think it doesn't want to assume single-byte for unknown faces, but
I'm not sure if this is really needed.
Handa-san, could you please comment on the suggested patch (below)?
> Index: src/xdisp.c
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
> retrieving revision 1.1089
> diff -c -r1.1089 xdisp.c
> *** src/xdisp.c 20 Apr 2006 23:03:03 -0000 1.1089
> --- src/xdisp.c 21 Apr 2006 08:47:17 -0000
> ***************
> *** 18495,18502 ****
> sure to use a face suitable for unibyte. */
> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
> }
> ! else if (glyph->u.ch < 128
> ! && glyph->face_id < BASIC_FACE_ID_SENTINEL)
> {
> /* Case of ASCII in a face known to fit ASCII. */
> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
> --- 18495,18501 ----
> sure to use a face suitable for unibyte. */
> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
> }
> ! else if (glyph->u.ch < 128)
> {
> /* Case of ASCII in a face known to fit ASCII. */
> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
> ***************
> *** 18897,18903 ****
> face_id = FACE_FOR_CHAR (f, face, c);
> face = FACE_FROM_ID (f, face_id);
> }
> ! else if (c < 128 && face_id < BASIC_FACE_ID_SENTINEL)
> {
> /* Case of ASCII in a face known to fit ASCII. */
> STORE_XCHAR2B (char2b, 0, c);
> --- 18896,18902 ----
> face_id = FACE_FOR_CHAR (f, face, c);
> face = FACE_FROM_ID (f, face_id);
> }
> ! else if (c < 128)
> {
> /* Case of ASCII in a face known to fit ASCII. */
> STORE_XCHAR2B (char2b, 0, c);
>
>
> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-devel
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible redisplay performance enhancements (revisited)
2006-04-21 9:47 Possible redisplay performance enhancements (revisited) YAMAMOTO Mitsuharu
2006-04-21 12:37 ` Eli Zaretskii
@ 2006-04-21 13:28 ` David Reitter
1 sibling, 0 replies; 6+ messages in thread
From: David Reitter @ 2006-04-21 13:28 UTC (permalink / raw
On 21 Apr 2006, at 10:47, YAMAMOTO Mitsuharu wrote:
> But text drawing
> with ATSUI on the Mac Carbon port is sensitive to these changes. I
> think ATSUI support is ready to be enabled by default once these
> issues are solved.
I agree. We have this enabled in our distribution, which is used by a
couple of thousand people (among them people working with various
Asian fonts), and I haven't received any complaints.
Thanks for the excellent work.
Another problem, obviously across ports, remains. When many faces
have been defined during the current session, creating new frames
slows down to a full second or so. The problem can be recreated with
packages such as color-theme, where you can flick through a lot of
themes, which AFAIK irreversibly defines a number of faces. Having
several hundred of them is not untypical.
A while ago I traced this down to the fact that all faces get copied
when a frame is created.
I reported the issue here and you made some improvements to the
Carbon port, but the general problems remains.
- D
Ref:
http://lists.gnu.org/archive/html/emacs-devel/2005-08/msg00861.html
http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-08/
msg00168.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible redisplay performance enhancements (revisited)
2006-04-21 12:37 ` Eli Zaretskii
@ 2006-04-24 7:35 ` Kenichi Handa
2006-04-24 9:52 ` YAMAMOTO Mitsuharu
0 siblings, 1 reply; 6+ messages in thread
From: Kenichi Handa @ 2006-04-24 7:35 UTC (permalink / raw
Cc: mituharu, emacs-devel
In article <u8xpzumtt.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
>> Date: Fri, 21 Apr 2006 18:47:40 +0900
>> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
>>
>> 2. Need to check face_id < BASIC_FACE_ID_SENTINEL in get_*_face_and_encoding?
>> http://lists.gnu.org/archive/html/emacs-devel/2006-02/msg00238.html
>>
>> No response, but I couldn't find any reason to distinguish
>> known faces from others with respect to encode_char.
> I think it doesn't want to assume single-byte for unknown faces, but
> I'm not sure if this is really needed.
> Handa-san, could you please comment on the suggested patch (below)?
I don't know why Gerd put that condition in the code. But,
as the code in fontset always uses a font specified for
ASCII for ASCII code (see below), I think the proposed patch
is safe.
static Lisp_Object
fontset_ref (fontset, c)
Lisp_Object fontset;
int c;
{
int charset, c1, c2;
Lisp_Object elt, defalt;
if (SINGLE_BYTE_CHAR_P (c))
return FONTSET_ASCII (fontset);
[...]
}
>> Index: src/xdisp.c
>> ===================================================================
>> RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
>> retrieving revision 1.1089
>> diff -c -r1.1089 xdisp.c
>> *** src/xdisp.c 20 Apr 2006 23:03:03 -0000 1.1089
>> --- src/xdisp.c 21 Apr 2006 08:47:17 -0000
>> ***************
>> *** 18495,18502 ****
>> sure to use a face suitable for unibyte. */
>> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
>> }
>> ! else if (glyph->u.ch < 128
>> ! && glyph->face_id < BASIC_FACE_ID_SENTINEL)
>> {
>> /* Case of ASCII in a face known to fit ASCII. */
>> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
>> --- 18495,18501 ----
>> sure to use a face suitable for unibyte. */
>> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
>> }
>> ! else if (glyph->u.ch < 128)
>> {
>> /* Case of ASCII in a face known to fit ASCII. */
>> STORE_XCHAR2B (char2b, 0, glyph->u.ch);
>> ***************
>> *** 18897,18903 ****
>> face_id = FACE_FOR_CHAR (f, face, c);
>> face = FACE_FROM_ID (f, face_id);
>> }
>> ! else if (c < 128 && face_id < BASIC_FACE_ID_SENTINEL)
>> {
>> /* Case of ASCII in a face known to fit ASCII. */
>> STORE_XCHAR2B (char2b, 0, c);
>> --- 18896,18902 ----
>> face_id = FACE_FOR_CHAR (f, face, c);
>> face = FACE_FROM_ID (f, face_id);
>> }
>> ! else if (c < 128)
>> {
>> /* Case of ASCII in a face known to fit ASCII. */
>> STORE_XCHAR2B (char2b, 0, c);
>>
>>
---
Kenichi Handa
handa@m17n.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible redisplay performance enhancements (revisited)
2006-04-24 7:35 ` Kenichi Handa
@ 2006-04-24 9:52 ` YAMAMOTO Mitsuharu
2006-04-24 12:14 ` Kenichi Handa
0 siblings, 1 reply; 6+ messages in thread
From: YAMAMOTO Mitsuharu @ 2006-04-24 9:52 UTC (permalink / raw
Cc: Eli Zaretskii, emacs-devel
>>>>> On Mon, 24 Apr 2006 16:35:28 +0900, Kenichi Handa <handa@m17n.org> said:
> I don't know why Gerd put that condition in the code. But, as the
> code in fontset always uses a font specified for ASCII for ASCII
> code (see below), I think the proposed patch is safe.
Thanks. It seems that the patch would not make things worse, at least.
I suspect get_glyph_face_and_encoding is not needed if `struct glyph'
contains `char2b' info and it is filled in append_glyph. That would
reduce the number of calls to rif->encode_char and enhance the
performance when CCL is thoroughly used as in Mac. But such a change
might be too drastic to do at this stage.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible redisplay performance enhancements (revisited)
2006-04-24 9:52 ` YAMAMOTO Mitsuharu
@ 2006-04-24 12:14 ` Kenichi Handa
0 siblings, 0 replies; 6+ messages in thread
From: Kenichi Handa @ 2006-04-24 12:14 UTC (permalink / raw
Cc: eliz, emacs-devel
In article <wly7xvpag6.wl%mituharu@math.s.chiba-u.ac.jp>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
> I suspect get_glyph_face_and_encoding is not needed if `struct glyph'
> contains `char2b' info and it is filled in append_glyph. That would
> reduce the number of calls to rif->encode_char and enhance the
> performance when CCL is thoroughly used as in Mac.
I once heard from Gerd that struct glyph is designed to be
short as far as possible. So we must be very careful in
incresing that size. But, it seems that the current member
u.ch is almost useless. I think it is better to store a
glyph code (unsigned) here. But...
> But such a change might be too drastic to do at this
> stage.
I fully agree.
---
Kenichi Handa
handa@m17n.org
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-04-24 12:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-21 9:47 Possible redisplay performance enhancements (revisited) YAMAMOTO Mitsuharu
2006-04-21 12:37 ` Eli Zaretskii
2006-04-24 7:35 ` Kenichi Handa
2006-04-24 9:52 ` YAMAMOTO Mitsuharu
2006-04-24 12:14 ` Kenichi Handa
2006-04-21 13:28 ` David Reitter
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).