* Initial frame faces
@ 2014-01-24 6:02 Dmitry Antipov
2014-01-24 7:53 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2014-01-24 6:02 UTC (permalink / raw)
To: Emacs development discussions
Do we really need faces on an initial frame? IIUC this frame has nothing
to display and so face information is useless. Moreover, when the first
'real' (TTY or window system) frame is created, initial frame is deleted
but free_face_cache is never called for it, thus creating memory leak:
==19033== 12,088 (40 direct, 12,048 indirect) bytes in 1 blocks are definitely lost in loss record 11,087 of 11,232
==19033== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==19033== by 0x5E1179: xmalloc (alloc.c:677)
==19033== by 0x51B43A: make_face_cache (xfaces.c:4202)
==19033== by 0x512D36: init_frame_faces (xfaces.c:666)
==19033== by 0x428442: make_initial_frame (frame.c:576)
==19033== by 0x4BA3ED: init_window_once (window.c:7096)
==19033== by 0x565B01: main (emacs.c:1201)
We can avoid creating face cache for initial frame with an
extra check in Fdisplay_supports_face_attributes_p, i.e.:
=== modified file 'src/frame.c'
--- src/frame.c 2014-01-11 10:01:01 +0000
+++ src/frame.c 2014-01-24 05:38:56 +0000
@@ -572,9 +572,6 @@
/* The default value of menu-bar-mode is t. */
set_menu_bar_lines (f, make_number (1), Qnil);
- if (!noninteractive)
- init_frame_faces (f);
-
last_nonminibuf_frame = f;
return f;
=== modified file 'src/xfaces.c'
--- src/xfaces.c 2014-01-01 07:43:34 +0000
+++ src/xfaces.c 2014-01-24 05:39:55 +0000
@@ -5046,6 +5046,13 @@
CHECK_LIVE_FRAME (frame);
f = XFRAME (frame);
+ /* Initial frame has no faces. */
+ if (FRAME_INITIAL_P (f))
+ {
+ eassert (FRAME_FACE_CACHE (f) == NULL);
+ return Qnil;
+ }
+
for (i = 0; i < LFACE_VECTOR_SIZE; i++)
attrs[i] = Qunspecified;
merge_face_ref (f, attributes, attrs, 1, 0);
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Initial frame faces
2014-01-24 6:02 Initial frame faces Dmitry Antipov
@ 2014-01-24 7:53 ` Eli Zaretskii
2014-01-24 8:38 ` Dmitry Antipov
0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2014-01-24 7:53 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: emacs-devel
> Date: Fri, 24 Jan 2014 10:02:21 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
>
> Do we really need faces on an initial frame? IIUC this frame has nothing
> to display and so face information is useless. Moreover, when the first
> 'real' (TTY or window system) frame is created, initial frame is deleted
> but free_face_cache is never called for it, thus creating memory leak:
I don't think the initial frame is deleted in TTY sessions. If I set
a breakpoint in delete_frame and run "emacs -Q -nw" under a debugger,
the breakpoint is never hit. Am I missing something?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Initial frame faces
2014-01-24 7:53 ` Eli Zaretskii
@ 2014-01-24 8:38 ` Dmitry Antipov
2014-01-24 9:08 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2014-01-24 8:38 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
On 01/24/2014 11:53 AM, Eli Zaretskii wrote:
> I don't think the initial frame is deleted in TTY sessions. If I set
> a breakpoint in delete_frame and run "emacs -Q -nw" under a debugger,
> the breakpoint is never hit. Am I missing something?
Argh, yes (I checked only the case with window system frames). But why
it is so? I.e. why the logic should be different between TTY and window
system frames? IIUC initial frame is just a stub to provide "something
looks and behaves like a frame" in batch mode or until the first real frame.
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Initial frame faces
2014-01-24 8:38 ` Dmitry Antipov
@ 2014-01-24 9:08 ` Eli Zaretskii
2014-01-27 9:51 ` Dmitry Antipov
0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2014-01-24 9:08 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: emacs-devel
> Date: Fri, 24 Jan 2014 12:38:47 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
>
> Argh, yes (I checked only the case with window system frames). But why
> it is so? I.e. why the logic should be different between TTY and window
> system frames? IIUC initial frame is just a stub to provide "something
> looks and behaves like a frame" in batch mode or until the first real frame.
No, initial frame is not a stub. It is a fully functional TTY frame.
(And what is the difference between a "stub" frame and a "real" frame
anyway?) It is deleted in the GUI session because it is "no longer
needed" after the first GUI frame is created.
I took that last part in quotes, because it sounds like baggage from
the past, when TTY and GUI frames couldn't co-exist in the same
session. So we could, for instance, keep that frame around, and reuse
it when the first TTY frame is requested. Not sure this is wise or
useful, though.
But perhaps we should step back a notch and discuss the original
problem. What problem(s) did you want to solve, exactly?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Initial frame faces
2014-01-24 9:08 ` Eli Zaretskii
@ 2014-01-27 9:51 ` Dmitry Antipov
2014-01-27 16:03 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2014-01-27 9:51 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
On 01/24/2014 01:08 PM, Eli Zaretskii wrote:
> But perhaps we should step back a notch and discuss the original
> problem. What problem(s) did you want to solve, exactly?
The problem is that free_frame_faces is never called when an initial
frame is deleted. With valgrind, this is reported as memory leak:
==19033== 12,088 (40 direct, 12,048 indirect) bytes in 1 blocks are definitely lost in loss record 11,087 of 11,232
==19033== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==19033== by 0x5E1179: xmalloc (alloc.c:677)
==19033== by 0x51B43A: make_face_cache (xfaces.c:4202)
==19033== by 0x512D36: init_frame_faces (xfaces.c:666)
==19033== by 0x428442: make_initial_frame (frame.c:576)
==19033== by 0x4BA3ED: init_window_once (window.c:7096)
==19033== by 0x565B01: main (emacs.c:1201)
IIUC this may be fixed in three ways:
1) If an initial frame is never used to display anything,
we don't need faces on that frame at all.
2) initial_terminal should have a hook to free faces on an initial frame.
3) If each frame always has a face cache, it's reasonable to call
free_frame_faces in delete_frame and not in frame type-specific hooks.
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Initial frame faces
2014-01-27 9:51 ` Dmitry Antipov
@ 2014-01-27 16:03 ` Eli Zaretskii
0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2014-01-27 16:03 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: emacs-devel
> Date: Mon, 27 Jan 2014 13:51:48 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
>
> On 01/24/2014 01:08 PM, Eli Zaretskii wrote:
>
> > But perhaps we should step back a notch and discuss the original
> > problem. What problem(s) did you want to solve, exactly?
>
> The problem is that free_frame_faces is never called when an initial
> frame is deleted. With valgrind, this is reported as memory leak:
>
> ==19033== 12,088 (40 direct, 12,048 indirect) bytes in 1 blocks are definitely lost in loss record 11,087 of 11,232
> ==19033== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==19033== by 0x5E1179: xmalloc (alloc.c:677)
> ==19033== by 0x51B43A: make_face_cache (xfaces.c:4202)
> ==19033== by 0x512D36: init_frame_faces (xfaces.c:666)
> ==19033== by 0x428442: make_initial_frame (frame.c:576)
> ==19033== by 0x4BA3ED: init_window_once (window.c:7096)
> ==19033== by 0x565B01: main (emacs.c:1201)
>
> IIUC this may be fixed in three ways:
>
> 1) If an initial frame is never used to display anything,
> we don't need faces on that frame at all.
>
> 2) initial_terminal should have a hook to free faces on an initial frame.
>
> 3) If each frame always has a face cache, it's reasonable to call
> free_frame_faces in delete_frame and not in frame type-specific hooks.
I think 2) is the best, because it leaves the possibility of having a
different delete_frame_hook for each terminal type (although they seem
to be identical for now). free_frame_faces already defends itself
against a NULL cache (actually, it over-defends itself, as all of its
subroutines check that as well), so it should be safe to call it from
initial_terminal's hook, even if the face cache is not initialized in
some situation.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-27 16:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-24 6:02 Initial frame faces Dmitry Antipov
2014-01-24 7:53 ` Eli Zaretskii
2014-01-24 8:38 ` Dmitry Antipov
2014-01-24 9:08 ` Eli Zaretskii
2014-01-27 9:51 ` Dmitry Antipov
2014-01-27 16:03 ` Eli Zaretskii
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).