From: Eli Zaretskii <eliz@gnu.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: cloos@jhcloos.com, emacs-devel@gnu.org
Subject: Re: Memory leak due to bidi?
Date: Wed, 03 Aug 2011 09:30:28 -0400 [thread overview]
Message-ID: <E1QobWm-0000o5-0i@fencepost.gnu.org> (raw)
In-Reply-To: <jwvty9zxpw4.fsf-monnier+emacs@gnu.org> (message from Stefan Monnier on Tue, 02 Aug 2011 21:10:14 -0400)
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 02 Aug 2011 21:10:14 -0400
>
> Let's not forget that it may have nothing to do with bidi. I just
> happened to see bidi_shelve_cache as a "frequent" caller of mmap, but
> according to Eli this is not necessarily abnormal.
I would be the first to be happy if it turns out cache shelving and
unshelving is not the culprit. However, as I still have my doubts, I
instrumented the relevant code as shown in the patch below. With this
patch, you can:
. Put a breakpoint on some rarely-used interactive function (I use
Fredraw_display), invoke it whenever you feel like it, and print
the value of the variable bidi_cache_total_alloc -- it should be
strictly zero, i.e. each allocation should be followed by a
corresponding deallocation before Emacs gets back to its command
loop. If you ever see a non-zero value, let alone a value that
goes up, please proceed with the next item below.
. Put a hardware watchpoint on bidi_cache_total_alloc, with the
following commands:
print bidi_cache_total_alloc
bt 4
continue
Then tell GDB to log all its output:
set logging on
and run Emacs until the value of bidi_cache_total_alloc again
increases. Then send me the logfile (gdb.txt in the directory
where you run GDB), or look into it to find some code that
allocates a buffer, but never frees it.
I am running with this patch for almost 3 hours, and the value of
bidi_cache_total_alloc is strictly zero every time I look at it in the
debugger.
TIA
Here's the patch:
=== modified file 'src/bidi.c'
--- src/bidi.c 2011-08-02 19:16:32 +0000
+++ src/bidi.c 2011-08-03 10:24:32 +0000
@@ -620,12 +620,15 @@ bidi_pop_it (struct bidi_it *bidi_it)
bidi_cache_last_idx = -1;
}
+ptrdiff_t bidi_cache_total_alloc;
+
/* Stash away a copy of the cache and its control variables. */
void *
bidi_shelve_cache (void)
{
unsigned char *databuf;
+ /* Empty cache. */
if (bidi_cache_idx == 0)
return NULL;
@@ -634,6 +637,12 @@ bidi_shelve_cache (void)
+ sizeof (bidi_cache_start_stack)
+ sizeof (bidi_cache_sp) + sizeof (bidi_cache_start)
+ sizeof (bidi_cache_last_idx));
+ bidi_cache_total_alloc +=
+ sizeof (bidi_cache_idx) + bidi_cache_idx * sizeof (struct bidi_it)
+ + sizeof (bidi_cache_start_stack)
+ + sizeof (bidi_cache_sp) + sizeof (bidi_cache_start)
+ + sizeof (bidi_cache_last_idx);
+
memcpy (databuf, &bidi_cache_idx, sizeof (bidi_cache_idx));
memcpy (databuf + sizeof (bidi_cache_idx),
bidi_cache, bidi_cache_idx * sizeof (struct bidi_it));
@@ -659,7 +668,7 @@ bidi_shelve_cache (void)
/* Restore the cache state from a copy stashed away by bidi_shelve_cache. */
void
-bidi_unshelve_cache (void *databuf)
+bidi_unshelve_cache (void *databuf, int just_free)
{
unsigned char *p = databuf;
@@ -672,30 +681,47 @@ bidi_unshelve_cache (void *databuf)
}
else
{
- memcpy (&bidi_cache_idx, p, sizeof (bidi_cache_idx));
- bidi_cache_ensure_space (bidi_cache_idx);
- memcpy (bidi_cache, p + sizeof (bidi_cache_idx),
- bidi_cache_idx * sizeof (struct bidi_it));
- memcpy (bidi_cache_start_stack,
- p + sizeof (bidi_cache_idx)
- + bidi_cache_idx * sizeof (struct bidi_it),
- sizeof (bidi_cache_start_stack));
- memcpy (&bidi_cache_sp,
- p + sizeof (bidi_cache_idx)
- + bidi_cache_idx * sizeof (struct bidi_it)
- + sizeof (bidi_cache_start_stack),
- sizeof (bidi_cache_sp));
- memcpy (&bidi_cache_start,
- p + sizeof (bidi_cache_idx)
- + bidi_cache_idx * sizeof (struct bidi_it)
- + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp),
- sizeof (bidi_cache_start));
- memcpy (&bidi_cache_last_idx,
- p + sizeof (bidi_cache_idx)
- + bidi_cache_idx * sizeof (struct bidi_it)
- + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
- + sizeof (bidi_cache_start),
- sizeof (bidi_cache_last_idx));
+ if (just_free)
+ {
+ ptrdiff_t idx;
+
+ memcpy (&idx, p, sizeof (bidi_cache_idx));
+ bidi_cache_total_alloc -=
+ sizeof (bidi_cache_idx) + idx * sizeof (struct bidi_it)
+ + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
+ + sizeof (bidi_cache_start) + sizeof (bidi_cache_last_idx);
+ }
+ else
+ {
+ memcpy (&bidi_cache_idx, p, sizeof (bidi_cache_idx));
+ bidi_cache_ensure_space (bidi_cache_idx);
+ memcpy (bidi_cache, p + sizeof (bidi_cache_idx),
+ bidi_cache_idx * sizeof (struct bidi_it));
+ memcpy (bidi_cache_start_stack,
+ p + sizeof (bidi_cache_idx)
+ + bidi_cache_idx * sizeof (struct bidi_it),
+ sizeof (bidi_cache_start_stack));
+ memcpy (&bidi_cache_sp,
+ p + sizeof (bidi_cache_idx)
+ + bidi_cache_idx * sizeof (struct bidi_it)
+ + sizeof (bidi_cache_start_stack),
+ sizeof (bidi_cache_sp));
+ memcpy (&bidi_cache_start,
+ p + sizeof (bidi_cache_idx)
+ + bidi_cache_idx * sizeof (struct bidi_it)
+ + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp),
+ sizeof (bidi_cache_start));
+ memcpy (&bidi_cache_last_idx,
+ p + sizeof (bidi_cache_idx)
+ + bidi_cache_idx * sizeof (struct bidi_it)
+ + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
+ + sizeof (bidi_cache_start),
+ sizeof (bidi_cache_last_idx));
+ bidi_cache_total_alloc -=
+ sizeof (bidi_cache_idx) + bidi_cache_idx * sizeof (struct bidi_it)
+ + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
+ + sizeof (bidi_cache_start) + sizeof (bidi_cache_last_idx);
+ }
xfree (p);
}
=== modified file 'src/dispextern.h'
--- src/dispextern.h 2011-08-02 19:16:32 +0000
+++ src/dispextern.h 2011-08-03 10:37:42 +0000
@@ -2978,7 +2978,7 @@ extern int bidi_mirror_char (int);
extern void bidi_push_it (struct bidi_it *);
extern void bidi_pop_it (struct bidi_it *);
extern void *bidi_shelve_cache (void);
-extern void bidi_unshelve_cache (void *);
+extern void bidi_unshelve_cache (void *, int);
/* Defined in xdisp.c */
=== modified file 'src/dispnew.c'
--- src/dispnew.c 2011-07-14 20:40:35 +0000
+++ src/dispnew.c 2011-08-03 10:32:00 +0000
@@ -5282,7 +5282,7 @@ buffer_posn_from_coords (struct window *
argument is ZV to prevent move_it_in_display_line from matching
based on buffer positions. */
move_it_in_display_line (&it, ZV, to_x, MOVE_TO_X);
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
Fset_buffer (old_current_buffer);
=== modified file 'src/indent.c'
--- src/indent.c 2011-07-14 21:35:23 +0000
+++ src/indent.c 2011-08-03 10:32:13 +0000
@@ -2135,7 +2135,7 @@ whether or not it is currently displayed
}
SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
}
if (BUFFERP (old_buffer))
=== modified file 'src/window.c'
--- src/window.c 2011-07-14 17:28:42 +0000
+++ src/window.c 2011-08-03 10:35:05 +0000
@@ -1379,7 +1379,7 @@ if it isn't already recorded. */)
if (it.current_y < it.last_visible_y)
move_it_past_eol (&it);
value = make_number (IT_CHARPOS (it));
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
if (old_buffer)
set_buffer_internal (old_buffer);
@@ -4273,7 +4273,7 @@ window_scroll_pixel_based (Lisp_Object w
}
start = it.current.pos;
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
}
else if (auto_window_vscroll_p)
{
@@ -4417,7 +4417,7 @@ window_scroll_pixel_based (Lisp_Object w
}
else
{
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
if (noerror)
return;
else if (n < 0) /* could happen with empty buffers */
@@ -4434,7 +4434,7 @@ window_scroll_pixel_based (Lisp_Object w
w->vscroll = 0;
else
{
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
if (noerror)
return;
else
@@ -4583,7 +4583,7 @@ window_scroll_pixel_based (Lisp_Object w
SET_PT_BOTH (charpos, bytepos);
}
}
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
}
@@ -5010,7 +5010,7 @@ displayed_window_lines (struct window *w
start_display (&it, w, start);
move_it_vertically (&it, height);
bottom_y = line_bottom_y (&it);
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
/* rms: On a non-window display,
the value of it.vpos at the bottom of the screen
@@ -5116,7 +5116,7 @@ and redisplay normally--don't erase and
move_it_vertically_backward (&it, window_box_height (w) / 2);
charpos = IT_CHARPOS (it);
bytepos = IT_BYTEPOS (it);
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
}
else if (iarg < 0)
{
@@ -5164,7 +5164,7 @@ and redisplay normally--don't erase and
}
if (h <= 0)
{
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
return Qnil;
}
@@ -5187,7 +5187,7 @@ and redisplay normally--don't erase and
charpos = IT_CHARPOS (it);
bytepos = IT_BYTEPOS (it);
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
}
else
{
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2011-08-03 05:24:30 +0000
+++ src/xdisp.c 2011-08-03 10:36:28 +0000
@@ -604,7 +604,7 @@ int current_mode_line_height, current_he
#define SAVE_IT(ITCOPY,ITORIG,CACHE) \
do { \
if (CACHE) \
- xfree (CACHE); \
+ bidi_unshelve_cache (CACHE, 1); \
ITCOPY = ITORIG; \
CACHE = bidi_shelve_cache(); \
} while (0)
@@ -613,7 +613,7 @@ int current_mode_line_height, current_he
do { \
if (pITORIG != pITCOPY) \
*(pITORIG) = *(pITCOPY); \
- bidi_unshelve_cache (CACHE); \
+ bidi_unshelve_cache (CACHE, 0); \
CACHE = NULL; \
} while (0)
@@ -1341,9 +1341,9 @@ pos_visible_p (struct window *w, EMACS_I
*vpos = it2.vpos;
}
else
- xfree (it2data);
+ bidi_unshelve_cache (it2data, 1);
}
- bidi_unshelve_cache (itdata);
+ bidi_unshelve_cache (itdata, 0);
if (old_buffer)
set_buffer_internal_1 (old_buffer);
@@ -2627,7 +2627,7 @@ init_iterator (struct it *it, struct win
it->paragraph_embedding = R2L;
else
it->paragraph_embedding = NEUTRAL_DIR;
- bidi_unshelve_cache (NULL);
+ bidi_unshelve_cache (NULL, 0);
bidi_init_it (charpos, IT_BYTEPOS (*it), FRAME_WINDOW_P (it->f),
&it->bidi_it);
}
@@ -5618,7 +5618,7 @@ back_to_previous_visible_line_start (str
pos = --IT_CHARPOS (it2);
--IT_BYTEPOS (it2);
it2.sp = 0;
- bidi_unshelve_cache (NULL);
+ bidi_unshelve_cache (NULL, 0);
it2.string_from_display_prop_p = 0;
it2.from_disp_prop_p = 0;
if (handle_display_prop (&it2) == HANDLED_RETURN
@@ -5828,7 +5828,7 @@ reseat_1 (struct it *it, struct text_pos
{
bidi_init_it (IT_CHARPOS (*it), IT_BYTEPOS (*it), FRAME_WINDOW_P (it->f),
&it->bidi_it);
- bidi_unshelve_cache (NULL);
+ bidi_unshelve_cache (NULL, 0);
it->bidi_it.paragraph_dir = NEUTRAL_DIR;
it->bidi_it.string.s = NULL;
it->bidi_it.string.lstring = Qnil;
@@ -8096,13 +8096,13 @@ move_it_in_display_line_to (struct it *i
done:
if (atpos_data)
- xfree (atpos_data);
+ bidi_unshelve_cache (atpos_data, 1);
if (atx_data)
- xfree (atx_data);
+ bidi_unshelve_cache (atx_data, 1);
if (wrap_data)
- xfree (wrap_data);
+ bidi_unshelve_cache (wrap_data, 1);
if (ppos_data)
- xfree (ppos_data);
+ bidi_unshelve_cache (ppos_data, 1);
/* Restore the iterator settings altered at the beginning of this
function. */
@@ -8137,7 +8137,7 @@ move_it_in_display_line (struct it *it,
(it, -1, prev_x, MOVE_TO_X);
}
else
- xfree (save_data);
+ bidi_unshelve_cache (save_data, 1);
}
else
move_it_in_display_line_to (it, to_charpos, to_x, op);
@@ -8396,7 +8396,7 @@ move_it_to (struct it *it, EMACS_INT to_
}
if (backup_data)
- xfree (backup_data);
+ bidi_unshelve_cache (backup_data, 1);
TRACE_MOVE ((stderr, "move_it_to: reached %d\n", reached));
}
@@ -8475,7 +8475,7 @@ move_it_vertically_backward (struct it *
RESTORE_IT (it, it, it2data);
if (nlines > 0)
move_it_by_lines (it, nlines);
- xfree (it3data);
+ bidi_unshelve_cache (it3data, 1);
}
else
{
@@ -8671,7 +8671,7 @@ move_it_by_lines (struct it *it, int dvp
if (IT_CHARPOS (*it) >= start_charpos)
RESTORE_IT (it, &it2, it2data);
else
- xfree (it2data);
+ bidi_unshelve_cache (it2data, 1);
}
else
RESTORE_IT (it, it, it2data);
next prev parent reply other threads:[~2011-08-03 13:30 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-02 4:42 Memory leak due to bidi? Eli Zaretskii
2011-08-02 19:12 ` Stefan Monnier
2011-08-02 19:16 ` Lars Magne Ingebrigtsen
2011-08-02 19:24 ` Andreas Schwab
2011-08-02 19:45 ` Lars Magne Ingebrigtsen
2011-08-02 20:12 ` joakim
2011-08-02 20:15 ` Lars Magne Ingebrigtsen
2011-08-02 20:37 ` joakim
2011-08-02 19:42 ` Andreas Röhler
2011-08-02 20:49 ` James Cloos
2011-08-03 1:10 ` Stefan Monnier
2011-08-03 13:30 ` Eli Zaretskii [this message]
2011-08-04 19:22 ` Antoine Levitt
2011-08-05 6:10 ` Eli Zaretskii
2011-08-03 4:29 ` Kenichi Handa
2011-08-03 13:16 ` Óscar Fuentes
2011-08-03 13:47 ` Eli Zaretskii
2011-08-04 16:00 ` Antoine Levitt
2011-08-04 16:30 ` Eli Zaretskii
2011-08-04 17:04 ` Antoine Levitt
2011-08-04 17:48 ` Eli Zaretskii
2011-08-05 3:43 ` Stephen J. Turnbull
2011-08-04 0:48 ` Kenichi Handa
2011-08-03 13:26 ` Andy Moreton
2011-08-03 13:44 ` Eli Zaretskii
2011-08-03 14:18 ` Stefan Monnier
2011-08-03 15:49 ` Andy Moreton
2011-08-03 16:16 ` Eli Zaretskii
2011-08-03 16:11 ` Eli Zaretskii
2011-08-03 16:01 ` Andy Moreton
2011-08-03 16:38 ` Eli Zaretskii
2011-08-04 23:15 ` Andy Moreton
2011-08-04 3:28 ` Stephen J. Turnbull
2011-08-04 5:15 ` Stefan Monnier
2011-08-04 5:19 ` Eli Zaretskii
2011-08-04 9:23 ` Stephen J. Turnbull
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E1QobWm-0000o5-0i@fencepost.gnu.org \
--to=eliz@gnu.org \
--cc=cloos@jhcloos.com \
--cc=emacs-devel@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).