Package: emacs;
Reported by: Óscar Fuentes <oscarfv <at> eclipso.eu>
Date: Fri, 8 Aug 2025 16:45:03 UTC
Severity: normal
Found in version 31.0.50
View this message in rfc822 format
From: Pip Cet <pipcet <at> protonmail.com> To: Alan Mackenzie <acm <at> muc.de> Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>, Óscar Fuentes <oscarfv <at> eclipso.eu>, Stefan Monnier <monnier <at> iro.umontreal.ca>, 79200 <at> debbugs.gnu.org Subject: bug#79200: 31.0.50; Duplicated elements for '#<marker at' in buffer-undo-list Date: Sun, 10 Aug 2025 14:56:56 +0000
"Alan Mackenzie" <acm <at> muc.de> writes: > Hello, Gerd. > > On Sun, Aug 10, 2025 at 06:55:45 +0200, Gerd Möllmann wrote: >> Pip Cet <pipcet <at> protonmail.com> writes: > >> >> - record_marker_adjustments undo.c:127 >> >> - record_delete record_delete:187 >> >> + casify_region casify_region:555 >> >> + adjust_after_replace adjust_after_replace:1408 >> >> + replace_range replace_range:1639 >> >> + del_range_2 del_range_2:2030 >> >> + record_change record_change:201 > >> >> where record_marker_adjustments puts basically all present markers in a >> >> buffer's undo list. del_range_2 I would interpret that any text deletion >> >> might do that. > >> > I think it may make more sense to find out who goes around creating >> > markers and not cleaning up after them. But I suspect many places cannot >> > easily be fixed because the markers might be exposed to Lisp. > >> Probably. Good question is how to find these places, especially when >> they are created in Lisp :-/. I think the Lisp part is easy: we can simply capture a backtrace and attach it when creating the marker. From f7a23531886fe458346f13ee08369d153b2de897 Mon Sep 17 00:00:00 2001 From: Pip Cet <pipcet <at> protonmail.com> Date: Sun, 10 Aug 2025 14:53:49 +0000 Subject: [PATCH] store Lisp backtraces for markers --- src/alloc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/alloc.c b/src/alloc.c index 07ca8474bf3..c8426e3a4dc 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -3812,6 +3812,12 @@ DEFUN ("make-marker", Fmake_marker, Smake_marker, 0, 0, 0, p->next = NULL; p->insertion_type = 0; p->need_adjustment = 0; + if (!NILP (Vmarker_backtrace)) + { + Lisp_Object backtrace = make_nil_vector (32); + get_backtrace (XVECTOR (backtrace)->contents, 32); + Fputhash (make_lisp_ptr (p, Lisp_Vectorlike), backtrace, Vmarker_backtrace); + } return make_lisp_ptr (p, Lisp_Vectorlike); } @@ -3836,6 +3842,12 @@ build_marker (struct buffer *buf, ptrdiff_t charpos, ptrdiff_t bytepos) m->need_adjustment = 0; m->next = BUF_MARKERS (buf); BUF_MARKERS (buf) = m; + if (!NILP (Vmarker_backtrace)) + { + Lisp_Object backtrace = make_nil_vector (32); + get_backtrace (XVECTOR (backtrace)->contents, 32); + Fputhash (make_lisp_ptr (m, Lisp_Vectorlike), backtrace, Vmarker_backtrace); + } return make_lisp_ptr (m, Lisp_Vectorlike); } @@ -7461,6 +7473,10 @@ syms_of_alloc (void) If this portion is smaller than `gc-cons-threshold', this is ignored. */); Vgc_cons_percentage = make_float (0.1); + DEFVAR_LISP ("marker-backtrace", Vmarker_backtrace, + doc: /* */); + Vmarker_backtrace = CALLN (Fmake_hash_table, QCtest, Qeq, QCweakness, Qkey); + DEFVAR_INT ("pure-bytes-used", pure_bytes_used, doc: /* No longer used. */); -- 2.50.0 Use like this: (dolist (el buffer-undo-list) (and (consp el) (markerp (car el)) (message "%S %S" (car el) (gethash (car el) marker-backtrace)))) > I've made a start on this (see patch below). It's mainly in the C files, > where I grepped for Fpoint_marker and Fcopy_marker, and tidied up the > uses which didn't tidy up their uses of markers. But also in some Lisp > files, which are directly part of the command loop (simple.el), or called > from it in hooks (jit.el, electric.el). > > Unfortunately, this hasn't helped much. When I type "asdfasdf" into > *scratch* on emacs -Q, then <BACKSPACE>, I still see five point markers > in buffer-undo-list. If I <BACKSPACE> a second time, this adds on a > further ten point markers to b-u-list. > > It's worth mentioning that these markers aren't eq to eachother. At > least, not all of them. I have a strong suspicion that there are > duplicates in the 15 markers in buffer-undo-list after my two deletions. > > It is unclear whether the <BACKSPACE> creates these markers, or just puts > existing markers into buffer-undo-list. Maybe we could do with a > primitive in marker.c which would print out the current buffer's list of > markers. That also sounds like a useful debugging aid! > >> >> I think a garbage-collect with the old GC will clean up the undo list >> >> eventually by calling compact_undo_list. With MPS, that doesn't work. >> >> The cleanup in compact_undo_list cannot be used because MPS's weak >> >> objects and GC work completely differently. > >> > That's my understanding, too. And it means that this probably isn't >> > horribly important to fix on the master branch. On feature/igc, it looks >> > like something has to be done. > >> >> At the moment I'm a bit out of ideas. Waiting for a divine inspiration, >> >> so to speak. > >> > Maybe some markers could be marked at creation time as not being >> > important enough to be recorded in the undo list? I'm not really sure >> > which markers are being recorded and which need to be for undo to work. > > This might be a silly question, but why do we have to put markers into > buffer-undo-list at all? Surely they should "fix themselves" as other > changes in the buffer get undone? I don't know the answer. I assume it's necessary for text removals which result in two markers sharing a position when they didn't previously, so we can undo those properly? >> Might be possible but also requires to find where the markers are >> created, find out if they are nver required for undo, and so on. > > Here's a patch which tidies up several markers. Unfortunately it doesn't > seem to make a massive difference to the problem. Have you tried applying the patch I posted earlier, too? It doesn't appear to be included in your changes, but it made a difference here. This is against feature/igc, but the code looks unchanged from master. From d5cf17f88fbba4cdfc61ebea725e12a2c8b9aa42 Mon Sep 17 00:00:00 2001 From: Pip Cet <pipcet <at> protonmail.com> Date: Sat, 9 Aug 2025 10:32:19 +0000 Subject: [PATCH] Detach markers explicitly so they don't wind up on the undo list * src/xdisp.c (unwind_format_mode_line): Detach markers when we're done with them. --- src/xdisp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/xdisp.c b/src/xdisp.c index 432dd5dceca..2406bb6f3d9 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -13828,6 +13828,7 @@ unwind_format_mode_line (Lisp_Object vector) current_buffer = XBUFFER (buffer); set_point_from_marker (AREF (vector, 11)); + detach_marker (AREF (vector, 11)); ASET (vector, 11, Qnil); current_buffer = cb; } -- 2.50.0 Pip
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.