GNU bug report logs - #78735
feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root

Previous Next

Full log

Message #8 received at 78735 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 78735 <at> debbugs.gnu.org
Subject: Re: bug#78735: feature/igc: [PATCH] Reduce the size of the kbd-buffer
 GC root
Date: Mon, 09 Jun 2025 20:08:36 +0000

"Helmut Eller" <eller.helmut <at> gmail.com> writes:

> This is a proposal to reduce of the size of the kbd-buffer root.
> Currently the kdb-buffer is an area of ~250kb that is scanned
> ambiguously.
>
> With the patch, instead of tracing the entire kbd_buffer, we only scan
> the region from kbd_fetch_ptr - 1 to kbd_store_ptr + 1.  The -1/+1 is
> supposed to cover the cases where MPS stops the mutator while those
> pointers are being updated.  If the kbd_buffer is empty, then only 180
> bytes are scanned.  It's still scanned ambiguously.

That sounds great to me.

> +static union buffered_input_event *
> +prev_kbd_event (union buffered_input_event *kbd_buffer,
> +		union buffered_input_event *ptr)
> +{
> +  return ptr == kbd_buffer ? kbd_buffer + KBD_BUFFER_SIZE - 1 : ptr - 1;
> +}
> +
> +static union buffered_input_event *
> +next_kbd_event (union buffered_input_event *kbd_buffer,
> +		union buffered_input_event *ptr)
> +{
> +  return ptr == kbd_buffer + KBD_BUFFER_SIZE - 1 ? kbd_buffer : ptr + 1;
> +}

Just out of curiosity, is there a reason for the extra kbd_buffer
argument?  Since this code is replicated a few times, maybe it would be
better to keep next_kbd_event in keyboard.c identical to the one in
igc.c.  Maybe not, though...

> +  union buffered_input_event *fetch
> +    = prev_kbd_event (kbd_buffer, kbd_fetch_ptr);
> +  union buffered_input_event *store
> +    = next_kbd_event (kbd_buffer, kbd_store_ptr);
> +
> +  if (fetch < store)
> +    return scan_ambig (ss, fetch, store, closure);

I think this will fail if kbd_store_ptr + 1 == kbd_fetch_ptr (i.e. the
kbd_buffer is full)?

if (fetch < store - 1)

should work, though, and it's not like this case is common enough to
worry about scanning the same event twice.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.