bug#72689: 31.0.50; Proposal to improve string-pixel-width

[David Ponce via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Hello,

The function string-pixel-width is essential for calculating pixel
dimensions, especially for UI components. And in this context this
function can be called very often when the display is refreshed.

I propose the attached patch to make string-pixel-width faster while
using less memory, as shown by the following results of a basic
benchmark run in emacs -Q to compare the current implementation and this
proposal:

;; Basic benchmark
(let* ((text1 (make-string 1000 ?x))
        (text2 (propertize text1 'line-prefix "XXXX ")))
   (list
    (string-pixel-width text1)
    (string-pixel-width text2)
    (progn (garbage-collect)
           (benchmark-run 10000 (string-pixel-width text1)))
    (progn (garbage-collect)
           (benchmark-run 10000 (string-pixel-width text2)))
    ;;(insert text "\n")
    ))

;; Result with current implementation (4 run):
(12000 12000 (1.854707611 17 0.120106147) (1.884129905 17 0.12258791599999996))

(12000 12000 (1.846544798 17 0.12243524500000003) (1.8822177530000002 17 0.12349287399999997))

(12000 12000 (1.851244125 17 0.12162041699999998) (1.860517709 17 0.12352999599999998))

(12000 12000 (1.8542218929999998 17 0.12164553900000001) (1.856302462 17 0.122891689))

;; Result with proposed implementation (4 run):
(12000 12000 (1.698974522 0 0.0) (1.727446 2 0.014782505999999973))

(12000 12000 (1.701800124 0 0.0) (1.728024111 2 0.014718454999999908))

(12000 12000 (1.6850850800000001 0 0.0) (1.732370238 2 0.014801913000000111))

(12000 12000 (1.7356390130000001 0 0.0) (1.7858915800000001 2 0.014816158000000135))

 From my observations, the new implementation is around 8% faster, and
trigger less GC.  When there is no line-prefix property to
remove, the new implementation doesn't trigger any GC after 10000 runs.
Otherwise, only 2 GC are triggered instead of 17.

Maybe this proposal might be of interest, or at least provide some ideas
for improvement.

Thanks


In GNU Emacs 31.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version
  3.24.43, cairo version 1.18.0) of 2024-08-17
Repository revision: 40eecd594ac60f38b6729fd9cf3474a8b9d133b9
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 40 (KDE Plasma)

Configured using:
  'configure --with-x-toolkit=gtk3 --with-cairo-xcb
  --with-native-compilation=no
  PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LC_TIME: fr_FR.utf8
   value of $LANG: fr_FR.UTF-8
   locale-coding-system: utf-8-unix


[-- Attachment #2: improve-string-pixel-width-V00.patch --]
[-- Type: text/x-patch, Size: 2913 bytes --]

2024-08-17  David Ponce  <da_vid@orange.fr>

	Tweak the implementation of string-pixel-width to run faster and
	use less memory.

	* subr-x.el (string--pixel-width-buffer): New variable and function.
	(string--pixel-width): Use it.  Prefer `remove-text-properties' to
	`propertize' to avoid creating a new string on each call.


diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 058c06bc5f6..1f564fa5628 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -336,6 +336,23 @@ named-let
       (cl-labels ((,name ,fargs . ,body)) #',name)
       . ,aargs)))
 
+(defvar string--pixel-width-buffer nil)
+
+(defsubst string--pixel-width-buffer ()
+  "Get internal buffer used to calculate the pixel width of a string."
+  ;; Keeping a work buffer around is more efficient than creating a
+  ;; new temporary buffer on each call to `string-pixel-width'.
+  (if (buffer-live-p string--pixel-width-buffer)
+      string--pixel-width-buffer
+    (with-current-buffer (get-buffer-create " *string-pixel-width*" t)
+      ;; If `display-line-numbers' is enabled in internal buffers
+      ;; (e.g. globally), it breaks width calculation (bug#59311).
+      ;; Disable line-prefix and wrap-prefix, for the same reason.
+      ;; Set all these variables only one time here: they
+      ;; automatically become buffer-local when set.
+      (setq display-line-numbers nil line-prefix nil wrap-prefix nil)
+      (setq string--pixel-width-buffer (current-buffer)))))
+
 ;;;###autoload
 (defun string-pixel-width (string &optional buffer)
   "Return the width of STRING in pixels.
@@ -344,22 +361,18 @@ string-pixel-width
   (declare (important-return-value t))
   (if (zerop (length string))
       0
-    ;; Keeping a work buffer around is more efficient than creating a
-    ;; new temporary buffer.
-    (with-current-buffer (get-buffer-create " *string-pixel-width*")
-      ;; If `display-line-numbers' is enabled in internal buffers
-      ;; (e.g. globally), it breaks width calculation (bug#59311)
-      (setq-local display-line-numbers nil)
-      (delete-region (point-min) (point-max))
-      ;; Disable line-prefix and wrap-prefix, for the same reason.
-      (setq line-prefix nil
-	    wrap-prefix nil)
+    (with-current-buffer (string--pixel-width-buffer)
+      (erase-buffer)
       (if buffer
           (setq-local face-remapping-alist
                       (with-current-buffer buffer
                         face-remapping-alist))
         (kill-local-variable 'face-remapping-alist))
-      (insert (propertize string 'line-prefix nil 'wrap-prefix nil))
+      (insert string)
+      ;; Prefer `remove-text-properties' to `propertize' to avoid
+      ;; creating a new string on each call.
+      (remove-text-properties
+       (point-min) (point-max) '(line-prefix nil wrap-prefix nil))
       (car (buffer-text-pixel-size nil nil t)))))
 
 ;;;###autoload