Re: [Emacs-diffs] /srv/bzr/emacs/trunk r109898: * alloc.c (mark_object): Revert window marking code

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r109898: * alloc.c (mark_object): Revert window marking code

[Dmitry Antipov]

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

On 09/06/2012 04:54 PM, Stefan Monnier wrote:

>>    * alloc.c (mark_object): Revert window marking code
>>    since it's unsafe for the Fset_window_configuration.
>
> Please revert the rest of the patch as well.

Reverted, safe approach attached.

BTW, are there any reasons to reset local variables in Fkill_buffer?

Dmitry


[-- Attachment #2: nullify.patch --]
[-- Type: text/plain, Size: 2548 bytes --]

=== modified file 'src/buffer.c'
--- src/buffer.c	2012-09-06 09:15:44 +0000
+++ src/buffer.c	2012-09-07 07:27:44 +0000
@@ -1700,6 +1700,19 @@
     }
 }
 
+/* Set all B's Lisp_Object slots to nil in
+   attempt to let GC to sweep them early.  */
+
+static inline void
+nullify_buffer (struct buffer *b)
+{
+  int offset;
+
+  FOR_EACH_PER_BUFFER_OBJECT_AT (offset)
+    set_per_buffer_value (b, offset, Qnil);
+  bset_undo_list (b, Qnil);
+}
+
 DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ",
        doc: /* Kill the buffer specified by BUFFER-OR-NAME.
 The argument may be a buffer or the name of an existing buffer.
@@ -1911,14 +1924,11 @@
       /* Perhaps we should explicitly free the interval tree here... */
     }
 
-  /* Reset the local variables, so that this buffer's local values
-     won't be protected from GC.  They would be protected
-     if they happened to remain cached in their symbols.
-     This gets rid of them for certain.  */
+  /* Get rid of local bindings.  */
   swap_out_buffer_local_variables (b);
-  reset_buffer_local_variables (b, 1);
 
-  bset_name (b, Qnil);
+  /* B is really dead after this.  */
+  nullify_buffer (b);
 
   BLOCK_INPUT;
   if (b->base_buffer)
@@ -1942,9 +1952,7 @@
       free_region_cache (b->width_run_cache);
       b->width_run_cache = 0;
     }
-  bset_width_table (b, Qnil);
   UNBLOCK_INPUT;
-  bset_undo_list (b, Qnil);
 
   /* Run buffer-list-update-hook.  */
   if (!NILP (Vrun_hooks))

=== modified file 'src/frame.c'
--- src/frame.c	2012-09-04 17:34:54 +0000
+++ src/frame.c	2012-09-07 07:13:54 +0000
@@ -1114,6 +1114,20 @@
   return 0;
 }
 
+/* Set all F's Lisp_Object slots to nil in attempt to let
+   GC to sweep them early.  Setting f->terminal to NULL
+   makes FRAME_LIVE_P false for this frame.  */
+
+static inline void
+nullify_frame (struct frame *f)
+{
+  Lisp_Object *ptr;
+
+  for (ptr = &f->name; ptr <= &f->current_tool_bar_string; ptr++)
+    *ptr = Qnil;
+  f->terminal = NULL;
+}
+
 /* Delete FRAME.  When FORCE equals Qnoelisp, delete FRAME
   unconditionally.  x_connection_closed and delete_terminal use
   this.  Any other value of FORCE implements the semantics
@@ -1321,7 +1335,9 @@
   {
     struct terminal *terminal = FRAME_TERMINAL (f);
     f->output_data.nothing = 0;
-    f->terminal = 0;             /* Now the frame is dead. */
+
+    /* F is really dead after this.  */
+    nullify_frame (f);
 
     /* If needed, delete the terminal that this frame was on.
        (This must be done after the frame is killed.) */

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r109898: * alloc.c (mark_object): Revert window marking code

[Stefan Monnier]

> BTW, are there any reasons to reset local variables in Fkill_buffer?

It sets the buffer-local slots to the global value, so from the GC's
behavior point of view, it's the same as setting them to Qnil since
those values will be live anyway.
IOW using reset_buffer_local_variables should have pretty much the same
effect as your new nullify_buffer, except for the few fields it might miss.

BTW, have you tried to figure out what keeps those
buffers/frames/windows live?

> +/* Set all F's Lisp_Object slots to nil in attempt to let
> +   GC to sweep them early.  Setting f->terminal to NULL
> +   makes FRAME_LIVE_P false for this frame.  */
> +
> +static inline void
> +nullify_frame (struct frame *f)
> +{
> +  Lisp_Object *ptr;
> +
> +  for (ptr = &f->name; ptr <= &f->current_tool_bar_string; ptr++)
> +    *ptr = Qnil;
> +  f->terminal = NULL;
> +}
> +
>  /* Delete FRAME.  When FORCE equals Qnoelisp, delete FRAME
>    unconditionally.  x_connection_closed and delete_terminal use
>    this.  Any other value of FORCE implements the semantics
> @@ -1321,7 +1335,9 @@
>    {
>      struct terminal *terminal = FRAME_TERMINAL (f);
> f-> output_data.nothing = 0;
> -    f->terminal = 0;             /* Now the frame is dead. */
> +
> +    /* F is really dead after this.  */
> +    nullify_frame (f);
 
I actually prefer having the f->terminal=NULL where it was, so that
nullify_frame really only affects the GC behavior and is
otherwise unnoticeable.  Same for nullify_buffer.


        Stefan