MPS: struct window, prev_buffers + next_buffers

MPS: struct window, prev_buffers + next_buffers

[Gerd Möllmann]

I have some questions about the handling of window::prev_buffers and
next_buffers. These are handled in the existing GC in this function in
alloc.c:

  /* Remove killed buffers or items whose car is a killed buffer from
     LIST, and mark other items.  Return changed LIST, which is marked.  */

  static Lisp_Object
  mark_discard_killed_buffers (Lisp_Object list)
  {
    Lisp_Object tail, *prev = &list;

    for (tail = list; CONSP (tail) && !cons_marked_p (XCONS (tail));
         tail = XCDR (tail))
      {
        Lisp_Object tem = XCAR (tail);
        if (CONSP (tem))
          tem = XCAR (tem);
        if (BUFFERP (tem) && !BUFFER_LIVE_P (XBUFFER (tem)))
          *prev = XCDR (tail);
        else
          {
            set_cons_marked (XCONS (tail));
            mark_object (XCAR (tail));
            prev = xcdr_addr (tail);
          }
      }
    mark_object (tail);
    return list;
  }

IGC must do something with these lists, too, I guess.

Some questions:

1. Does igc really have to do something? I don't see something
noticeably growing, even after hours of using Emacs with igc.

2. Is it an option to leaving these lists alone in GC?

3. If igc should do something, how frequently? IOW what makes these
lists grow? And perhaps could we do this cleanup where new entries are
pushed onto them?

4. Note that the loop in the function stops when a cons cell is marked.
So it doesn't always take entries off the list that contain dead
buffers. I don't understand that at all.

TIA

Re: MPS: struct window, prev_buffers + next_buffers

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Helmut Eller <eller.helmut@gmail.com>
> Date: Sat, 22 Jun 2024 09:24:59 +0200
> 
> 1. Does igc really have to do something? I don't see something
> noticeably growing, even after hours of using Emacs with igc.

How many buffers did that session create and delete?  Do you see
killed buffers not collected (i.e. buffer objects whose name is nil
that are not garbage-collected because the window's prev_buffers or
next_buffers list references those killed buffers?  That's what the
comment in mark_window says:

  /* Filter out killed buffers from both buffer lists
     in attempt to help GC to reclaim killed buffers faster.
     We can do it elsewhere for live windows, but this is the
     best place to do it for dead windows.  */
  wset_prev_buffers
    (w, mark_discard_killed_buffers (w->prev_buffers));
  wset_next_buffers
    (w, mark_discard_killed_buffers (w->next_buffers));

> 2. Is it an option to leaving these lists alone in GC?

Isn't this the same question as 1, just IOW?

> 3. If igc should do something, how frequently? IOW what makes these
> lists grow? And perhaps could we do this cleanup where new entries are
> pushed onto them?

The growth of these lists is not the problem, I think.  The problem is
that they reference killed buffers, and that prevents those killed
buffers from being GCed.

> 4. Note that the loop in the function stops when a cons cell is marked.
> So it doesn't always take entries off the list that contain dead
> buffers. I don't understand that at all.

I guess the cons cell being marked means that someone has a reference
to it, and therefore it cannot be GCed?

Re: MPS: struct window, prev_buffers + next_buffers

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, Helmut Eller <eller.helmut@gmail.com>
>> Date: Sat, 22 Jun 2024 09:24:59 +0200
>> 
>> 1. Does igc really have to do something? I don't see something
>> noticeably growing, even after hours of using Emacs with igc.
>
> How many buffers did that session create and delete?  Do you see
> killed buffers not collected (i.e. buffer objects whose name is nil
> that are not garbage-collected because the window's prev_buffers or
> next_buffers list references those killed buffers?  That's what the
> comment in mark_window says:

How many buffers get killed is hard to tell. I have usually 50-100 file
buffers, which are kind of long-lived, plus constantly temp buffers (I
guess) that are created by vertico and alike.

I've not noticed that the number of buffers increases noticeably. There
is always some variation that I would expect from conservative roots, of
course, control stack and alike.

Maybe it's because windows aren't long-lived? No idea.

>
>   /* Filter out killed buffers from both buffer lists
>      in attempt to help GC to reclaim killed buffers faster.
>      We can do it elsewhere for live windows, but this is the
>      best place to do it for dead windows.  */
>   wset_prev_buffers
>     (w, mark_discard_killed_buffers (w->prev_buffers));
>   wset_next_buffers
>     (w, mark_discard_killed_buffers (w->next_buffers));
>
>> 2. Is it an option to leaving these lists alone in GC?
>
> Isn't this the same question as 1, just IOW?

Maybe :-)

>> 3. If igc should do something, how frequently? IOW what makes these
>> lists grow? And perhaps could we do this cleanup where new entries are
>> pushed onto them?
>
> The growth of these lists is not the problem, I think.  The problem is
> that they reference killed buffers, and that prevents those killed
> buffers from being GCed.
>
>> 4. Note that the loop in the function stops when a cons cell is marked.
>> So it doesn't always take entries off the list that contain dead
>> buffers. I don't understand that at all.
>
> I guess the cons cell being marked means that someone has a reference
> to it, and therefore it cannot be GCed?

But it won't be GC'd anyway, becuase it's marked. And if the buffer in
the cons is killed, it remains in the list. That makes no sense to me.

Re: MPS: struct window, prev_buffers + next_buffers

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  eller.helmut@gmail.com
> Date: Sat, 22 Jun 2024 10:13:33 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> Cc: Eli Zaretskii <eliz@gnu.org>, Helmut Eller <eller.helmut@gmail.com>
> >> Date: Sat, 22 Jun 2024 09:24:59 +0200
> >> 
> >> 1. Does igc really have to do something? I don't see something
> >> noticeably growing, even after hours of using Emacs with igc.
> >
> > How many buffers did that session create and delete?  Do you see
> > killed buffers not collected (i.e. buffer objects whose name is nil
> > that are not garbage-collected because the window's prev_buffers or
> > next_buffers list references those killed buffers?  That's what the
> > comment in mark_window says:
> 
> How many buffers get killed is hard to tell.

Can't you write some simple code to count them?  A killed buffer has
its name nil, so it is easy to spot (we also have BUFFER_LIVE_P).  We
also have window-next-buffers and window-prev-buffers that return
these two lists.

> I have usually 50-100 file buffers, which are kind of long-lived,
> plus constantly temp buffers (I guess) that are created by vertico
> and alike.

I think the use case is that you visit a file in a window, and then
kill it after some time.  Do that several times, switching between
buffers in between, and you should have those killed buffers in the
prev_buffers and next_buffers lists of the window.

> Maybe it's because windows aren't long-lived? No idea.

Yes, you should do that in a single window to see the effect, I think.

> >> 4. Note that the loop in the function stops when a cons cell is marked.
> >> So it doesn't always take entries off the list that contain dead
> >> buffers. I don't understand that at all.
> >
> > I guess the cons cell being marked means that someone has a reference
> > to it, and therefore it cannot be GCed?
> 
> But it won't be GC'd anyway, becuase it's marked. And if the buffer in
> the cons is killed, it remains in the list. That makes no sense to me.

But the code doesn't GC buffers, it just removes them from the
prev_buffers and next_buffers lists.  If a cons cell of the list is
marked, removing the buffer (which could subsequently lead to GC'ing
the buffer) might not be expected by the code which uses data that
references that buffer, even though the buffer is killed.

Re: MPS: struct window, prev_buffers + next_buffers

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> But it won't be GC'd anyway, becuase it's marked. And if the buffer in
>> the cons is killed, it remains in the list. That makes no sense to me.
>
> But the code doesn't GC buffers, it just removes them from the
> prev_buffers and next_buffers lists.  If a cons cell of the list is
> marked, removing the buffer (which could subsequently lead to GC'ing
> the buffer) might not be expected by the code which uses data that
> references that buffer, even though the buffer is killed.

You mean the possible change to the cdr of the cons cell could affect
the client? Hm, that could be the reason. Thanks.

I think I'll try to let window-{next,prev}-buffers clean the list
before returning it. It doesn't look like something to me that a weak
vector or something like that could solve. WDYT?
  

Re: MPS: struct window, prev_buffers + next_buffers

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  eller.helmut@gmail.com
> Date: Sat, 22 Jun 2024 10:53:57 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> But it won't be GC'd anyway, becuase it's marked. And if the buffer in
> >> the cons is killed, it remains in the list. That makes no sense to me.
> >
> > But the code doesn't GC buffers, it just removes them from the
> > prev_buffers and next_buffers lists.  If a cons cell of the list is
> > marked, removing the buffer (which could subsequently lead to GC'ing
> > the buffer) might not be expected by the code which uses data that
> > references that buffer, even though the buffer is killed.
> 
> You mean the possible change to the cdr of the cons cell could affect
> the client? Hm, that could be the reason. Thanks.
> 
> I think I'll try to let window-{next,prev}-buffers clean the list
> before returning it. It doesn't look like something to me that a weak
> vector or something like that could solve. WDYT?

That would work, but what if these functions are not called for a
while?

Also, it would make these simple accessors more expensive.

Re: MPS: struct window, prev_buffers + next_buffers

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> I think I'll try to let window-{next,prev}-buffers clean the list
>> before returning it. It doesn't look like something to me that a weak
>> vector or something like that could solve. WDYT?
>
> That would work, but what if these functions are not called for a
> while?

Hm. I suspect it won't do much harm, from what I see here at least, with
my usage pattern and config.

> Also, it would make these simple accessors more expensive.

True.

I'll push something like what I said in a few minutes. Let's see how it
fares.