From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: Failing to GC killed buffers considered harmful Date: Mon, 30 Mar 2020 14:32:30 -0400 Message-ID: References: <838sjj5jg9.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="56133"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Eli Zaretskii , Daniel Colascione , emacs-devel@gnu.org To: Pip Cet Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Mar 30 20:33:55 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jIzER-000EUL-AF for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Mar 2020 20:33:55 +0200 Original-Received: from localhost ([::1]:54786 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jIzEQ-000815-8r for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Mar 2020 14:33:54 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45647) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jIzDG-0006II-Av for emacs-devel@gnu.org; Mon, 30 Mar 2020 14:32:47 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jIzDC-0002dM-RT for emacs-devel@gnu.org; Mon, 30 Mar 2020 14:32:41 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:58754) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jIzD9-0002ZO-Os; Mon, 30 Mar 2020 14:32:35 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 90B8C100596; Mon, 30 Mar 2020 14:32:34 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 37EFA100473; Mon, 30 Mar 2020 14:32:32 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1585593152; bh=1zKquHaj/n05JvHv9L3fE92pCg3roQt016R21wpY0DQ=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=YQPEs5njnxoqI0tDxhMcCpbrn98shAPi+BBK/MDuE05dTmBzQq58utaCdrAYombsD tx7csCDPnI/Vzf9ntuspKxVv/rXUqEiM3EzvV5UMU2eGvNPPsOVAbxZqusUN/M4pOj BDBlvO3gvTOE37kitoPrhsuQcwgtwpO3TVs5u+JItEV+0aGuiUWvfKguaAtzSaVtHo CBfEwTu2J/I567ML02hOtCPrFsX5lWr9/R0UcVy1oZoG1DQh8L2JfXwE1Vqvy6Ru9d lNdV9CGbu4yJ/pXqUr1viLxcrVgbHTPrZHBQF6/VptDbSKJI7RO/PBd47FT+MixZHb VQLnenYQ/UfvQ== Original-Received: from alfajor (unknown [104.247.241.114]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 97B89120790; Mon, 30 Mar 2020 14:32:31 -0400 (EDT) In-Reply-To: (Stefan Monnier's message of "Mon, 30 Mar 2020 13:02:25 -0400") X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 132.204.25.50 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:246051 Archived-At: > FWIW, maybe we should get rid of all_buffers. > AFAICT the only reason we need it is to unmark the buffers during the > sweep phase, and we could allocate buffers like any other > pseudovector instead. The patch below seems to work. The only worrisome part I think is that `live_buffer_holding` allowed pointers into buffer objects, whereas `live_vector_p` only treats pointers to the beginning of the object as a valid reference. Not sure why buffers would be more likely to have valid pointers into them (and can't remember discussions about that either), so I assume the difference was not important (it just happened to be easier to support that for buffers). Any objection? Stefan diff --git a/src/alloc.c b/src/alloc.c index eed73bcdc4..a5e6ad038f 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -432,7 +432,6 @@ no_sanitize_memcpy (void *dest, void const *src, size_t size) enum mem_type { MEM_TYPE_NON_LISP, - MEM_TYPE_BUFFER, MEM_TYPE_CONS, MEM_TYPE_STRING, MEM_TYPE_SYMBOL, @@ -3331,12 +3330,10 @@ allocate_pseudovector (int memlen, int lisplen, struct buffer * allocate_buffer (void) { - struct buffer *b = lisp_malloc (sizeof *b, false, MEM_TYPE_BUFFER); - + struct buffer *b + = ALLOCATE_PSEUDOVECTOR (struct buffer, cursor_in_non_selected_windows_, + PVEC_BUFFER); BUFFER_PVEC_INIT (b); - /* Put B on the chain of all buffers including killed ones. */ - b->next = all_buffers; - all_buffers = b; /* Note that the rest fields of B are not initialized. */ return b; } @@ -4596,40 +4593,6 @@ live_vector_p (struct mem_node *m, void *p) return !NILP (live_vector_holding (m, p)); } -/* If P is a pointer into a valid buffer object, return the buffer. - Otherwise, return nil. M is a pointer to the mem_block for P. - If IGNORE_KILLED is non-zero, treat killed buffers as invalid. */ - -static Lisp_Object -live_buffer_holding (struct mem_node *m, void *p, bool ignore_killed) -{ - /* P must point into the block, and the buffer must not - have been killed unless ALL-BUFFERS is true. */ - if (m->type == MEM_TYPE_BUFFER) - { - struct buffer *b = m->start; - char *cb = m->start; - char *cp = p; - ptrdiff_t offset = cp - cb; - if (0 <= offset && offset < sizeof *b - && !(ignore_killed && NILP (b->name_))) - { - Lisp_Object obj; - XSETBUFFER (obj, b); - return obj; - } - } - return Qnil; -} - -/* If P is a pointer into a live (valid and not killed) buffer object, - return non-zero. */ -static bool -live_buffer_p (struct mem_node *m, void *p) -{ - return !NILP (live_buffer_holding (m, p, true)); -} - /* Mark OBJ if we can prove it's a Lisp_Object. */ static void @@ -4684,8 +4647,7 @@ mark_maybe_object (Lisp_Object obj) break; case Lisp_Vectorlike: - mark_p = (EQ (obj, live_vector_holding (m, po)) - || EQ (obj, live_buffer_holding (m, po, false))); + mark_p = (EQ (obj, live_vector_holding (m, po))); break; default: @@ -4754,10 +4716,6 @@ mark_maybe_pointer (void *p) /* Nothing to do; not a pointer to Lisp memory. */ break; - case MEM_TYPE_BUFFER: - obj = live_buffer_holding (m, p, false); - break; - case MEM_TYPE_CONS: obj = live_cons_holding (m, p); break; @@ -5157,9 +5115,6 @@ valid_lisp_object_p (Lisp_Object obj) case MEM_TYPE_SPARE: return 0; - case MEM_TYPE_BUFFER: - return live_buffer_p (m, p) ? 1 : 2; - case MEM_TYPE_CONS: return live_cons_p (m, p); @@ -5976,7 +5931,7 @@ maybe_garbage_collect (void) void garbage_collect (void) { - struct buffer *nextb; + Lisp_Object tail, buffer; char stack_top_variable; bool message_p; ptrdiff_t count = SPECPDL_INDEX (); @@ -5992,8 +5947,8 @@ garbage_collect (void) /* Don't keep undo information around forever. Do this early on, so it is no problem if the user quits. */ - FOR_EACH_BUFFER (nextb) - compact_buffer (nextb); + FOR_EACH_LIVE_BUFFER (tail, buffer) + compact_buffer (XBUFFER (buffer)); byte_ct tot_before = (profiler_memory_running ? total_bytes_of_live_objects () @@ -6083,8 +6038,9 @@ garbage_collect (void) compact_font_caches (); - FOR_EACH_BUFFER (nextb) + FOR_EACH_LIVE_BUFFER (tail, buffer) { + struct buffer *nextb = XBUFFER (buffer); if (!EQ (BVAR (nextb, undo_list), Qt)) bset_undo_list (nextb, compact_undo_list (BVAR (nextb, undo_list))); /* Now that we have stripped the elements that need not be @@ -6613,23 +6569,12 @@ #define CHECK_ALLOCATED_AND_LIVE_SYMBOL() ((void) 0) = PSEUDOVECTOR_TYPE (ptr); if (pvectype != PVEC_SUBR && - pvectype != PVEC_BUFFER && !main_thread_p (po)) CHECK_LIVE (live_vector_p); switch (pvectype) { case PVEC_BUFFER: -#if GC_CHECK_MARKED_OBJECTS - { - struct buffer *b; - FOR_EACH_BUFFER (b) - if (b == po) - break; - if (b == NULL) - emacs_abort (); - } -#endif /* GC_CHECK_MARKED_OBJECTS */ mark_buffer ((struct buffer *) ptr); break; @@ -7108,25 +7053,17 @@ unchain_dead_markers (struct buffer *buffer) static void sweep_buffers (void) { - struct buffer *buffer, **bprev = &all_buffers; + Lisp_Object tail, buf; gcstat.total_buffers = 0; - for (buffer = all_buffers; buffer; buffer = *bprev) - if (!vectorlike_marked_p (&buffer->header)) - { - *bprev = buffer->next; - lisp_free (buffer); - } - else - { - if (!pdumper_object_p (buffer)) - XUNMARK_VECTOR (buffer); - /* Do not use buffer_(set|get)_intervals here. */ - buffer->text->intervals = balance_intervals (buffer->text->intervals); - unchain_dead_markers (buffer); - gcstat.total_buffers++; - bprev = &buffer->next; - } + FOR_EACH_LIVE_BUFFER (tail, buf) + { + struct buffer *buffer = XBUFFER (buf); + /* Do not use buffer_(set|get)_intervals here. */ + buffer->text->intervals = balance_intervals (buffer->text->intervals); + unchain_dead_markers (buffer); + gcstat.total_buffers++; + } } /* Sweep: find all structures not marked, and free them. */ diff --git a/src/buffer.c b/src/buffer.c index 4e121ca4ca..355b34a980 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -51,11 +51,6 @@ Copyright (C) 1985-1989, 1993-1995, 1997-2020 Free Software Foundation, #include "w32heap.h" /* for mmap_* */ #endif -/* First buffer in chain of all buffers (in reverse order of creation). - Threaded through ->header.next.buffer. */ - -struct buffer *all_buffers; - /* This structure holds the default values of the buffer-local variables defined with DEFVAR_PER_BUFFER, that have special slots in each buffer. The default value occupies the same slot in this structure @@ -1786,15 +1781,11 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", ask questions or their hooks get errors. */ if (!b->base_buffer && b->indirections > 0) { - struct buffer *other; + Lisp_Object tail, other; - FOR_EACH_BUFFER (other) - if (other->base_buffer == b) - { - Lisp_Object buf; - XSETBUFFER (buf, other); - Fkill_buffer (buf); - } + FOR_EACH_LIVE_BUFFER (tail, other) + if (XBUFFER (other)->base_buffer == b) + Fkill_buffer (other); /* Exit if we now have killed the base buffer (Bug#11665). */ if (!BUFFER_LIVE_P (b)) @@ -2351,10 +2342,10 @@ DEFUN ("buffer-swap-text", Fbuffer_swap_text, Sbuffer_swap_text, error ("Cannot swap indirect buffers's text"); { /* This is probably harder to make work. */ - struct buffer *other; - FOR_EACH_BUFFER (other) - if (other->base_buffer == other_buffer - || other->base_buffer == current_buffer) + Lisp_Object tail, other; + FOR_EACH_LIVE_BUFFER (tail, other) + if (XBUFFER (other)->base_buffer == other_buffer + || XBUFFER (other)->base_buffer == current_buffer) error ("One of the buffers to swap has indirect buffers"); } @@ -2502,7 +2493,7 @@ DEFUN ("set-buffer-multibyte", Fset_buffer_multibyte, Sset_buffer_multibyte, (Lisp_Object flag) { struct Lisp_Marker *tail, *markers; - struct buffer *other; + Lisp_Object btail, other; ptrdiff_t begv, zv; bool narrowed = (BEG != BEGV || Z != ZV); bool modified_p = !NILP (Fbuffer_modified_p (Qnil)); @@ -2755,13 +2746,16 @@ DEFUN ("set-buffer-multibyte", Fset_buffer_multibyte, Sset_buffer_multibyte, /* Copy this buffer's new multibyte status into all of its indirect buffers. */ - FOR_EACH_BUFFER (other) - if (other->base_buffer == current_buffer && BUFFER_LIVE_P (other)) - { - BVAR (other, enable_multibyte_characters) - = BVAR (current_buffer, enable_multibyte_characters); - other->prevent_redisplay_optimizations_p = 1; - } + FOR_EACH_LIVE_BUFFER (btail, other) + { + struct buffer *o = XBUFFER (other); + if (o->base_buffer == current_buffer && BUFFER_LIVE_P (o)) + { + BVAR (o, enable_multibyte_characters) + = BVAR (current_buffer, enable_multibyte_characters); + o->prevent_redisplay_optimizations_p = true; + } + } /* Restore the modifiedness of the buffer. */ if (!modified_p && !NILP (Fbuffer_modified_p (Qnil))) @@ -5327,8 +5321,6 @@ init_buffer_once (void) Vbuffer_alist = Qnil; current_buffer = 0; pdumper_remember_lv_ptr_raw (¤t_buffer, Lisp_Vectorlike); - all_buffers = 0; - pdumper_remember_lv_ptr_raw (&all_buffers, Lisp_Vectorlike); QSFundamental = build_pure_c_string ("Fundamental"); @@ -5359,7 +5351,7 @@ init_buffer (void) #ifdef USE_MMAP_FOR_BUFFERS if (dumped_with_unexec_p ()) { - struct buffer *b; + Lisp_Object tail, buffer; #ifndef WINDOWSNT /* These must be reset in the dumped Emacs, to avoid stale @@ -5381,8 +5373,9 @@ init_buffer (void) " *code-conversion-work*". They are created by init_buffer_once and init_window_once (which are not called in the dumped Emacs), and by the first call to coding.c routines. */ - FOR_EACH_BUFFER (b) + FOR_EACH_LIVE_BUFFER (tail, buffer) { + struct buffer *b = XBUFFER (buffer); b->text->beg = NULL; enlarge_buffer_text (b, 0); } diff --git a/src/buffer.h b/src/buffer.h index 31f497ea40..abb1294d03 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -570,9 +570,6 @@ #define BVAR(buf, field) ((buf)->field ## _) In an indirect buffer, this is the own_text field of another buffer. */ struct buffer_text *text; - /* Next buffer, in chain of all buffers, including killed ones. */ - struct buffer *next; - /* Char position of point in buffer. */ ptrdiff_t pt; @@ -1104,15 +1101,6 @@ BUFFER_CHECK_INDIRECTION (struct buffer *b) } } -/* Chain of all buffers, including killed ones. */ - -extern struct buffer *all_buffers; - -/* Used to iterate over the chain above. */ - -#define FOR_EACH_BUFFER(b) \ - for ((b) = all_buffers; (b); (b) = (b)->next) - /* This structure holds the default values of the buffer-local variables that have special slots in each buffer. The default value occupies the same slot in this structure diff --git a/src/pdumper.c b/src/pdumper.c index e52163cdae..0f3835578a 100644 --- a/src/pdumper.c +++ b/src/pdumper.c @@ -2845,8 +2844,6 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer) ctx->obj_offset + dump_offsetof (struct buffer, text), base_offset + dump_offsetof (struct buffer, own_text)); - dump_field_lv_rawptr (ctx, out, buffer, &buffer->next, - Lisp_Vectorlike, WEIGHT_NORMAL); DUMP_FIELD_COPY (out, buffer, pt); DUMP_FIELD_COPY (out, buffer, pt_byte); DUMP_FIELD_COPY (out, buffer, begv);