bug#75342: [PATCH] Speed up asynchronous man page fontifying

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Stefan Kangas]

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

Severity: wishlist

The below patch makes asynchronous fontifying of man pages faster.

After typing `M-x man RET gcc-14 RET`, it takes around 5 minutes on this
fast MacBook Pro M2 (2022) machine until the whole 1.43 MiB man page is
displayed.  With a hot cache, the best time I have managed to measure is
173.432 seconds.

We seem to call `man--maybe-fontify-manpage` many times on very small
chunks: on my machine, it processes ~30 characters at a time.  Things
get substantially faster if we make `Man-bgproc-filter` fontify the
buffer in much larger chunks.  The main drawback of doing this is that
we risk very briefly seeing an incorrect display flash by in the man
buffer (e.g., with `end-of-buffer`).

My measurements show that 32 KiB might be a good choice, and gives a
~95% speedup:

| Chunk size (KiB) | Completion time (s) |
|------------------+---------------------|
|                4 |                29.3 |
|                8 |                18.2 |
|               16 |                12.7 |
|               32 |                 7.4 |
|               64 |                 6.5 |
|              128 |                 5.4 |

Is this the best approach to optimize something like this, or am I
overlooking something obvious?  Note that I didn't add a variable for
the chunk size, but we could easily add one, if that'd be useful.

When I set `Man-prefer-synchronous-call` to t, it takes ~1.8 seconds to
process the same page.  I guess that this is the lower bound for how
fast we could make the asynchronous call.

Please see the attached.

[-- Attachment #2: 0001-Fontify-man-page-in-32-KiB-chunks.patch --]
[-- Type: text/x-patch, Size: 3053 bytes --]

From 2f677cf4457ae4a2fdf02cee658988da3a27dafc Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sat, 4 Jan 2025 07:38:33 +0100
Subject: [PATCH] Fontify man page in 32 KiB chunks

* lisp/man.el (man--bgproc-filter-fontify): New function.
(Man-bgproc-filter): Use above new function; fontify in 32 KiB chunks.
(Man-bgproc-sentinel): Use above new function.
(man--bgproc-filter-last-pos): New variable.
(Man-getpage-in-background): Set above new variable to 0.
---
 lisp/man.el | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/lisp/man.el b/lisp/man.el
index 54ca8cbae9f..470d6790ba2 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -417,6 +417,7 @@ Man-reverse
 (defvar-local Man-original-frame nil)
 (defvar-local Man-arguments nil)
 (put 'Man-arguments 'permanent-local t)
+(defvar-local man--bgproc-filter-last-pos nil)
 
 (defvar-local Man--sections nil)
 (defvar-local Man--refpages nil)
@@ -1243,8 +1244,9 @@ Man-getpage-in-background
 			  (Man-shell-file-name)
 			  shell-command-switch
 			  (format (Man-build-man-command) man-args))))
-	       (set-process-sentinel proc 'Man-bgproc-sentinel)
-	       (set-process-filter proc 'Man-bgproc-filter))
+               (set-process-sentinel proc #'Man-bgproc-sentinel)
+               (set-process-filter proc #'Man-bgproc-filter)
+               (setq man--bgproc-filter-last-pos 0))
 	   (let* ((inhibit-read-only t)
 		  (exit-status
 		   (process-file
@@ -1515,6 +1517,19 @@ man--maybe-fontify-manpage
       (Man-fontify-manpage)
     (Man-cleanup-manpage)))
 
+(defun man--bgproc-filter-fontify ()
+  (let ((inhibit-read-only t))
+    (save-restriction
+      (narrow-to-region
+       (save-excursion
+         (goto-char man--bgproc-filter-last-pos)
+         ;; Process whole sections (Bug#36927).
+         (Man-previous-section 1)
+         (point))
+       (point))
+      (setq man--bgproc-filter-last-pos (point))
+      (man--maybe-fontify-manpage))))
+
 (defun Man-bgproc-filter (process string)
   "Manpage background process filter.
 When manpage command is run asynchronously, PROCESS is the process
@@ -1532,15 +1547,9 @@ Man-bgproc-filter
 	    (save-excursion
 	      (goto-char beg)
 	      (insert string)
-	      (save-restriction
-		(narrow-to-region
-		 (save-excursion
-		   (goto-char beg)
-                   ;; Process whole sections (Bug#36927).
-                   (Man-previous-section 1)
-                   (point))
-		 (point))
-		(man--maybe-fontify-manpage))
+              ;; Chunk the processing.
+              (when (>= (- beg man--bgproc-filter-last-pos) 0)
+                (man--bgproc-filter-fontify))
 	      (set-marker (process-mark process) (point-max)))))))))
 
 (defun Man-bgproc-sentinel (process msg)
@@ -1560,6 +1569,7 @@ Man-bgproc-sentinel
 	    (set-process-buffer process nil))
 
       (with-current-buffer Man-buffer
+        (man--bgproc-filter-fontify)
 	(save-excursion
 	  (let ((case-fold-search nil)
                 (inhibit-read-only t))
-- 
2.47.1

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Stefan Kangas]

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

Stefan Kangas <stefankangas@gmail.com> writes:

> My measurements show that 32 KiB might be a good choice, and gives a
> ~95% speedup:
>
> | Chunk size (KiB) | Completion time (s) |
> |------------------+---------------------|
> |                4 |                29.3 |
> |                8 |                18.2 |
> |               16 |                12.7 |
> |               32 |                 7.4 |
> |               64 |                 6.5 |
> |              128 |                 5.4 |

BTW, to reproduce these measurements, try the attached.

[-- Attachment #2: 0001-Report-time-to-process-man-page.patch --]
[-- Type: text/x-patch, Size: 2315 bytes --]

From 4fd6ecc3d599cedabc821c2d85cc010770dd537b Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sat, 4 Jan 2025 06:01:55 +0100
Subject: [PATCH] Report time to process man page

---
 lisp/man.el | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/lisp/man.el b/lisp/man.el
index 54ca8cbae9f..36c136db4a6 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -1210,6 +1210,8 @@ Man-start-calling
     (setenv "MAN_KEEP_FORMATTING" "1")
     ,@body))
 
+(defvar-local man--background-time-start nil)
+
 (defun Man-getpage-in-background (topic)
   "Use TOPIC to build and fire off the manpage and cleaning command.
 Return the buffer in which the manpage will appear."
@@ -1230,6 +1232,7 @@ Man-getpage-in-background
 	(setq Man-original-frame (selected-frame))
 	(setq Man-arguments man-args)
 	(Man-mode)
+        (setq man--background-time-start (current-time))
 	(setq mode-line-process
 	      (concat " " (propertize (if Man-fontify-manpage-flag
 					  "[formatting...]"
@@ -1621,11 +1624,19 @@ Man-bgproc-sentinel
                       (setq message (format "Can't find the %s manpage"
                                             (Man-page-from-arguments args)))))
 
-		(if Man-fontify-manpage-flag
-		    (setq message (format "%s man page formatted"
-			                  (Man-page-from-arguments Man-arguments)))
-		  (setq message (format "%s man page cleaned up"
-			                (Man-page-from-arguments Man-arguments))))
+                (let ((elapsed-time
+                       (format-time-string
+                        " [in %s.%3N seconds]"
+                        (time-subtract (current-time)
+                                       man--background-time-start))))
+		  (setq message
+                        (if Man-fontify-manpage-flag
+                            (format "%s man page formatted%s"
+			            (Man-page-from-arguments Man-arguments)
+                                    elapsed-time)
+                          (format "%s man page cleaned up%s"
+                                  (Man-page-from-arguments Man-arguments)
+                                  elapsed-time))))
 		(unless (and (processp process)
 			     (not (eq (process-status process) 'exit)))
 		  (setq mode-line-process nil))
-- 
2.47.1

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Eli Zaretskii]

> Cc: Juri Linkov <juri@linkov.net>
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 4 Jan 2025 01:22:03 -0600
> 
> We seem to call `man--maybe-fontify-manpage` many times on very small
> chunks: on my machine, it processes ~30 characters at a time.  Things
> get substantially faster if we make `Man-bgproc-filter` fontify the
> buffer in much larger chunks.  The main drawback of doing this is that
> we risk very briefly seeing an incorrect display flash by in the man
> buffer (e.g., with `end-of-buffer`).

Man-bgproc-filter also affects how we process sections of the man
page, see bug#36927.  Wouldn't larger chunks increase the probability
of making an error there?

> My measurements show that 32 KiB might be a good choice, and gives a
> ~95% speedup:
> 
> | Chunk size (KiB) | Completion time (s) |
> |------------------+---------------------|
> |                4 |                29.3 |
> |                8 |                18.2 |
> |               16 |                12.7 |
> |               32 |                 7.4 |
> |               64 |                 6.5 |
> |              128 |                 5.4 |
> 
> Is this the best approach to optimize something like this, or am I
> overlooking something obvious?  Note that I didn't add a variable for
> the chunk size, but we could easily add one, if that'd be useful.
> 
> When I set `Man-prefer-synchronous-call` to t, it takes ~1.8 seconds to
> process the same page.  I guess that this is the lower bound for how
> fast we could make the asynchronous call.

Why not make Man-prefer-synchronous-call t by default, then?

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> My measurements show that 32 KiB might be a good choice, and gives a
>> ~95% speedup:
>> 
>> | Chunk size (KiB) | Completion time (s) |
>> |------------------+---------------------|
>> |                4 |                29.3 |
>> |                8 |                18.2 |
>> |               16 |                12.7 |
>> |               32 |                 7.4 |
>> |               64 |                 6.5 |
>> |              128 |                 5.4 |
>> 
>> Is this the best approach to optimize something like this, or am I
>> overlooking something obvious?  Note that I didn't add a variable for
>> the chunk size, but we could easily add one, if that'd be useful.
>> 
>> When I set `Man-prefer-synchronous-call` to t, it takes ~1.8 seconds to
>> process the same page.  I guess that this is the lower bound for how
>> fast we could make the asynchronous call.
>
> Why not make Man-prefer-synchronous-call t by default, then?

The call might take longer on slower machines (or network connections)
and then block Emacs until the call was done.

Never block ui unless you can do avoid it.

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Eli Zaretskii]

> From: Björn Bidar <bjorn.bidar@thaodan.de>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  75342@debbugs.gnu.org,
>   juri@linkov.net
> Date: Sat, 04 Jan 2025 11:33:18 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> When I set `Man-prefer-synchronous-call` to t, it takes ~1.8 seconds to
> >> process the same page.  I guess that this is the lower bound for how
> >> fast we could make the asynchronous call.
> >
> > Why not make Man-prefer-synchronous-call t by default, then?
> 
> The call might take longer on slower machines (or network connections)
> and then block Emacs until the call was done.

Slower connections (I'm guessing you mean remote man pages?) could be
recognized exempted from synchronous operation.  As for slower
machines: this is a defcustom, so users of slower machines can
customize it if the synchronous formatting is too long for them.

> Never block ui unless you can do avoid it.

Except that this comes at a price here: the time until I can see the
full man page could be very long.  So this is not a back-and-white
situation.

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Björn Bidar <bjorn.bidar@thaodan.de>
>> Cc: Stefan Kangas <stefankangas@gmail.com>,  75342@debbugs.gnu.org,
>>   juri@linkov.net
>> Date: Sat, 04 Jan 2025 11:33:18 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> When I set `Man-prefer-synchronous-call` to t, it takes ~1.8 seconds to
>> >> process the same page.  I guess that this is the lower bound for how
>> >> fast we could make the asynchronous call.
>> >
>> > Why not make Man-prefer-synchronous-call t by default, then?
>> 
>> The call might take longer on slower machines (or network connections)
>> and then block Emacs until the call was done.
>
> Slower connections (I'm guessing you mean remote man pages?) could be
> recognized exempted from synchronous operation.

Yeah that's what I meant.

> As for slower machines: this is a defcustom, so users of slower machines can
> customize it if the synchronous formatting is too long for them.

I don't know yes its customizable but it could lead to some surprises.

>> Never block ui unless you can do avoid it.
>
> Except that this comes at a price here: the time until I can see the
> full man page could be very long.  So this is not a back-and-white
> situation.

Why is the price so high? The time to call man should be the same in
both asynchronous and synchronous call.

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Eli Zaretskii]

> From: Björn Bidar <bjorn.bidar@thaodan.de>
> Cc: 75342@debbugs.gnu.org,  stefankangas@gmail.com,  juri@linkov.net
> Date: Sat, 04 Jan 2025 23:13:06 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Never block ui unless you can do avoid it.
> >
> > Except that this comes at a price here: the time until I can see the
> > full man page could be very long.  So this is not a back-and-white
> > situation.
> 
> Why is the price so high? The time to call man should be the same in
> both asynchronous and synchronous call.

I think Stefan explained that: we read the text in small chunks, and
each chunk we read requires non-trivial processing, see the filter
function.

bug#75342: [PATCH] Speed up asynchronous man page fontifying

[Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors]

I tried the patch on OpenSUSE Linux:
- with patch: gcc-14 man page formatted [in 12.790 seconds]
- without patch: gcc-14 man page formatted [in 24.272 seconds]

Man-prefer-synchronous-call t: gcc-14 man page formatted [in 0.559 seconds]

I was surprised how fast the synchronous formatting was. Why is the
asynchronous operating so slow?