From: Paul Eggert <eggert@cs.ucla.edu>
To: Eli Zaretskii <eliz@gnu.org>
Cc: jrm@ftfl.ca, mattiase@acm.org, 37006@debbugs.gnu.org
Subject: bug#37006: 27.0.50; garbage collection not happening after 26de2d42
Date: Tue, 13 Aug 2019 12:32:24 -0700 [thread overview]
Message-ID: <0bc956d1-4cf5-a886-1703-49ee0aeb3d58@cs.ucla.edu> (raw)
In-Reply-To: <83ef1prly5.fsf@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]
Eli Zaretskii wrote:
> OBJECT_CT_MAX should have the value EMACS_INT_MAX.
Not if EMACS_INT_MAX < INTPTR_MAX, since object counts might overflow in that
case. However, I take your point that consing_until_gc can easily be made to
hold any fixnum value, so I installed the first attached patch. This is to some
extent overkill, since these variables should not be assumed to have this sort
of fine-grained control, but the change is tiny so should be fine.
Come to think of it, the limit should be INTMAX_MAX not EMACS_INT_MAX since
gc-cons-threshold could exceed EMACS_INT_MAX. So I installed the second attached
patch to do that.
>> I don't see why the threshold needs to be recomputed on each maybe_gc call. I
>> suppose we could add a new builtin variable type, so that the threshold could be
>> recomputed whenever GC-related builtin variables are changed; that should do the
>> trick without slowing down maybe_gc.
>
> I don't think I understand what this proposal means in practice. Can
> you elaborate, or show an example?
The idea would be to have a type that is like struct Lisp_Objfwd but with an
extra member, a function to be called whenever the variable is accessed. (Or
perhaps two extra members, a getter and a setter.) This could be useful for
other builtin vars, I suspect.
> How else would you succeed in reacting to the change "soon enough"?
There are other possibilities. We could have a timer, for example.
>>> We must also notice the memory-full condition there.
>>
>> memory_full already does that, no? It sets consing_until_gc.
>
> It sets it to a positive value, so no immediate GC will follow. The
> original code was setting the threshold to a very small value, so GC
> would happen immediately.
Are you talking about the change in commit
2019-07-20T02:40:03Z!eggert@cs.ucla.edu
(26de2d42d0460c5b193456950a568cb04a29dc00)? If so, I'm not quite following, as
the old code did not GC until consing_since_gc > memory_full_cons_threshold. I
expect that the idea was to not thrash doing GCs when memory is full.
I think the code in memory_full which sets
> consing_until_gc should be amended to (a) not raise consing_until_gc
> if the current value is already below memory_full_cons_threshold, and
> (b) probably even set it to the negative of sizeof (struct cons_block)
> so as to cause an immediate GC.
Immediate-GC might cause GC thrashing, no? But (a) makes sense so I installed
the third attached patch.
[-- Attachment #2: 0001-Let-consing_until_gc-exceed-INTPTR_MAX.patch --]
[-- Type: text/x-patch, Size: 4076 bytes --]
From a354736e1dfe5a7e4ddbb1ee7f1373be2b5bbe09 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 13 Aug 2019 12:11:35 -0700
Subject: [PATCH 1/3] Let consing_until_gc exceed INTPTR_MAX
Suggested by Eli Zaretskii (Bug#37006#46).
* src/alloc.c (consing_until_gc): Now of type consing_ct.
All uses changed, so gc-cons-threshold no longer saturates
against OBJECT_CT_MAX.
(object_ct): Move typedef here from lisp.h.
* src/lisp.h (consing_ct, CONSING_CT_MAX): New type and macro.
(OBJECT_CT_MAX): Remove. Replace all uses with CONSING_CT_MAX.
---
src/alloc.c | 21 ++++++++++-----------
src/lisp.h | 10 +++++++---
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/alloc.c b/src/alloc.c
index c7419e2fa5..7bed3f4488 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -224,7 +224,7 @@ struct emacs_globals globals;
/* maybe_gc collects garbage if this goes negative. */
-object_ct consing_until_gc;
+consing_ct consing_until_gc;
#ifdef HAVE_PDUMPER
/* Number of finalizers run: used to loop over GC until we stop
@@ -236,9 +236,10 @@ int number_finalizers_run;
bool gc_in_progress;
-/* System byte counts reported by GC. */
+/* System byte and object counts reported by GC. */
typedef uintptr_t byte_ct;
+typedef intptr_t object_ct;
/* Number of live and free conses etc. */
@@ -2546,7 +2547,7 @@ free_cons (struct Lisp_Cons *ptr)
might incorrectly return non-zero. */
int incr = sizeof *ptr;
if (INT_ADD_WRAPV (consing_until_gc, incr, &consing_until_gc))
- consing_until_gc = OBJECT_CT_MAX;
+ consing_until_gc = CONSING_CT_MAX;
gcstat.total_free_conses++;
}
@@ -5501,7 +5502,7 @@ staticpro (Lisp_Object const *varaddress)
static void
allow_garbage_collection (intmax_t consing)
{
- consing_until_gc = consing - (OBJECT_CT_MAX - consing_until_gc);
+ consing_until_gc = consing - (CONSING_CT_MAX - consing_until_gc);
garbage_collection_inhibited--;
}
@@ -5511,7 +5512,7 @@ inhibit_garbage_collection (void)
ptrdiff_t count = SPECPDL_INDEX ();
record_unwind_protect_intmax (allow_garbage_collection, consing_until_gc);
garbage_collection_inhibited++;
- consing_until_gc = OBJECT_CT_MAX;
+ consing_until_gc = CONSING_CT_MAX;
return count;
}
@@ -5817,7 +5818,7 @@ garbage_collect_1 (struct gcstat *gcst)
/* In case user calls debug_print during GC,
don't let that cause a recursive GC. */
- consing_until_gc = OBJECT_CT_MAX;
+ consing_until_gc = CONSING_CT_MAX;
/* Save what's currently displayed in the echo area. Don't do that
if we are GC'ing because we've run out of memory, since
@@ -5932,19 +5933,17 @@ garbage_collect_1 (struct gcstat *gcst)
consing_until_gc = memory_full_cons_threshold;
else
{
- intptr_t threshold = min (max (GC_DEFAULT_THRESHOLD / 10,
- gc_cons_threshold),
- OBJECT_CT_MAX);
+ consing_ct threshold = max (gc_cons_threshold, GC_DEFAULT_THRESHOLD / 10);
if (FLOATP (Vgc_cons_percentage))
{
double tot = (XFLOAT_DATA (Vgc_cons_percentage)
* total_bytes_of_live_objects ());
if (threshold < tot)
{
- if (tot < OBJECT_CT_MAX)
+ if (tot < CONSING_CT_MAX)
threshold = tot;
else
- threshold = OBJECT_CT_MAX;
+ threshold = CONSING_CT_MAX;
}
}
consing_until_gc = threshold;
diff --git a/src/lisp.h b/src/lisp.h
index 63baab5d63..043f2f738e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3793,9 +3793,13 @@ extern void flush_stack_call_func (void (*func) (void *arg), void *arg);
extern void garbage_collect (void);
extern const char *pending_malloc_warning;
extern Lisp_Object zero_vector;
-typedef intptr_t object_ct; /* Signed type of object counts reported by GC. */
-#define OBJECT_CT_MAX INTPTR_MAX
-extern object_ct consing_until_gc;
+#define CONSING_CT_MAX max (INTPTR_MAX, EMACS_INT_MAX)
+#if CONSING_CT_MAX == INTPTR_MAX
+typedef intptr_t consing_ct;
+#else
+typedef EMACS_INT consing_ct;
+#endif
+extern consing_ct consing_until_gc;
#ifdef HAVE_PDUMPER
extern int number_finalizers_run;
#endif
--
2.17.1
[-- Attachment #3: 0002-Let-consing_until_gc-exceed-EMACS_INT_MAX.patch --]
[-- Type: text/x-patch, Size: 3478 bytes --]
From b80559be212292d44ce14ca5e94505cab4d9a868 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 13 Aug 2019 12:20:40 -0700
Subject: [PATCH 2/3] Let consing_until_gc exceed EMACS_INT_MAX
This builds on the previous patch.
* src/alloc.c (consing_until_gc): Now of type intmax_t,
since gc-cons-threshold can be up to INTMAX_MAX. All uses changed.
* src/lisp.h (CONSING_CT_MAX, consing_ct): Remove.
---
src/alloc.c | 16 ++++++++--------
src/lisp.h | 8 +-------
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/src/alloc.c b/src/alloc.c
index 7bed3f4488..14b0a7b838 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -224,7 +224,7 @@ struct emacs_globals globals;
/* maybe_gc collects garbage if this goes negative. */
-consing_ct consing_until_gc;
+intmax_t consing_until_gc;
#ifdef HAVE_PDUMPER
/* Number of finalizers run: used to loop over GC until we stop
@@ -2547,7 +2547,7 @@ free_cons (struct Lisp_Cons *ptr)
might incorrectly return non-zero. */
int incr = sizeof *ptr;
if (INT_ADD_WRAPV (consing_until_gc, incr, &consing_until_gc))
- consing_until_gc = CONSING_CT_MAX;
+ consing_until_gc = INTMAX_MAX;
gcstat.total_free_conses++;
}
@@ -5502,7 +5502,7 @@ staticpro (Lisp_Object const *varaddress)
static void
allow_garbage_collection (intmax_t consing)
{
- consing_until_gc = consing - (CONSING_CT_MAX - consing_until_gc);
+ consing_until_gc = consing - (INTMAX_MAX - consing_until_gc);
garbage_collection_inhibited--;
}
@@ -5512,7 +5512,7 @@ inhibit_garbage_collection (void)
ptrdiff_t count = SPECPDL_INDEX ();
record_unwind_protect_intmax (allow_garbage_collection, consing_until_gc);
garbage_collection_inhibited++;
- consing_until_gc = CONSING_CT_MAX;
+ consing_until_gc = INTMAX_MAX;
return count;
}
@@ -5818,7 +5818,7 @@ garbage_collect_1 (struct gcstat *gcst)
/* In case user calls debug_print during GC,
don't let that cause a recursive GC. */
- consing_until_gc = CONSING_CT_MAX;
+ consing_until_gc = INTMAX_MAX;
/* Save what's currently displayed in the echo area. Don't do that
if we are GC'ing because we've run out of memory, since
@@ -5933,17 +5933,17 @@ garbage_collect_1 (struct gcstat *gcst)
consing_until_gc = memory_full_cons_threshold;
else
{
- consing_ct threshold = max (gc_cons_threshold, GC_DEFAULT_THRESHOLD / 10);
+ intmax_t threshold = max (gc_cons_threshold, GC_DEFAULT_THRESHOLD / 10);
if (FLOATP (Vgc_cons_percentage))
{
double tot = (XFLOAT_DATA (Vgc_cons_percentage)
* total_bytes_of_live_objects ());
if (threshold < tot)
{
- if (tot < CONSING_CT_MAX)
+ if (tot < INTMAX_MAX)
threshold = tot;
else
- threshold = CONSING_CT_MAX;
+ threshold = INTMAX_MAX;
}
}
consing_until_gc = threshold;
diff --git a/src/lisp.h b/src/lisp.h
index 043f2f738e..0370c52fad 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3793,13 +3793,7 @@ extern void flush_stack_call_func (void (*func) (void *arg), void *arg);
extern void garbage_collect (void);
extern const char *pending_malloc_warning;
extern Lisp_Object zero_vector;
-#define CONSING_CT_MAX max (INTPTR_MAX, EMACS_INT_MAX)
-#if CONSING_CT_MAX == INTPTR_MAX
-typedef intptr_t consing_ct;
-#else
-typedef EMACS_INT consing_ct;
-#endif
-extern consing_ct consing_until_gc;
+extern intmax_t consing_until_gc;
#ifdef HAVE_PDUMPER
extern int number_finalizers_run;
#endif
--
2.17.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Don-t-increase-consing_until_gc-when-out-of-memory.patch --]
[-- Type: text/x-patch; name="0003-Don-t-increase-consing_until_gc-when-out-of-memory.patch", Size: 1063 bytes --]
From f4974d6fe6137f436763998be27afafea9866098 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 13 Aug 2019 12:28:53 -0700
Subject: [PATCH 3/3] =?UTF-8?q?Don=E2=80=99t=20increase=20consing=5Funtil?=
=?UTF-8?q?=5Fgc=20when=20out=20of=20memory?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/alloc.c (memory_full): Don’t increase consing_until_gc.
Suggested by Eli Zaretskii (Bug#37006#46).
---
src/alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/alloc.c b/src/alloc.c
index 14b0a7b838..0548a09cb8 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3866,7 +3866,7 @@ memory_full (size_t nbytes)
if (! enough_free_memory)
{
Vmemory_full = Qt;
- consing_until_gc = memory_full_cons_threshold;
+ consing_until_gc = min (consing_until_gc, memory_full_cons_threshold);
/* The first time we get here, free the spare memory. */
for (int i = 0; i < ARRAYELTS (spare_memory); i++)
--
2.17.1
next prev parent reply other threads:[~2019-08-13 19:32 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <5075406D-6DB8-4560-BB64-7198526FCF9F@acm.org>
2019-08-11 16:23 ` bug#37006: 27.0.50; garbage collection not happening after 26de2d42 Mattias Engdegård
2019-08-11 17:07 ` Eli Zaretskii
[not found] ` <83h86nu0pq.fsf@gnu.org>
[not found] ` <86pnlbphus.fsf@phe.ftfl.ca>
2019-08-12 2:31 ` Eli Zaretskii
2019-08-12 14:34 ` Joseph Mingrone
2019-08-12 16:49 ` Eli Zaretskii
2019-08-12 17:00 ` Mattias Engdegård
2019-08-13 15:37 ` Eli Zaretskii
2019-08-13 16:48 ` Mattias Engdegård
2019-08-13 17:04 ` Eli Zaretskii
2019-08-13 17:29 ` Mattias Engdegård
2019-08-13 17:21 ` Paul Eggert
2019-08-13 17:53 ` Eli Zaretskii
2019-08-13 19:32 ` Paul Eggert [this message]
2019-08-14 16:06 ` Eli Zaretskii
2019-08-15 1:37 ` Paul Eggert
2019-08-15 14:17 ` Eli Zaretskii
2019-08-15 18:51 ` Paul Eggert
2019-08-15 19:34 ` Eli Zaretskii
2019-09-14 7:51 ` Paul Eggert
2019-09-14 8:30 ` Eli Zaretskii
2019-08-11 12:39 Joseph Mingrone
2019-08-11 15:13 ` Eli Zaretskii
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0bc956d1-4cf5-a886-1703-49ee0aeb3d58@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=37006@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=jrm@ftfl.ca \
--cc=mattiase@acm.org \
/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 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.