GNU bug report logs - #72689
31.0.50; Proposal to improve string-pixel-width

Previous Next

Package: emacs;

Reported by: David Ponce <da_vid <at> orange.fr>

Date: Sat, 17 Aug 2024 22:05:01 UTC

Severity: normal

Found in version 31.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: David Ponce <da_vid <at> orange.fr>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 72689 <at> debbugs.gnu.org
Subject: bug#72689: 31.0.50; Proposal to improve string-pixel-width
Date: Tue, 20 Aug 2024 17:12:45 +0200
On 18/08/2024 11:15 AM, Eli Zaretskii wrote:
[...]
>>> Thanks.  The idea SGTM, but I think the implementation needs to cater
>>> for the case where more than one execution thread performs this job
>>> "in parallel" (however improbable this could sound), so we need to be
>>> able to detect when this buffer is "busy".  The simplest way is to use
>>> some boolean buffer-local variable, which will be set non-nil when the
>>> function starts using the buffer and reset to nil when the function is
>>> done with its job.

I've been thinking more about the parallelism issue when a function
reuses a temporary buffer for its activity, and I wonder if we could
use a simple API like the one below to safely get an exclusive working
buffer without having to create a new one on each call?

;; ---------------------------------------------------------------
(defvar work-buffer--list nil)
(defvar work-buffer--lock (make-mutex))

(defsubst work-buffer--get ()
  "Get a work buffer."
  (let ((buffer (with-mutex work-buffer--lock
                  (pop work-buffer--list))))
    (if (buffer-live-p buffer)
        buffer
      (generate-new-buffer " *work*" t))))

(defsubst work-buffer--release (buffer)
  "Release work BUFFER."
  (if (buffer-live-p buffer)
      (with-current-buffer buffer
        ;; Flush BUFFER before making it available again, i.e. clear
        ;; its contents, remove all overlays and buffer-local
        ;; variables.  Is it enough to safely reuse the buffer?
        (erase-buffer)
        (delete-all-overlays)
        (let (change-major-mode-hook) (kill-all-local-variables t))
        ;; Make the buffer available again.
        (with-mutex work-buffer--lock
          (push buffer work-buffer--list)))))

;;;###autoload
(defmacro with-work-buffer (&rest body)
  "Create a work buffer, and evaluate BODY there like `progn'.
Like `with-temp-buffer', but reuse an already created temporary
buffer when possible, instead of creating a new one on each call."
  (declare (indent 0) (debug t))
  (let ((work-buffer (make-symbol "work-buffer")))
    `(let ((,work-buffer (work-buffer--get)))
       (with-current-buffer ,work-buffer
         (unwind-protect
	     (progn ,@body)
           (work-buffer--release ,work-buffer))))))
;; ---------------------------------------------------------------

Here is how string-pixel-width could be implemented to use the above
API:

(defun string-pixel-W (string &optional buffer)
  "Return the width of STRING in pixels.
If BUFFER is non-nil, use the face remappings from that buffer when
determining the 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-work-buffer
      ;; 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.
      (setq display-line-numbers nil
            line-prefix nil wrap-prefix nil)
      (if buffer
          (setq-local face-remapping-alist
                      (with-current-buffer buffer
                        face-remapping-alist))
        (kill-local-variable 'face-remapping-alist))
      (erase-buffer)
      (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)))))

;; Quick benchmarck
(let* ((text1 (make-string 1000 ?x))
       (text2 (propertize text1 'line-prefix "XXXX "))
       (runs 1000))
  (list
   (progn (garbage-collect)
          (benchmark-run runs (string-pixel-width text2)))
   (progn (garbage-collect)
          (benchmark-run runs (string-pixel-W text2)))
   ))

Compared to my previous proposal the quick benchmark above shows
similar results for both performance and memory usage, but the new
implementation is simpler, and the API might be useful in other
similar cases.

WDYT?




This bug report was last modified 263 days ago.

Previous Next


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