From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#37006: 27.0.50; garbage collection not happening after 26de2d42 Date: Tue, 13 Aug 2019 12:32:24 -0700 Organization: UCLA Computer Science Department Message-ID: <0bc956d1-4cf5-a886-1703-49ee0aeb3d58@cs.ucla.edu> References: <5075406D-6DB8-4560-BB64-7198526FCF9F@acm.org> <83h86nu0pq.fsf@gnu.org> <86pnlbphus.fsf@phe.ftfl.ca> <83a7cft8qx.fsf@gnu.org> <868srysb9x.fsf@phe.ftfl.ca> <83wofis508.fsf@gnu.org> <2687613f-ba89-25cf-9517-5311106aef9a@cs.ucla.edu> <83ef1prly5.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------4523E105E1821A76D13A372E" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="35566"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 Cc: jrm@ftfl.ca, mattiase@acm.org, 37006@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Aug 13 21:33:19 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hxcXm-000973-K0 for geb-bug-gnu-emacs@m.gmane.org; Tue, 13 Aug 2019 21:33:18 +0200 Original-Received: from localhost ([::1]:55076 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hxcXl-00020k-Lk for geb-bug-gnu-emacs@m.gmane.org; Tue, 13 Aug 2019 15:33:17 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:52198) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hxcXY-00020A-73 for bug-gnu-emacs@gnu.org; Tue, 13 Aug 2019 15:33:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hxcXW-0004Fm-D9 for bug-gnu-emacs@gnu.org; Tue, 13 Aug 2019 15:33:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:39837) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hxcXW-0004Fc-98 for bug-gnu-emacs@gnu.org; Tue, 13 Aug 2019 15:33:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hxcXW-0006LD-3T for bug-gnu-emacs@gnu.org; Tue, 13 Aug 2019 15:33:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 13 Aug 2019 19:33:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 37006 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 37006-submit@debbugs.gnu.org id=B37006.156572475524309 (code B ref 37006); Tue, 13 Aug 2019 19:33:02 +0000 Original-Received: (at 37006) by debbugs.gnu.org; 13 Aug 2019 19:32:35 +0000 Original-Received: from localhost ([127.0.0.1]:48658 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hxcX5-0006K1-0L for submit@debbugs.gnu.org; Tue, 13 Aug 2019 15:32:35 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:58548) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hxcX2-0006Jn-Cf for 37006@debbugs.gnu.org; Tue, 13 Aug 2019 15:32:33 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 9DF72162732; Tue, 13 Aug 2019 12:32:26 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id UVck_yB_e_Yq; Tue, 13 Aug 2019 12:32:25 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 38800162733; Tue, 13 Aug 2019 12:32:25 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id JvsQf8kSbgcL; Tue, 13 Aug 2019 12:32:25 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id ED71D162732; Tue, 13 Aug 2019 12:32:24 -0700 (PDT) In-Reply-To: <83ef1prly5.fsf@gnu.org> Content-Language: en-US X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:164948 Archived-At: This is a multi-part message in MIME format. --------------4523E105E1821A76D13A372E Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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. --------------4523E105E1821A76D13A372E Content-Type: text/x-patch; name="0001-Let-consing_until_gc-exceed-INTPTR_MAX.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Let-consing_until_gc-exceed-INTPTR_MAX.patch" >From a354736e1dfe5a7e4ddbb1ee7f1373be2b5bbe09 Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 --------------4523E105E1821A76D13A372E Content-Type: text/x-patch; name="0002-Let-consing_until_gc-exceed-EMACS_INT_MAX.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Let-consing_until_gc-exceed-EMACS_INT_MAX.patch" >From b80559be212292d44ce14ca5e94505cab4d9a868 Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 --------------4523E105E1821A76D13A372E Content-Type: text/x-patch; name="0003-Don-t-increase-consing_until_gc-when-out-of-memory.patch" Content-Disposition: attachment; filename*0="0003-Don-t-increase-consing_until_gc-when-out-of-memory.patc"; filename*1="h" Content-Transfer-Encoding: quoted-printable >From f4974d6fe6137f436763998be27afafea9866098 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 13 Aug 2019 12:28:53 -0700 Subject: [PATCH 3/3] =3D?UTF-8?q?Don=3DE2=3D80=3D99t=3D20increase=3D20con= sing=3D5Funtil?=3D =3D?UTF-8?q?=3D5Fgc=3D20when=3D20out=3D20of=3D20memory?=3D MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit * src/alloc.c (memory_full): Don=E2=80=99t 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 =3D Qt; - consing_until_gc =3D memory_full_cons_threshold; + consing_until_gc =3D min (consing_until_gc, memory_full_cons_thres= hold); =20 /* The first time we get here, free the spare memory. */ for (int i =3D 0; i < ARRAYELTS (spare_memory); i++) --=20 2.17.1 --------------4523E105E1821A76D13A372E--