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

Previous Next

Full log

Message #38 received at 72689 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 72689 <at> debbugs.gnu.org
Subject: Re: bug#72689: 31.0.50; Proposal to improve string-pixel-width
Date: Wed, 21 Aug 2024 22:43:08 +0200

On 21/08/2024 3:17 PM, Eli Zaretskii wrote:
[...]
>>
>> 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?
> 
> Thanks, but using a mutex is overkill: there could be no race between
> two or more threads in this case in accessing the buffer-local
> variable, because only one Lisp thread can be running at any given
> time.  So the simpler method of testing the "busy" flag should be
> sufficient.

I used a mutex to protect the global variable `work-buffer--list'
(which holds the list buffers available to be reused) against
concurrent accesses during the very simple update operations: pop an
available buffer, push a released buffer to make it available.

Of course, if you guarantee that only one Lisp thread can be running
at any given time, protecting `work-buffer--list' against concurrent
accesses is not necessary and mutex can be removed.

>> 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.
> 
> Simpler implementation is OK, but I think it will be simpler yet if
> you remove the mutex, which is not needed.

Here is the new version without mutex (no significant impact on
performance compared to the version with mutex):

(defvar work-buffer--list nil)

(defsubst work-buffer--get ()
  "Get a work buffer."
  (let ((buffer (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.
        (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))))))

;;; Apply the work-buffer API to `string-pixel-width'.
;;
(defun string-pixel-width (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)))))

Should I prepare a patch of subr-x.el to include both the proposed
`work-buffer' API, and an implementation of `string-pixel-width' using
it?

Thanks!

Previous Next

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