* Re: Memory leak in keyboard variables? @ 2008-12-15 1:26 Kenichi Handa 2008-12-15 3:16 ` Chong Yidong 0 siblings, 1 reply; 11+ messages in thread From: Kenichi Handa @ 2008-12-15 1:26 UTC (permalink / raw) To: emacs-devel > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Memory leak in keyboard variables? 2008-12-15 1:26 Memory leak in keyboard variables? Kenichi Handa @ 2008-12-15 3:16 ` Chong Yidong 2008-12-15 4:06 ` image cache (was: Memory leak in keyboard variables?) Stefan Monnier 2008-12-16 4:31 ` Memory leak in keyboard variables? Kenichi Handa 0 siblings, 2 replies; 11+ messages in thread From: Chong Yidong @ 2008-12-15 3:16 UTC (permalink / raw) To: Kenichi Handa; +Cc: emacs-devel 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); ^ permalink raw reply [flat|nested] 11+ messages in thread
* image cache (was: Memory leak in keyboard variables?) 2008-12-15 3:16 ` Chong Yidong @ 2008-12-15 4:06 ` Stefan Monnier 2008-12-15 4:12 ` Eli Zaretskii 2008-12-16 4:31 ` Memory leak in keyboard variables? Kenichi Handa 1 sibling, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2008-12-15 4:06 UTC (permalink / raw) To: Chong Yidong; +Cc: emacs-devel, Kenichi Handa BTW, does anyone know why each frame has an image cache (even though there's really only one image cache per terminal)? It seems that code could be simplified/improved if the image cache pointers were moved from the frame objects to the terminal objects. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache (was: Memory leak in keyboard variables?) 2008-12-15 4:06 ` image cache (was: Memory leak in keyboard variables?) Stefan Monnier @ 2008-12-15 4:12 ` Eli Zaretskii 2008-12-15 15:19 ` image cache Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2008-12-15 4:12 UTC (permalink / raw) To: Stefan Monnier; +Cc: cyd, handa, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Date: Sun, 14 Dec 2008 23:06:55 -0500 > Cc: emacs-devel@gnu.org, Kenichi Handa <handa@m17n.org> > > > BTW, does anyone know why each frame has an image cache (even though > there's really only one image cache per terminal)? A left-over from before mult-tty? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache 2008-12-15 4:12 ` Eli Zaretskii @ 2008-12-15 15:19 ` Stefan Monnier 2008-12-15 16:08 ` Chong Yidong 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2008-12-15 15:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: cyd, handa, emacs-devel >> BTW, does anyone know why each frame has an image cache (even though >> there's really only one image cache per terminal)? > A left-over from before multi-tty? I don't think it's related. The same "problem" was there in Emacs-22, and Emacs-21. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache 2008-12-15 15:19 ` image cache Stefan Monnier @ 2008-12-15 16:08 ` Chong Yidong 2008-12-15 20:09 ` Stefan Monnier 2008-12-16 1:41 ` Miles Bader 0 siblings, 2 replies; 11+ messages in thread From: Chong Yidong @ 2008-12-15 16:08 UTC (permalink / raw) To: Stefan Monnier; +Cc: Eli Zaretskii, handa, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> BTW, does anyone know why each frame has an image cache (even though >>> there's really only one image cache per terminal)? >> A left-over from before multi-tty? > > I don't think it's related. The same "problem" was there in Emacs-22, > and Emacs-21. Note that fixing requires changing all the functions in image.c that require a frame argument, so that they use a terminal argument instead. Otherwise, if you associate image caches to terminals, you run into problems with terminals that have no frame (e.g. terminals created using x-open-connection), because you can't supply those image functions with a frame argument. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache 2008-12-15 16:08 ` Chong Yidong @ 2008-12-15 20:09 ` Stefan Monnier 2008-12-16 1:41 ` Miles Bader 1 sibling, 0 replies; 11+ messages in thread From: Stefan Monnier @ 2008-12-15 20:09 UTC (permalink / raw) To: Chong Yidong; +Cc: Eli Zaretskii, handa, emacs-devel >>>> BTW, does anyone know why each frame has an image cache (even though >>>> there's really only one image cache per terminal)? >>> A left-over from before multi-tty? >> I don't think it's related. The same "problem" was there in Emacs-22, >> and Emacs-21. > Note that fixing requires changing all the functions in image.c that > require a frame argument, so that they use a terminal argument instead. > Otherwise, if you associate image caches to terminals, you run into > problems with terminals that have no frame (e.g. terminals created using > x-open-connection), because you can't supply those image functions with > a frame argument. Yes, such a change will/would imply a whole bunch of changes, indeed. I'm just wondering what is the reason for having chosen frames rather than terminals when writing the code. Seeing the current state of the code, it appears that terminals would have been a much more natural choice, so either I'm missing something, or something has changed which makes it the natural choice now even tho it wasn't earlier, or the author had some odd notion of "natural" (or maybe didn't even think much about it and just chose it arbitrarily). Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache 2008-12-15 16:08 ` Chong Yidong 2008-12-15 20:09 ` Stefan Monnier @ 2008-12-16 1:41 ` Miles Bader 2008-12-16 4:56 ` Chetan Pandya 1 sibling, 1 reply; 11+ messages in thread From: Miles Bader @ 2008-12-16 1:41 UTC (permalink / raw) To: Chong Yidong; +Cc: Eli Zaretskii, emacs-devel, Stefan Monnier, handa Chong Yidong <cyd@stupidchicken.com> writes: > Note that fixing requires changing all the functions in image.c that > require a frame argument, so that they use a terminal argument instead. > Otherwise, if you associate image caches to terminals, you run into > problems with terminals that have no frame (e.g. terminals created using > x-open-connection), because you can't supply those image functions with > a frame argument. Frameless terminals can't use those functions currently anyway, right? So one could change the current implementation to put the cache in the terminal data structure, but preserve the current interfaces. This would yield more sharing, without losing functionality. -Miles -- You can hack anything you want, with TECO and DDT. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache 2008-12-16 1:41 ` Miles Bader @ 2008-12-16 4:56 ` Chetan Pandya 2008-12-16 7:02 ` Chetan Pandya 0 siblings, 1 reply; 11+ messages in thread From: Chetan Pandya @ 2008-12-16 4:56 UTC (permalink / raw) To: Chong Yidong, Miles Bader Cc: Eli Zaretskii, handa, Stefan Monnier, emacs-devel --- On Tue, 12/16/08, Miles Bader <miles@gnu.org> wrote: > Chong Yidong <cyd@stupidchicken.com> writes: > > Note that fixing requires changing all the functions in image.c that > > require a frame argument, so that they use a terminal argument instead. > > Otherwise, if you associate image caches to terminals, you run into > > problems with terminals that have no frame (e.g. terminals created using > > x-open-connection), because you can't supply those image functions with > > a frame argument. > > Frameless terminals can't use those functions currently > anyway, right? > > So one could change the current implementation to put the > cache in the > terminal data structure, but preserve the current > interfaces. This > would yield more sharing, without losing functionality. Isn't that how it is currently? From frame.h: /* Return a pointer to the image cache of frame F. */ #define FRAME_IMAGE_CACHE(F) ((F)->terminal->image_cache) Chetan > > -Miles > > -- > You can hack anything you want, with TECO and DDT. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: image cache 2008-12-16 4:56 ` Chetan Pandya @ 2008-12-16 7:02 ` Chetan Pandya 0 siblings, 0 replies; 11+ messages in thread From: Chetan Pandya @ 2008-12-16 7:02 UTC (permalink / raw) To: Chong Yidong, Miles Bader Cc: Eli Zaretskii, emacs-devel, Stefan Monnier, handa I looked at this some more and looks like it is not so everywhere. However, there seem to be more problems. It isn't clear how this is organized. For examples images are shared across frames, but there is no reference counting to know when it can be refreshed without affecting other frames and when it cannot. --- On Tue, 12/16/08, Chetan Pandya <pandyacus@sbcglobal.net> wrote: > From: Chetan Pandya <pandyacus@sbcglobal.net> > Subject: Re: image cache > To: "Chong Yidong" <cyd@stupidchicken.com>, "Miles Bader" <miles@gnu.org> > Cc: "Eli Zaretskii" <eliz@gnu.org>, handa@m17n.org, "Stefan Monnier" <monnier@iro.umontreal.ca>, emacs-devel@gnu.org > Date: Tuesday, December 16, 2008, 4:56 AM > --- On Tue, 12/16/08, Miles Bader <miles@gnu.org> > wrote: > > > > Chong Yidong <cyd@stupidchicken.com> writes: > > > Note that fixing requires changing all the > functions in image.c that > > > require a frame argument, so that they use a > terminal argument instead. > > > Otherwise, if you associate image caches to > terminals, you run into > > > problems with terminals that have no frame (e.g. > terminals created using > > > x-open-connection), because you can't supply > those image functions with > > > a frame argument. > > > > Frameless terminals can't use those functions > currently > > anyway, right? > > > > So one could change the current implementation to put > the > > cache in the > > terminal data structure, but preserve the current > > interfaces. This > > would yield more sharing, without losing > functionality. > > Isn't that how it is currently? > > From frame.h: > /* Return a pointer to the image cache of frame F. */ > #define FRAME_IMAGE_CACHE(F) > ((F)->terminal->image_cache) > > Chetan > > > > -Miles > > > > -- > > You can hack anything you want, with TECO and DDT. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Memory leak in keyboard variables? 2008-12-15 3:16 ` Chong Yidong 2008-12-15 4:06 ` image cache (was: Memory leak in keyboard variables?) Stefan Monnier @ 2008-12-16 4:31 ` Kenichi Handa 1 sibling, 0 replies; 11+ messages in thread From: Kenichi Handa @ 2008-12-16 4:31 UTC (permalink / raw) To: Chong Yidong; +Cc: emacs-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-12-16 7:02 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-12-15 1:26 Memory leak in keyboard variables? Kenichi Handa 2008-12-15 3:16 ` Chong Yidong 2008-12-15 4:06 ` image cache (was: Memory leak in keyboard variables?) Stefan Monnier 2008-12-15 4:12 ` Eli Zaretskii 2008-12-15 15:19 ` image cache Stefan Monnier 2008-12-15 16:08 ` Chong Yidong 2008-12-15 20:09 ` Stefan Monnier 2008-12-16 1:41 ` Miles Bader 2008-12-16 4:56 ` Chetan Pandya 2008-12-16 7:02 ` Chetan Pandya 2008-12-16 4:31 ` Memory leak in keyboard variables? Kenichi Handa
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.