From: "Simen Heggestøyl" <simenheg@gmail.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 37393@debbugs.gnu.org, sdl.web@gmail.com
Subject: bug#37393: 26.2.90; [PATCH] Speed up 'csv-align-fields'
Date: Sun, 15 Sep 2019 17:55:44 +0200 [thread overview]
Message-ID: <5d7e5f00.1c69fb81.43f55.3fa2@mx.google.com> (raw)
In-Reply-To: <jwvk1adh0r1.fsf-monnier+emacs@gnu.org> (message from Stefan Monnier on Thu, 12 Sep 2019 13:46:36 -0400)
[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP.patch --]
[-- Type: text/x-diff, Size: 1948 bytes --]
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
[-- Attachment #3: Type: text/plain, Size: 9 bytes --]
-- Simen
next prev parent reply other threads:[~2019-09-15 15:55 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-12 17:07 bug#37393: 26.2.90; [PATCH] Speed up 'csv-align-fields' Simen Heggestøyl
2019-09-12 17:46 ` Stefan Monnier
2019-09-15 15:55 ` Simen Heggestøyl [this message]
2019-09-15 16:17 ` Eli Zaretskii
2019-09-15 18:43 ` Stefan Monnier
2019-09-17 16:53 ` Simen Heggestøyl
2019-09-17 17:23 ` Eli Zaretskii
2019-09-17 19:14 ` Stefan Monnier
2019-09-18 2:34 ` Eli Zaretskii
2019-09-18 19:59 ` Simen Heggestøyl
2019-09-18 20:08 ` Stefan Monnier
2019-09-19 15:51 ` Simen Heggestøyl
2019-09-19 17:30 ` Eli Zaretskii
2019-10-09 16:33 ` Simen Heggestøyl
2019-09-17 19:12 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5d7e5f00.1c69fb81.43f55.3fa2@mx.google.com \
--to=simenheg@gmail.com \
--cc=37393@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
--cc=sdl.web@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.