Memory leak in keyboard variables?

Memory leak in keyboard variables?

[Chong Yidong]

I've been looking into the memory leak that occurs when terminal frames
are killed.  From Markus Triska's recipe:

 emacs -nw -f server-start
 for i in {1..100}; do emacsclient -t -e "(save-buffers-kill-terminal)"; done 

I think at least some of this leakage is due to unfreed Lisp objects.
For instance, this code in xterm.el leads to ~ 1000 unfreed conses per
terminal created and destroyed:

   (let ((map (copy-keymap xterm-function-map)))
      ...
      (set-keymap-parent map (keymap-parent input-decode-map))
      (set-keymap-parent input-decode-map map)))

Now, input-decode-map is defined in keyboard.c, using DEFVAR_KBOARD.  It
is a Lisp_Misc_Kboard_Objfwd object, and if I'm not mistaken, such
objects are not garbage-collected.

When the terminal is killed, are its keyboard's Lisp_Misc_Kboard_Objfwd
objects freed?  As far as I can tell, they are not freed.  But I am no
expert in this part of the code, so maybe someone else can clue me in.

Re: Memory leak in keyboard variables?

[Andreas Schwab]

Chong Yidong <cyd@stupidchicken.com> writes:

> Now, input-decode-map is defined in keyboard.c, using DEFVAR_KBOARD.  It
> is a Lisp_Misc_Kboard_Objfwd object, and if I'm not mistaken, such
> objects are not garbage-collected.

All Objfwd types are not real objects on their own, but static markers
to tell the evaluator to look elsewhere for the value.  If the
pointed-to object is still live then it will not be collected, of
course.

> When the terminal is killed, are its keyboard's Lisp_Misc_Kboard_Objfwd
> objects freed?

All such objects are allocated only once before dumping.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Memory leak in keyboard variables?

[Chong Yidong]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

Andreas Schwab <schwab@suse.de> writes:

>> input-decode-map is defined in keyboard.c, using DEFVAR_KBOARD.  It
>> is a Lisp_Misc_Kboard_Objfwd object, and if I'm not mistaken, such
>> objects are not garbage-collected.
>
> All Objfwd types are not real objects on their own, but static markers
> to tell the evaluator to look elsewhere for the value.  If the
> pointed-to object is still live then it will not be collected, of
> course.
>
>> When the terminal is killed, are its keyboard's Lisp_Misc_Kboard_Objfwd
>> objects freed?
>
> All such objects are allocated only once before dumping.

I see.  Some Lisp_Objects associated with this code are definitely being
leaked, though.

After a more rigorous test, I think the problem is not in xterm.el, but
in encoded-kb.el.  I used the following debug code to track how many
conses are used:

*** trunk/lisp/server.el.~1.175.~	2008-11-18 11:53:41.000000000 -0500
--- trunk/lisp/server.el	2008-12-11 09:50:50.000000000 -0500
***************
*** 574,581 ****
--- 574,584 ----
+ (defvar my-gc-cons-tracker nil)
+ 
  (defun server-create-tty-frame (tty type proc)
    (add-to-list 'frame-inherited-parameters 'client)
+   (push (caar (garbage-collect)) my-gc-cons-tracker)
    (let ((frame

With this, I found that most of the unfreed memory is coming from a
single spot in encoded-kbd-setup-keymap (encoded-kb.el:334):

   ((eq (coding-system-type coding) 'utf-8)

    (let ((i #xC0))
      (while (< i 256)
      (define-key keymap
        (vector i) 'encoded-kbd-self-insert-utf-8)
        (setq i (1+ i))))

Attached is a graph of conses used verses number of emacsclient
terminals allocated, over a total of 100 terminal open/close iterations.
With the present code, the number of conses used grows linearly.  If
that section of code is commented out, the curve is almost flat.


[-- Attachment #2: encoded-kbd-leak.png --]
[-- Type: image/png, Size: 5860 bytes --]

[-- Attachment #3: Type: text/plain, Size: 238 bytes --]


So either the way the encoded-kbd map is set up prevents
garbage-collection from working (e.g., it might set up some kind of
circular structure), or there is a bug that prevents us from freeing
keymaps associated with deleted keyboards.

Re: Memory leak in keyboard variables?

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> I see.  Some Lisp_Objects associated with this code are definitely being
> leaked, though.

As it turns out, the majority of the memory leak reported in bug#1504
actually occurs outside Lisp.  Basically, term.c was not freeing the
face cache upon frame deletion, unlike the other terminals.

The Lisp leak I discussed in my earlier message is real, but it is about
an order of magnitude less severe (and remains unsolved).

Re: Memory leak in keyboard variables?

[Markus Triska]

Chong Yidong <cyd@stupidchicken.com> writes:

> As it turns out, the majority of the memory leak reported in bug#1504
> actually occurs outside Lisp.  Basically, term.c was not freeing the
> face cache upon frame deletion, unlike the other terminals.

Please also try emacsclient with "-c" instead of "-t" - there seems to
be a probably different and still quite big leak there as will.

Re: Memory leak in keyboard variables?

[Chong Yidong]

Markus Triska <markus.triska@gmx.at> writes:

>> As it turns out, the majority of the memory leak reported in bug#1504
>> actually occurs outside Lisp.  Basically, term.c was not freeing the
>> face cache upon frame deletion, unlike the other terminals.
>
> Please also try emacsclient with "-c" instead of "-t" - there seems to
> be a probably different and still quite big leak there as will.

One thing I noticed is the xftfont_open is run whenever we call
"emacsclient -c", but xftfont_close is never called.  It appears that
font_open_entity is called to allocate a font object, it is in general
not recorded anywhere.  Could this be part of the problem?

Handa-san, do you have any thoughts?

Re: Memory leak in keyboard variables?

[Chong Yidong]

Markus Triska <markus.triska@gmx.at> writes:

> Please also try emacsclient with "-c" instead of "-t" - there seems to
> be a probably different and still quite big leak there as will.

With the recent fix to font_clear_cache, the leak is reduced to 30-40k
per frame.  This leak seems to be tied to GTK and X toolkits somehow.
It does not appear when Emacs is compiled with --with-x-toolkit=no.

Re: Memory leak in keyboard variables?

[Stephen J. Turnbull]

Chong Yidong writes:
 > Markus Triska <markus.triska@gmx.at> writes:
 > 
 > > Please also try emacsclient with "-c" instead of "-t" - there seems to
 > > be a probably different and still quite big leak there as will.
 > 
 > With the recent fix to font_clear_cache, the leak is reduced to 30-40k
 > per frame.  This leak seems to be tied to GTK and X toolkits somehow.
 > It does not appear when Emacs is compiled with --with-x-toolkit=no.

The toolkits undoubtedly do their own font caching, and probably won't
release the space until Emacs exits.

Re: Memory leak in keyboard variables?

[Chong Yidong]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

> Chong Yidong writes:
>  > Markus Triska <markus.triska@gmx.at> writes:
>  > 
>  > > Please also try emacsclient with "-c" instead of "-t" - there seems to
>  > > be a probably different and still quite big leak there as will.
>  > 
>  > With the recent fix to font_clear_cache, the leak is reduced to 30-40k
>  > per frame.  This leak seems to be tied to GTK and X toolkits somehow.
>  > It does not appear when Emacs is compiled with --with-x-toolkit=no.
>
> The toolkits undoubtedly do their own font caching, and probably won't
> release the space until Emacs exits.

Yes, this is a possibility.

Another data point: the leak occurs when the menu-bar is enabled, but
not when the menu-bar is disabled.  It's not necessary to see the leak
using Emacsclient, as ordinary frame creating/deletion shows it:

(dotimes (i 15)
  (let* ((params '((window-system . x)
                   (menu-bar-lines . 1)
                   (tool-bar-lines . 1)))
         frame)
    (setq frame (x-create-frame params))
    (delete-frame frame)
    (garbage-collect)))

I have not been able to track down the source of this leak within Emacs.
As far as I can tell, the existing menu-bar items allocation functions
(in xmenu.c, menu.c, keyboard.c, and gtkutil.c) free all the memory they
allocate, yet about 10k of memory remains unfreed with each frame
created.

Re: Memory leak in keyboard variables?

[Jan Djärv]

Chong Yidong skrev:
> "Stephen J. Turnbull" <stephen@xemacs.org> writes:
> 
>> Chong Yidong writes:
>>  > Markus Triska <markus.triska@gmx.at> writes:
>>  > 
>>  > > Please also try emacsclient with "-c" instead of "-t" - there seems to
>>  > > be a probably different and still quite big leak there as will.
>>  > 
>>  > With the recent fix to font_clear_cache, the leak is reduced to 30-40k
>>  > per frame.  This leak seems to be tied to GTK and X toolkits somehow.
>>  > It does not appear when Emacs is compiled with --with-x-toolkit=no.
>>
>> The toolkits undoubtedly do their own font caching, and probably won't
>> release the space until Emacs exits.
> 
> Yes, this is a possibility.
> 
> Another data point: the leak occurs when the menu-bar is enabled, but
> not when the menu-bar is disabled.  It's not necessary to see the leak
> using Emacsclient, as ordinary frame creating/deletion shows it:
> 
> (dotimes (i 15)
>   (let* ((params '((window-system . x)
>                    (menu-bar-lines . 1)
>                    (tool-bar-lines . 1)))
>          frame)
>     (setq frame (x-create-frame params))
>     (delete-frame frame)
>     (garbage-collect)))
> 
> I have not been able to track down the source of this leak within Emacs.
> As far as I can tell, the existing menu-bar items allocation functions
> (in xmenu.c, menu.c, keyboard.c, and gtkutil.c) free all the memory they
> allocate, yet about 10k of memory remains unfreed with each frame
> created.
> 

From what I see, the frames aren't garabge collected, and then neither is the
menu bar items in f->menu_bar_vector, which isn't used for the non-toolkit case.

The frame is at least referenced from recent-keys, and maybe one more place
which I haven't found.

Setting f->menu_bar_vector to Qnil when deleting the frame improves the
situation quite a bit, I'm not sure if it totally eliminates the leak.  I've
checked in that change.

	Jan D.

Re: Memory leak in keyboard variables?

[Markus Triska]

Jan Djärv <jan.h.d@swipnet.se> writes:

> Setting f->menu_bar_vector to Qnil when deleting the frame improves
> the situation quite a bit, I'm not sure if it totally eliminates the
> leak. I've checked in that change.

Thank you! emacsclient -t seems not to leak any more. With -c it still
does, at least when Emacs is compiled with Gtk. When I compile Emacs
without X toolkit, the daemon crashes after quitting emacsclient on OSX.
I sent a report about this yesterday, and it has so far failed to show
up. Is there a problem with the tracker?

Re: Memory leak in keyboard variables?

[Dan Nicolaescu]

Jan Djärv <jan.h.d@swipnet.se> writes:

  > Chong Yidong skrev:
  > > "Stephen J. Turnbull" <stephen@xemacs.org> writes:
  > > 
  > >> Chong Yidong writes:
  > >>  > Markus Triska <markus.triska@gmx.at> writes:
  > >>  > 
  > >>  > > Please also try emacsclient with "-c" instead of "-t" - there seems to
  > >>  > > be a probably different and still quite big leak there as will.
  > >>  > 
  > >>  > With the recent fix to font_clear_cache, the leak is reduced to 30-40k
  > >>  > per frame.  This leak seems to be tied to GTK and X toolkits somehow.
  > >>  > It does not appear when Emacs is compiled with --with-x-toolkit=no.
  > >>
  > >> The toolkits undoubtedly do their own font caching, and probably won't
  > >> release the space until Emacs exits.
  > > 
  > > Yes, this is a possibility.
  > > 
  > > Another data point: the leak occurs when the menu-bar is enabled, but
  > > not when the menu-bar is disabled.  It's not necessary to see the leak
  > > using Emacsclient, as ordinary frame creating/deletion shows it:
  > > 
  > > (dotimes (i 15)
  > >   (let* ((params '((window-system . x)
  > >                    (menu-bar-lines . 1)
  > >                    (tool-bar-lines . 1)))
  > >          frame)
  > >     (setq frame (x-create-frame params))
  > >     (delete-frame frame)
  > >     (garbage-collect)))
  > > 
  > > I have not been able to track down the source of this leak within Emacs.
  > > As far as I can tell, the existing menu-bar items allocation functions
  > > (in xmenu.c, menu.c, keyboard.c, and gtkutil.c) free all the memory they
  > > allocate, yet about 10k of memory remains unfreed with each frame
  > > created.
  > > 
  > 
  > From what I see, the frames aren't garabge collected, and then neither is the
  > menu bar items in f->menu_bar_vector, which isn't used for the non-toolkit case.
  > 
  > The frame is at least referenced from recent-keys, and maybe one more place
  > which I haven't found.
  > 
  > Setting f->menu_bar_vector to Qnil when deleting the frame improves the
  > situation quite a bit, I'm not sure if it totally eliminates the leak.  I've
  > checked in that change.

Following that logic, maybe all Lisp_Objects in struct frame need to be set to nil.
I did that:


Index: frame.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/frame.c,v
retrieving revision 1.402
diff -u -3 -p -u -p -r1.402 frame.c
--- frame.c   20 Dec 2008 15:59:50 -0000        1.402
+++ frame.c   20 Dec 2008 17:41:35 -0000
@@ -1632,6 +1628,27 @@ But FORCE inhibits this too.  */)
   kb->Vdefault_minibuffer_frame = Qnil;
     }
 
+  f->name = Qnil;
+  f->icon_name = Qnil;
+  f->title = Qnil;
+  f->focus_frame = Qnil;
+  f->root_window = Qnil;
+  f->selected_window = Qnil;
+  f->minibuffer_window = Qnil;
+  f->param_alist = Qnil;
+  f->scroll_bars = Qnil;
+  f->condemned_scroll_bars = Qnil;
+  f->menu_bar_items = Qnil;
+  f->face_alist = Qnil;
+  f->buffer_predicate = Qnil;
+  f->buffer_list = Qnil;
+  f->buried_buffer_list = Qnil;
+  f->menu_bar_window = Qnil;
+  f->tool_bar_window = Qnil;
+  f->tool_bar_items = Qnil;
+  f->desired_tool_bar_string = Qnil;
+  f->current_tool_bar_string = Qnil;
+
   /* Cause frame titles to update--necessary if we now have just one frame.  */
   update_mode_lines = 1;
 
and now this version of the test code above:

(dotimes (i 15)
  (let* ((params '((window-system . nil)
                   (menu-bar-lines . 1)
                   (tool-bar-lines . 1)))
         frame)
    (setq frame (x-create-frame params))
    (delete-frame frame)
    (with-current-buffer "*scratch*"
      (insert (format "%S\n"
                    (garbage-collect))))))

shows:

((66416 . 11688) (13589 . 0) (688 . 250) 147723 356072 (122 . 9) (455 . 907) (8425 . 3608))
((66423 . 11306) (13589 . 0) (688 . 247) 147717 356093 (122 . 9) (456 . 906) (8424 . 3609))
((66425 . 11304) (13589 . 0) (688 . 247) 147717 356114 (122 . 9) (457 . 905) (8424 . 3609))
((66427 . 11302) (13589 . 0) (688 . 247) 147717 356135 (122 . 9) (458 . 904) (8424 . 3609))
((66429 . 11300) (13589 . 0) (688 . 247) 147717 356156 (122 . 9) (459 . 903) (8424 . 3609))
((66431 . 11298) (13589 . 0) (688 . 247) 147717 356177 (122 . 9) (460 . 902) (8424 . 3609))
((66433 . 11296) (13589 . 0) (688 . 247) 147717 356198 (122 . 9) (461 . 901) (8424 . 3609))
((66435 . 11294) (13589 . 0) (688 . 247) 147717 356219 (122 . 9) (462 . 900) (8424 . 3609))
((66437 . 11292) (13589 . 0) (688 . 247) 147717 356240 (122 . 9) (463 . 899) (8424 . 3609))
((66439 . 11290) (13589 . 0) (688 . 247) 147717 356261 (122 . 9) (464 . 898) (8424 . 3609))
((66441 . 11288) (13589 . 0) (688 . 247) 147717 356282 (122 . 9) (465 . 897) (8424 . 3609))
((66443 . 11286) (13589 . 0) (688 . 247) 147717 356303 (122 . 9) (466 . 896) (8424 . 3609))
((66445 . 11284) (13589 . 0) (688 . 247) 147717 356324 (122 . 9) (467 . 895) (8424 . 3609))
((66447 . 11282) (13589 . 0) (688 . 247) 147717 356345 (122 . 9) (468 . 894) (8424 . 3609))
((66449 . 11280) (13589 . 0) (688 . 247) 147717 356366 (122 . 9) (469 . 893) (8424 . 3609))

before the above change it was showing:

((66496 . 11612) (13589 . 0) (691 . 250) 147758 356390 (123 . 8) (455 . 907) (8431 . 3602))
((66593 . 11390) (13589 . 0) (695 . 243) 147800 356777 (123 . 8) (456 . 906) (8437 . 3596))
((66685 . 11298) (13589 . 0) (699 . 243) 147848 357164 (123 . 8) (457 . 905) (8444 . 3589))
((66777 . 11206) (13589 . 0) (703 . 243) 147896 357551 (123 . 8) (458 . 904) (8451 . 3582))
((66869 . 11114) (13589 . 0) (707 . 243) 147944 357938 (123 . 8) (459 . 903) (8458 . 3575))
((66961 . 11022) (13589 . 0) (711 . 243) 147992 358325 (123 . 8) (460 . 902) (8465 . 3568))
((67053 . 10930) (13589 . 0) (715 . 243) 148040 358712 (123 . 8) (461 . 901) (8472 . 3561))
((67145 . 10838) (13589 . 0) (719 . 243) 148088 359099 (123 . 8) (462 . 900) (8479 . 3554))
((67237 . 10746) (13589 . 0) (723 . 243) 148136 359486 (123 . 8) (463 . 899) (8486 . 3547))
((67329 . 10654) (13589 . 0) (727 . 201) 148184 359873 (123 . 8) (464 . 898) (8493 . 3540))
((67421 . 10562) (13589 . 0) (731 . 201) 148232 360260 (123 . 8) (465 . 897) (8500 . 3533))
((67513 . 10470) (13589 . 0) (735 . 201) 148280 360647 (123 . 8) (466 . 896) (8507 . 3526))
((67605 . 10378) (13589 . 0) (739 . 201) 148328 361034 (123 . 8) (467 . 895) (8514 . 3519))
((67697 . 10286) (13589 . 0) (743 . 201) 148376 361421 (123 . 8) (468 . 894) (8521 . 3512))
((67789 . 10194) (13589 . 0) (747 . 201) 148424 361808 (123 . 8) (469 . 893) (8528 . 3505))

Re: Memory leak in keyboard variables?

[Dan Nicolaescu]

Dan Nicolaescu <dann@ics.uci.edu> writes:

  > Jan Djärv <jan.h.d@swipnet.se> writes:
  > 
  >   > Chong Yidong skrev:
  >   > > "Stephen J. Turnbull" <stephen@xemacs.org> writes:
  >   > > 
  >   > >> Chong Yidong writes:
  >   > >>  > Markus Triska <markus.triska@gmx.at> writes:
  >   > >>  > 
  >   > >>  > > Please also try emacsclient with "-c" instead of "-t" - there seems to
  >   > >>  > > be a probably different and still quite big leak there as will.
  >   > >>  > 
  >   > >>  > With the recent fix to font_clear_cache, the leak is reduced to 30-40k
  >   > >>  > per frame.  This leak seems to be tied to GTK and X toolkits somehow.
  >   > >>  > It does not appear when Emacs is compiled with --with-x-toolkit=no.
  >   > >>
  >   > >> The toolkits undoubtedly do their own font caching, and probably won't
  >   > >> release the space until Emacs exits.
  >   > > 
  >   > > Yes, this is a possibility.
  >   > > 
  >   > > Another data point: the leak occurs when the menu-bar is enabled, but
  >   > > not when the menu-bar is disabled.  It's not necessary to see the leak
  >   > > using Emacsclient, as ordinary frame creating/deletion shows it:
  >   > > 
  >   > > (dotimes (i 15)
  >   > >   (let* ((params '((window-system . x)
  >   > >                    (menu-bar-lines . 1)
  >   > >                    (tool-bar-lines . 1)))
  >   > >          frame)
  >   > >     (setq frame (x-create-frame params))
  >   > >     (delete-frame frame)
  >   > >     (garbage-collect)))
  >   > > 
  >   > > I have not been able to track down the source of this leak within Emacs.
  >   > > As far as I can tell, the existing menu-bar items allocation functions
  >   > > (in xmenu.c, menu.c, keyboard.c, and gtkutil.c) free all the memory they
  >   > > allocate, yet about 10k of memory remains unfreed with each frame
  >   > > created.
  >   > > 
  >   > 
  >   > From what I see, the frames aren't garabge collected, and then neither is the
  >   > menu bar items in f->menu_bar_vector, which isn't used for the non-toolkit case.
  >   > 
  >   > The frame is at least referenced from recent-keys, and maybe one more place
  >   > which I haven't found.
  >   > 
  >   > Setting f->menu_bar_vector to Qnil when deleting the frame improves the
  >   > situation quite a bit, I'm not sure if it totally eliminates the leak.  I've
  >   > checked in that change.
  > 
  > Following that logic, maybe all Lisp_Objects in struct frame need to be set to nil.

Ignore this, Jan's patch is enough... 

Re: Memory leak in keyboard variables?

[Chong Yidong]

Jan Djärv <jan.h.d@swipnet.se> writes:

> From what I see, the frames aren't garabge collected, and then neither
> is the menu bar items in f->menu_bar_vector, which isn't used for the
> non-toolkit case.
>
> The frame is at least referenced from recent-keys, and maybe one more
> place which I haven't found.
>
> Setting f->menu_bar_vector to Qnil when deleting the frame improves
> the situation quite a bit, I'm not sure if it totally eliminates the
> leak.  I've checked in that change.

I see.  Thanks for finding the problem.

Re: Memory leak in keyboard variables?

[Stefan Monnier]

> Now, input-decode-map is defined in keyboard.c, using DEFVAR_KBOARD.  It
> is a Lisp_Misc_Kboard_Objfwd object, and if I'm not mistaken, such
> objects are not garbage-collected.

No, indeed, they're not GC'd: they're static.

> When the terminal is killed, are its keyboard's Lisp_Misc_Kboard_Objfwd
> objects freed?  As far as I can tell, they are not freed.  But I am no
> expert in this part of the code, so maybe someone else can clue me in.

The Lisp_Misc_Kboard_Objfwd only stores the location of the Lisp_Object
slot in the keyboard objects.  So the objects will becomes unreachable
when the keyboard object is freed.
Maybe the keyboard object is not freed when the terminal is freed?


        Stefan

Re: Memory leak in keyboard variables?

[Kenichi Handa]

> One thing I noticed is the xftfont_open is run whenever we
> call "emacsclient -c", but xftfont_close is never called.
> It appears that font_open_entity is called to allocate a
> font object, it is in general not recorded anywhere.

No.  A font object is recorded in FONT_OBJLIST_INDEX of a
font-entity.

> Could this be part of the problem?

The strategy is to record all font-objects in font-entities, and
record all font-entities in a cache of each font-backend.  The caches
are freed when `delete-frame' calls font_update_drivers with
new_drivers as nil through this calling sequence.

font_update_drivers -> font_finish_cache -> font_clear_cache

At that time all font-entities and font-obects are freed.

If there's a memory leak for font objectes, it means that there is a
bug at some code implementing the above strategy.

I don't have a time to tackle this problem at the moment,
but I'll take care of it as soon as possible.

---
Kenichi Handa
handa@m17n.org

Re: Memory leak in keyboard variables?

[Chong Yidong]

Kenichi Handa <handa@m17n.org> writes:

> The strategy is to record all font-objects in font-entities, and
> record all font-entities in a cache of each font-backend.  The caches
> are freed when `delete-frame' calls font_update_drivers with
> new_drivers as nil

Thanks for the explanation.  It was very helpful.

I think the problem is that font_clear_cache is incorrectly written.
For some reason, it assumes that the font cache entries have the form

  (font-spec [entity1 entity2...])

when in fact, they have the form

  (font-spec entity1 entity2...)

The following patch to font_clear_cache frees 60-70k of memory per
terminal.

Do you know why font_clear_cache was written this way, and whether there
could be any other places in the font code that make this incorrect
assumption?

*** trunk/src/font.c.~1.99.~	2008-12-13 10:39:30.000000000 -0500
--- trunk/src/font.c	2008-12-14 22:06:26.000000000 -0500
***************
*** 2651,2671 ****
       struct font_driver *driver;
  {
    Lisp_Object tail, elt;
  
    /* CACHE = (DRIVER-TYPE NUM-FRAMES FONT-CACHE-DATA ...) */
    for (tail = XCDR (XCDR (cache)); CONSP (tail); tail = XCDR (tail))
      {
        elt = XCAR (tail);
!       if (CONSP (elt) && FONT_SPEC_P (XCAR (elt)) && VECTORP (XCDR (elt)))
  	{
! 	  Lisp_Object vec = XCDR (elt);
! 	  int i;
! 
! 	  for (i = 0; i < ASIZE (vec); i++)
  	    {
! 	      Lisp_Object entity = AREF (vec, i);
  
! 	      if (EQ (driver->type, AREF (entity, FONT_TYPE_INDEX)))
  		{
  		  Lisp_Object objlist = AREF (entity, FONT_OBJLIST_INDEX);
  
--- 2651,2671 ----
       struct font_driver *driver;
  {
    Lisp_Object tail, elt;
+   Lisp_Object tail2, entity;
  
    /* CACHE = (DRIVER-TYPE NUM-FRAMES FONT-CACHE-DATA ...) */
    for (tail = XCDR (XCDR (cache)); CONSP (tail); tail = XCDR (tail))
      {
        elt = XCAR (tail);
!       /* elt should have the form (FONT-SPEC FONT-ENTITY ...) */
!       if (CONSP (elt) && FONT_SPEC_P (XCAR (elt)))
  	{
! 	  for (tail2 = XCDR (elt); CONSP (tail2); tail2 = XCDR (tail2))
  	    {
! 	      entity = XCAR (tail2);
  
! 	      if (FONT_ENTITY_P (entity)
! 		  && EQ (driver->type, AREF (entity, FONT_TYPE_INDEX)))
  		{
  		  Lisp_Object objlist = AREF (entity, FONT_OBJLIST_INDEX);
  

Re: Memory leak in keyboard variables?

[Kenichi Handa]

In article <87oczeuo0a.fsf@cyd.mit.edu>, Chong Yidong <cyd@stupidchicken.com> writes:

> I think the problem is that font_clear_cache is incorrectly written.
> For some reason, it assumes that the font cache entries have the form

>   (font-spec [entity1 entity2...])

> when in fact, they have the form

>   (font-spec entity1 entity2...)

> The following patch to font_clear_cache frees 60-70k of memory per
> terminal.

Ah, it seems that your patch is correct.

> Do you know why font_clear_cache was written this way,

In the early version of font-backend codes,
font_driver->list returns a vector.  I changed it to return
a list at sometime, but have forgotten to adjust
font_clear_cache.

> and whether there
> could be any other places in the font code that make this incorrect
> assumption?

I've just read through font.c, but the other places look ok.

---
Kenichi Handa
handa@m17n.org

Re: Memory leak in keyboard variables?

[Chetan Pandya]

Although not related, spotted while looking at the code in font.c: font_update_drivers
       for (list = f->font_driver_list; list; list = list->next)
 	if (! list->on)
-	  list_table[i] = list;
+	  list_table[i++] = list;
       list_table[i] = NULL;
 
Chetan


--- On Mon, 12/15/08, Chong Yidong <cyd@stupidchicken.com> wrote:

> From: Chong Yidong <cyd@stupidchicken.com>
> > The strategy is to record all font-objects in
> font-entities, and
> > record all font-entities in a cache of each
> font-backend.  The caches
> > are freed when `delete-frame' calls
> font_update_drivers with
> > new_drivers as nil
> 
> Thanks for the explanation.  It was very helpful.
> 
> I think the problem is that font_clear_cache is incorrectly
> written.
> For some reason, it assumes that the font cache entries
> have the form
> 
>   (font-spec [entity1 entity2...])
> 
> when in fact, they have the form
> 
>   (font-spec entity1 entity2...)
> 
> The following patch to font_clear_cache frees 60-70k of
> memory per
> terminal.
> 
> Do you know why font_clear_cache was written this way, and
> whether there
> could be any other places in the font code that make this
> incorrect
> assumption?
> 
> *** trunk/src/font.c.~1.99.~	2008-12-13 10:39:30.000000000
> -0500
> --- trunk/src/font.c	2008-12-14 22:06:26.000000000 -0500
> ***************
> *** 2651,2671 ****
>        struct font_driver *driver;
>   {
>     Lisp_Object tail, elt;
>   
>     /* CACHE = (DRIVER-TYPE NUM-FRAMES FONT-CACHE-DATA ...)
> */
>     for (tail = XCDR (XCDR (cache)); CONSP (tail); tail =
> XCDR (tail))
>       {
>         elt = XCAR (tail);
> !       if (CONSP (elt) && FONT_SPEC_P (XCAR (elt))
> && VECTORP (XCDR (elt)))
>   	{
> ! 	  Lisp_Object vec = XCDR (elt);
> ! 	  int i;
> ! 
> ! 	  for (i = 0; i < ASIZE (vec); i++)
>   	    {
> ! 	      Lisp_Object entity = AREF (vec, i);
>   
> ! 	      if (EQ (driver->type, AREF (entity,
> FONT_TYPE_INDEX)))
>   		{
>   		  Lisp_Object objlist = AREF (entity,
> FONT_OBJLIST_INDEX);
>   
> --- 2651,2671 ----
>        struct font_driver *driver;
>   {
>     Lisp_Object tail, elt;
> +   Lisp_Object tail2, entity;
>   
>     /* CACHE = (DRIVER-TYPE NUM-FRAMES FONT-CACHE-DATA ...)
> */
>     for (tail = XCDR (XCDR (cache)); CONSP (tail); tail =
> XCDR (tail))
>       {
>         elt = XCAR (tail);
> !       /* elt should have the form (FONT-SPEC FONT-ENTITY
> ...) */
> !       if (CONSP (elt) && FONT_SPEC_P (XCAR
> (elt)))
>   	{
> ! 	  for (tail2 = XCDR (elt); CONSP (tail2); tail2 = XCDR
> (tail2))
>   	    {
> ! 	      entity = XCAR (tail2);
>   
> ! 	      if (FONT_ENTITY_P (entity)
> ! 		  && EQ (driver->type, AREF (entity,
> FONT_TYPE_INDEX)))
>   		{
>   		  Lisp_Object objlist = AREF (entity,
> FONT_OBJLIST_INDEX);

Re: Memory leak in keyboard variables?

[Chong Yidong]

Chetan Pandya <pandyacus@sbcglobal.net> writes:

> Although not related, spotted while looking at the code in font.c:
> font_update_drivers
>
>        for (list = f->font_driver_list; list; list = list->next)
>  	if (! list->on)
> -	  list_table[i] = list;
> +	  list_table[i++] = list;
>        list_table[i] = NULL;

I've checked in the fix, thank you.