bug#37393: 26.2.90; [PATCH] Speed up 'csv-align-fields'

[Simen Heggestøyl]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Sounds good.  I rarely use large CSV files, but I know the operation is slow.
>
> I'm OK with the patch, tho please see my comment below.

Thanks for reviewing it.

> 40s is still slow, but a factor of 2 is good, thanks.

Yes (though 40s is the time for all three benchmark runs, so one
alignment is 40s/3).

> If you're interested in this line, I think there are two avenues to
> improve the behavior further:
> - align lazily via jit-lock (this way the time is determined by the
>   amount of text displayed rather than the total file size).

Wouldn't that still depend on knowing the column widths? I find that the
column width computation is taking about 80% of the time when calling
'csv-align-fields' (after the patch).

> - make align-fields' into a mode, where fields are kept aligned even while
>   the buffer is modified.

That sounds nice.

>>  (defun csv--column-widths ()
>> -  (let ((widths '()))
>> +  (let ((column-widths '())
>> +        (field-widths '()))
>
> I think the return value is now sufficiently complex that the function
> deserves a docstring describing it.

Agreed, I'll add one before I install the patch.

I've also attached a new suggestion for speeding up the column width
computation itself by eliminating another 'current-column'-call. I'm not
too sure about its correctness yet, but it seems to work in a few tests
I've done, and it sped up 'csv--column-widths' by a factor of 1.3–1.4.

>From c3c077170aefa8ba0cd5d8f8b824c85eb0f01a66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sun, 15 Sep 2019 17:31:40 +0200
Subject: [PATCH] WIP

---
 packages/csv-mode/csv-mode.el | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/packages/csv-mode/csv-mode.el b/packages/csv-mode/csv-mode.el
index dc2555687..00107f51e 100644
--- a/packages/csv-mode/csv-mode.el
+++ b/packages/csv-mode/csv-mode.el
@@ -976,18 +976,26 @@ The fields yanked are those last killed by 
`csv-kill-fields'."
     (while (not (eobp))                   ; for each record...
       (or (csv-not-looking-at-record)
           (let ((w column-widths)
-                (col (current-column))
+                (col-beg (current-column))
+                col-end
                 field-width)
             (while (not (eolp))
               (csv-end-of-field)
-              (setq field-width (- (current-column) col))
+              (setq col-end (current-column))
+              (setq field-width (- col-end col-beg))
               (push field-width field-widths)
               (if w
                   (if (> field-width (car w)) (setcar w field-width))
                 (setq w (list field-width)
                       column-widths (nconc column-widths w)))
-              (or (eolp) (forward-char))  ; Skip separator.
-              (setq w (cdr w) col (current-column)))))
+              (unless (eolp)
+                (forward-char)            ; Skip separator.
+                (setq w (cdr w))
+                (setq col-beg (if (= (char-before) ?\t)
+                                  (* (/ (+ col-end tab-width)
+                                        tab-width)
+                                     tab-width)
+                                (+ col-end (char-width (char-before)))))))))
       (forward-line))
     (list column-widths (nreverse field-widths))))
 
-- 
2.23.0

-- Simen