unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Kangas <stefan@marxist.se>
To: Noam Postavsky <npostavs@gmail.com>
Cc: Emerick Rogul <emerick@gmail.com>,
	1501@debbugs.gnu.org, Chong Yidong <cyd@stupidchicken.com>
Subject: bug#1501: Emacs 22 loses undo buffer
Date: Tue, 19 Oct 2021 18:01:43 -0700	[thread overview]
Message-ID: <CADwFkmnS-jA9s=J28-KqUNBnxH0HZTyEnrcN0onnOvPQfp79uw@mail.gmail.com> (raw)
In-Reply-To: <87ef1engij.fsf@gmail.com> (Noam Postavsky's message of "Wed, 21 Aug 2019 21:19:32 -0400")

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

tags 1501 + patch
thanks

Noam Postavsky <npostavs@gmail.com> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> Noam Postavsky <npostavs@gmail.com> writes:
>>
>>> Stefan Kangas <stefan@marxist.se> writes:
>>>
>>> > increase in the memory usage of each undo record, especially when
>>> > using font-lock-mode.  I'm not sure that is a serious problem, since
>>> > memory is only getting cheaper, but it might be worth investigating.
>>> > On the other hand, we could just decide that this is not worth the
>>> > effort and close this as wontfix.
>>>
>>> Hmm, it sounds like the problem might just be due to saving text
>>> properties in the undo records?  If so, maybe a simple fix is to just
>>> drop them (or drop only face and font-lock-face properties).
>>
>> Is it not worth saving also that information?
>
> Definitely not face, since it's overwritten as soon as font-lock runs.
> It's true font-lock-face can sometimes be set manually, though usually
> it's computed by font-lock rules.

This would be fairly simple to do, as in the attached patch.  But I'm
not sure that we should make this change, since both `face' and
`font-lock-face' could be used by a major mode at various times, without
getting automatically re-added by font-lock.

From (info "(elisp) Precalculated Fontification"):

    But if the mode does not use the normal Font Lock machinery, it
    should not set the variable ‘font-lock-defaults’.  In this case the
    ‘face’ property will not be overridden, so using the ‘face’ property
    could work too.

IOW, I'm not sure that the proposed change won't introduce subtle bugs.

Other than that, we have doubled all undo limits in Emacs 27.1, so maybe
that's enough of a fix for now?

Any other opinions?

[-- Attachment #2: 0001-Decrease-size-of-undo-entries.patch --]
[-- Type: text/x-diff, Size: 897 bytes --]

From 7b0fea42f793d7b7f9fa9938b3c99b1f28323531 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Wed, 20 Oct 2021 02:42:31 +0200
Subject: [PATCH] Decrease size of undo entries

* src/undo.c (record_delete): Remove the 'face' property from undo
entries to save space.  (Bug1501)
---
 src/undo.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/undo.c b/src/undo.c
index 2db401ebc7..260cf792c7 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -164,6 +164,13 @@ record_delete (ptrdiff_t beg, Lisp_Object string, bool record_markers)
 {
   Lisp_Object sbeg;
 
+  /* Remove the `face' property to save space.  (Bug1501)  */
+  if (!NILP (string))
+    Fremove_list_of_text_properties (make_fixnum (0),
+				     make_fixnum (SCHARS (string)),
+				     CALLN (Flist, Qface),
+				     string);
+
   if (EQ (BVAR (current_buffer, undo_list), Qt))
     return;
 
-- 
2.30.2


  reply	other threads:[~2021-10-20  1:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-05 16:51 bug#1501: Emacs 22 loses undo buffer Emerick Rogul
2019-08-20 18:36 ` Stefan Kangas
2019-08-20 18:45   ` Noam Postavsky
2019-08-21 14:46     ` Stefan Kangas
2019-08-22  1:19       ` Noam Postavsky
2021-10-20  1:01         ` Stefan Kangas [this message]
2021-10-20 12:05           ` Eli Zaretskii
2021-10-21 20:46             ` Stefan Kangas
  -- strict thread matches above, loose matches on Subject: below --
2008-12-06  4:01 Chong Yidong
2008-12-06 21:02 Emerick Rogul
2008-12-07  1:31 ` Chong Yidong
2008-12-07  2:03   ` Emerick Rogul
2008-12-07  5:36     ` Chong Yidong
2008-12-07  5:48     ` Chong Yidong
2008-12-07 12:36       ` Emerick Rogul
2009-01-27 20:58 Chong Yidong
2009-04-24 22:14 Suresh Kannan

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='CADwFkmnS-jA9s=J28-KqUNBnxH0HZTyEnrcN0onnOvPQfp79uw@mail.gmail.com' \
    --to=stefan@marxist.se \
    --cc=1501@debbugs.gnu.org \
    --cc=cyd@stupidchicken.com \
    --cc=emerick@gmail.com \
    --cc=npostavs@gmail.com \
    /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).