GNU logs - #78735, boring messages

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#78735: feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root
Resent-From: Helmut Eller <eller.helmut@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 09 Jun 2025 19:38:03 +0000
Resent-Message-ID: <handler.78735.B.17494978299818 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 78735 <at> debbugs.gnu.org
From: Helmut Eller <eller.helmut@HIDDEN>
Date: Mon, 09 Jun 2025 21:36:45 +0200
Message-ID: <87v7p4g10y.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Received-SPF: pass client-ip=2a00:1450:4864:20::334;
 envelope-from=eller.helmut@HIDDEN; helo=mail-wm1-x334.google.com
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001,
 RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001,
 SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: 1.0 (+)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.0 (/)

--=-=-=
Content-Type: text/plain

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.



--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=0001-Reduce-the-size-of-the-kbd-buffer-GC-root.patch

From d1e9eeb7d3cfea48d1037564fda50b511e80a52b Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@HIDDEN>
Date: Mon, 9 Jun 2025 18:28:10 +0200
Subject: [PATCH] Reduce the size of the kbd-buffer GC root

* src/igc.c (root_create_kbd_buffer): New.
(root_create_main_thread): Use it.
(scan_kbd_buffer_ambig): New. The actual scan function.
(next_kbd_event, prev_kbd_event): New helpers, similar to next_kbd_event
in keyboard.c.
* src/keyboard.c (init_keyboard): Move the GC root creation code to
root_create_kbd_buffer in igc.c.
---
 src/igc.c      | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/keyboard.c |  4 ----
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/src/igc.c b/src/igc.c
index f63b0fe8c88..5f7a1a78cda 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -60,6 +60,7 @@
 #include "thread.h"
 #include "treesit.h"
 #include "termchar.h"
+#include "keyboard.h"
 #ifdef HAVE_WINDOW_SYSTEM
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -1725,6 +1726,48 @@ scan_tty_list (mps_ss_t ss, void *start, void *end, void *closure)
   return MPS_RES_OK;
 }
 
+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;
+}
+
+static mps_res_t
+scan_kbd_buffer_ambig (mps_ss_t ss, void *start, void *end, void *closure)
+{
+  igc_assert (start == kbd_buffer);
+  igc_assert (end == kbd_buffer + ARRAYELTS (kbd_buffer));
+
+  /* Instead of tracing the entire kbd_buffer, 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. */
+
+  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);
+  else
+    {
+      mps_res_t res
+	= scan_ambig (ss, fetch, kbd_buffer + KBD_BUFFER_SIZE, closure);
+      if (res == MPS_RES_OK)
+	res = scan_ambig (ss, kbd_buffer, store, closure);
+      return res;
+    }
+}
+
 /***********************************************************************
 			 Default pad, fwd, ...
  ***********************************************************************/
@@ -2905,6 +2948,14 @@ root_create_tty_list (struct igc *gc)
 		     scan_tty_list, "tty-list");
 }
 
+static void
+root_create_kbd_buffer (struct igc *gc)
+{
+  root_create (gc, kbd_buffer, kbd_buffer + ARRAYELTS (kbd_buffer),
+	       mps_rank_ambig (), scan_kbd_buffer_ambig, NULL,
+	       true, "kbd-buffer");
+}
+
 static void
 root_create_main_thread (struct igc *gc)
 {
@@ -4934,6 +4985,7 @@ make_igc (void)
   root_create_main_thread (gc);
   root_create_exact_ptr (gc, &current_thread);
   root_create_exact_ptr (gc, &all_threads);
+  root_create_kbd_buffer (gc);
 
   enable_messages (gc, true);
   return gc;
diff --git a/src/keyboard.c b/src/keyboard.c
index 72c6e4c17d8..1c2b4ba9e63 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -12892,10 +12892,6 @@ delete_kboard (KBOARD *kb)
 void
 init_keyboard (void)
 {
-#ifdef HAVE_MPS
-  igc_root_create_ambig (kbd_buffer, kbd_buffer + ARRAYELTS (kbd_buffer),
-			 "kbd-buffer");
-#endif
   /* This is correct before outermost invocation of the editor loop.  */
   command_loop_level = -1;
   quit_char = Ctl ('g');
-- 
2.39.5


--=-=-=--

Message sent:

Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Mailer: MIME-tools 5.505 (Entity 5.505)
Content-Type: text/plain; charset=utf-8
X-Loop: help-debbugs@HIDDEN
From: help-debbugs@HIDDEN (GNU bug Tracking System)
To: Helmut Eller <eller.helmut@HIDDEN>
Subject: bug#78735: Acknowledgement (feature/igc: [PATCH] Reduce the size
 of the kbd-buffer GC root)
Message-ID: <handler.78735.B.17494978299818.ack <at> debbugs.gnu.org>
References: <87v7p4g10y.fsf@HIDDEN>
X-Gnu-PR-Message: ack 78735
X-Gnu-PR-Package: emacs
X-Gnu-PR-Keywords: patch
Reply-To: 78735 <at> debbugs.gnu.org
Date: Mon, 09 Jun 2025 19:38:04 +0000

Thank you for filing a new bug report with debbugs.gnu.org.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 bug-gnu-emacs@HIDDEN

If you wish to submit further information on this problem, please
send it to 78735 <at> debbugs.gnu.org.

Please do not send mail to help-debbugs@HIDDEN unless you wish
to report a problem with the Bug-tracking system.

--=20
78735: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D78735
GNU Bug Tracking System
Contact help-debbugs@HIDDEN with problems

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#78735: feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root
Resent-From: Pip Cet <pipcet@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 09 Jun 2025 20:09:05 +0000
Resent-Message-ID: <handler.78735.B78735.174949974130552 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: Helmut Eller <eller.helmut@HIDDEN>
Cc: 78735 <at> debbugs.gnu.org
Date: Mon, 09 Jun 2025 20:08:36 +0000
From: Pip Cet <pipcet@HIDDEN>
Message-ID: <87y0u03cg0.fsf@HIDDEN>
In-Reply-To: <87v7p4g10y.fsf@HIDDEN>
References: <87v7p4g10y.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

"Helmut Eller" <eller.helmut@HIDDEN> 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,
> +=09=09union buffered_input_event *ptr)
> +{
> +  return ptr =3D=3D kbd_buffer ? kbd_buffer + KBD_BUFFER_SIZE - 1 : ptr =
- 1;
> +}
> +
> +static union buffered_input_event *
> +next_kbd_event (union buffered_input_event *kbd_buffer,
> +=09=09union buffered_input_event *ptr)
> +{
> +  return ptr =3D=3D 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
> +    =3D prev_kbd_event (kbd_buffer, kbd_fetch_ptr);
> +  union buffered_input_event *store
> +    =3D 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 =3D=3D 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

Last modified: Mon, 9 Jun 2025 20:15:03 UTC

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