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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Pip Cet <pipcet <at> protonmail.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#78735: closed (feature/igc: [PATCH] Reduce the size of the
 kbd-buffer GC root)
Date: Fri, 13 Jun 2025 13:09:02 +0000

[Message part 1 (text/plain, inline)]

Your message dated Fri, 13 Jun 2025 13:07:50 +0000
with message-id <87jz5fyel8.fsf <at> protonmail.com>
and subject line Re: bug#78735: feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root
has caused the debbugs.gnu.org bug report #78735,
regarding feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
78735: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78735
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Helmut Eller <eller.helmut <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root
Date: Mon, 09 Jun 2025 21:36:45 +0200

[Message part 3 (text/plain, inline)]

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.

[0001-Reduce-the-size-of-the-kbd-buffer-GC-root.patch (text/x-diff, attachment)]

[Message part 5 (message/rfc822, inline)]

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

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

> On Mon, Jun 09 2025, Pip Cet wrote:
>
> [...]
>>> +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...
>
> The extra argument avoids accessing the global variable; that's all.
>
>>> +  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)?
>
> Hmm, indeed.
>
>> 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.
>
> Yes, that should work.

I've applied a new version of this patch, and am closing the bug.
Thanks again, Helmut!

Pip

Previous Next

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