Hi all, For the hackathon, I'm working on improving table alignment performance since I've seen tables be slow in my day-to-day.
I have a couple patches, with which I see nearly 4x improvement on Ihor's long table test case (~23k rows, went from ~41 seconds -> ~11 seconds): https://0x0.st/KOVt.txt These patches improve org-string-width performance (fewer allocations) and cache its results in org-table-align. The actual improvement you'll see depends on how many cells have the same text (for cache hits), but I expect ~2x improvement in the worst cases. I think there's still quite a lot of room for optimization here. I only dabble in Elisp; comments and modifications are welcome... And I will complete the FSF copyright waiver. Best, Kenny
>From 960502e02aa20eef17cbb23fae04c43c8fc78add Mon Sep 17 00:00:00 2001 From: Kenny Chen <[email protected]> Date: Sat, 22 Nov 2025 12:28:12 -0500 Subject: [PATCH 1/4] org-macs.el: Cache Emacs version check for `org-string-width' * lisp/org-macs.el (org-string-width): Cache the `version<' call for performance. --- lisp/org-macs.el | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index c3be41d02..9ab2eda88 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -1159,6 +1159,8 @@ delimiting S." ((= cursor end) 0) (t (string-width (substring s cursor end))))))) +(defvar org-string-width--old-emacs (version< emacs-version "28")) + (defun org--string-width-1 (string) "Return width of STRING when displayed in the current buffer. Unlike `string-width', this function takes into consideration @@ -1173,7 +1175,7 @@ Results may be off sometimes if it cannot handle a given Return width in pixels when PIXELS is non-nil. When PIXELS is nil, DEFAULT-FACE is the face used to calculate relative STRING width. When REFERENCE-FACE is nil, `default' face is used." - (if (and (version< emacs-version "28") (not pixels)) + (if (and org-string-width--old-emacs (not pixels)) ;; FIXME: Fallback to old limited version, because ;; `window-pixel-width' is buggy in older Emacs. (org--string-width-1 string) -- 2.52.0
>From 3689d742fdf934a28d20c382951d710e8025f95e Mon Sep 17 00:00:00 2001 From: Kenny Chen <[email protected]> Date: Sat, 22 Nov 2025 14:35:02 -0500 Subject: [PATCH 2/4] org-macs.el: Construct `buffer-invisibility-spec' directly * lisp/org-macs.el (org-string-width): Construct the `buffer-invisibility-spec' with only one iteration. --- lisp/org-macs.el | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index 9ab2eda88..c609ff0b3 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -1207,7 +1207,14 @@ STRING width. When REFERENCE-FACE is nil, `default' face is used." '(org-fold-drawer org-fold-block org-fold-outline)))) - (push el result))) + (push + ;; Consider ellipsis to have 0 width. + ;; It is what Emacs 28+ does, but we have + ;; to force it in earlier Emacs versions. + (if (and (consp el) (cdr el)) + (list (car el)) + el) + result))) result))) (current-char-property-alias-alist char-property-alias-alist)) (with-current-buffer (get-buffer-create " *Org string width*" t) @@ -1215,16 +1222,7 @@ STRING width. When REFERENCE-FACE is nil, `default' face is used." (setq-local line-prefix nil) (setq-local wrap-prefix nil) (setq-local buffer-invisibility-spec - (if (listp current-invisibility-spec) - (mapcar (lambda (el) - ;; Consider ellipsis to have 0 width. - ;; It is what Emacs 28+ does, but we have - ;; to force it in earlier Emacs versions. - (if (and (consp el) (cdr el)) - (list (car el)) - el)) - current-invisibility-spec) - current-invisibility-spec)) + current-invisibility-spec) (setq-local char-property-alias-alist current-char-property-alias-alist) (let (pixel-width symbol-width) -- 2.52.0
>From d5fc2852cd165d4bb2552bcffa73422963c0c56c Mon Sep 17 00:00:00 2001 From: Kenny Chen <[email protected]> Date: Sat, 22 Nov 2025 15:46:19 -0500 Subject: [PATCH 3/4] org-table.el: Cache cell width when aligning tables * lisp/org-table.el (org-table-align): Cache `org-string-width' results and reuse when possible instead of calling it for every cell, twice. (org-table--align-field): Accept a new optional argument for the field width. --- lisp/org-table.el | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/lisp/org-table.el b/lisp/org-table.el index 551e18e9f..124427172 100644 --- a/lisp/org-table.el +++ b/lisp/org-table.el @@ -4376,11 +4376,11 @@ extension of the given file name, and finally on the variable (user-error "TABLE_EXPORT_FORMAT invalid"))))) ;;;###autoload -(defun org-table--align-field (field width align) +(defun org-table--align-field (field width align &optional field-width) "Format FIELD according to column WIDTH and alignment ALIGN. FIELD is a string. WIDTH is a number. ALIGN is either \"c\", \"l\" or\"r\"." - (let* ((spaces (- width (org-string-width field nil 'org-table))) + (let* ((spaces (- width (or field-width (org-string-width field nil 'org-table)))) (prefix (pcase align ("l" "") ("r" (make-string spaces ?\s)) @@ -4409,7 +4409,15 @@ FIELD is a string. WIDTH is a number. ALIGN is either \"c\", (rows (remq 'hline table)) (widths nil) (alignments nil) - (columns-number 1)) + (columns-number 1) + (cell-width-cache (make-hash-table :test 'equal)) + (get-or-compute-cell-width + (lambda (cell) + (or (gethash cell cell-width-cache) + (puthash + cell + (org-string-width cell nil 'org-table) + cell-width-cache))))) (if (null rows) ;; Table contains only horizontal rules. Compute the ;; number of columns anyway, and choose an arbitrary width @@ -4429,7 +4437,7 @@ FIELD is a string. WIDTH is a number. ALIGN is either \"c\", (non-empty 0)) (dolist (row rows) (let ((cell (or (nth i row) ""))) - (setq max-width (max max-width (org-string-width cell nil 'org-table))) + (setq max-width (max max-width (funcall get-or-compute-cell-width cell))) (cond (fixed-align? nil) ((equal cell "") nil) ((string-match "\\`<\\([lrc]\\)[0-9]*>\\'" cell) @@ -4470,10 +4478,16 @@ FIELD is a string. WIDTH is a number. ALIGN is either \"c\", (append row (make-list offset ""))))) (mapconcat #'identity - (cl-mapcar #'org-table--align-field - fields - widths - alignments) + (cl-mapcar + (lambda (field width alignment) + (org-table--align-field + field + width + alignment + (funcall get-or-compute-cell-width field))) + fields + widths + alignments) "|"))) "|"))) (if (equal new previous) -- 2.52.0
>From c190bda021cfa9ad19b899b97bfb89da39c197c0 Mon Sep 17 00:00:00 2001 From: Kenny Chen <[email protected]> Date: Sat, 22 Nov 2025 15:31:23 -0500 Subject: [PATCH 4/4] org-table.el: Cache invisibility spec for `org-string-width' * lisp/org-macs.el (org-string-width): Refactor invisibility spec construction into a separate function, and accept the spec as a new optional argument. * lisp/org-table.el (org-table-align): Call `org-string-width-invisibility-spec' once and pass the spec to the `org-string-width' calls. --- lisp/org-macs.el | 55 +++++++++++++++++++++++++---------------------- lisp/org-table.el | 3 ++- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index c609ff0b3..0aa12cfb4 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -1170,7 +1170,34 @@ Results may be off sometimes if it cannot handle a given `display' value." (org--string-from-props string 'display 0 (length string))) -(defun org-string-width (string &optional pixels default-face) +(defun org-string-width-invisibility-spec () + "Return the invisibility spec of this buffer without folds and ellipses." + ;; We need to remove the folds to make sure that folded table + ;; alignment is not messed up. + (or (and (not (listp buffer-invisibility-spec)) + buffer-invisibility-spec) + (let (result) + (dolist (el buffer-invisibility-spec) + (unless (or (memq el + '(org-fold-drawer + org-fold-block + org-fold-outline)) + (and (listp el) + (memq (car el) + '(org-fold-drawer + org-fold-block + org-fold-outline)))) + (push + ;; Consider ellipsis to have 0 width. + ;; It is what Emacs 28+ does, but we have + ;; to force it in earlier Emacs versions. + (if (and (consp el) (cdr el)) + (list (car el)) + el) + result))) + result))) + +(defun org-string-width (string &optional pixels default-face invisibility-spec) "Return width of STRING when displayed in the current buffer. Return width in pixels when PIXELS is non-nil. When PIXELS is nil, DEFAULT-FACE is the face used to calculate relative @@ -1191,31 +1218,7 @@ STRING width. When REFERENCE-FACE is nil, `default' face is used." ;; when PIXELS are requested though). (unless pixels (put-text-property 0 (length string) 'face (or default-face 'default) string)) - (let (;; We need to remove the folds to make sure that folded table - ;; alignment is not messed up. - (current-invisibility-spec - (or (and (not (listp buffer-invisibility-spec)) - buffer-invisibility-spec) - (let (result) - (dolist (el buffer-invisibility-spec) - (unless (or (memq el - '(org-fold-drawer - org-fold-block - org-fold-outline)) - (and (listp el) - (memq (car el) - '(org-fold-drawer - org-fold-block - org-fold-outline)))) - (push - ;; Consider ellipsis to have 0 width. - ;; It is what Emacs 28+ does, but we have - ;; to force it in earlier Emacs versions. - (if (and (consp el) (cdr el)) - (list (car el)) - el) - result))) - result))) + (let ((current-invisibility-spec (or invisibility-spec (org-string-width-invisibility-spec))) (current-char-property-alias-alist char-property-alias-alist)) (with-current-buffer (get-buffer-create " *Org string width*" t) (setq-local display-line-numbers nil) diff --git a/lisp/org-table.el b/lisp/org-table.el index 124427172..769f05190 100644 --- a/lisp/org-table.el +++ b/lisp/org-table.el @@ -4410,13 +4410,14 @@ FIELD is a string. WIDTH is a number. ALIGN is either \"c\", (widths nil) (alignments nil) (columns-number 1) + (invisibility-spec (org-string-width-invisibility-spec)) (cell-width-cache (make-hash-table :test 'equal)) (get-or-compute-cell-width (lambda (cell) (or (gethash cell cell-width-cache) (puthash cell - (org-string-width cell nil 'org-table) + (org-string-width cell nil 'org-table invisibility-spec) cell-width-cache))))) (if (null rows) ;; Table contains only horizontal rules. Compute the -- 2.52.0
