GNU bug report logs -
#67536
29.1; Calc mode's math-read-preprocess-string conses unnecessarily
Previous Next
Reported by: Raffael Stocker <r.stocker <at> mnet-mail.de>
Date: Wed, 29 Nov 2023 21:32:02 UTC
Severity: normal
Found in version 29.1
Done: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67536 in the body.
You can then email your comments to 67536 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Wed, 29 Nov 2023 21:32:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Raffael Stocker <r.stocker <at> mnet-mail.de>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Wed, 29 Nov 2023 21:32:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Org table re-calculation is very slow, partly due to
math-read-preprocess-string of calc mode consing unnecessarily. For
example, in one (large) table, I get the following memory usage from the
profiler:
...
60,252,646 96% - org-ctrl-c-ctrl-c
60,248,166 96% - org-table-calc-current-TBLFM
60,216,431 96% - funcall-interactively
60,205,119 96% - org-table-recalculate
49,094,651 78% - org-table-eval-formula
32,624,631 52% - calc-eval
32,624,631 52% - calc-do-calc-eval
32,620,487 52% - calc-do-calc-eval
32,611,151 52% - math-read-exprs
29,388,838 47% + math-read-preprocess-string
2,343,257 3% + math-read-expr-list
...
The reason for the slow-down seems to be that
math-read-preprocess-string conses a lot, keeping the GC busy. This is
due to heavy use of replace-regexp-in-string in this function:
(defun math-read-preprocess-string (str)
"Replace some substrings of STR by Calc equivalents."
(setq str
(replace-regexp-in-string (concat "[" math-read-superscripts "]+")
"^(\\&)" str))
(setq str
(replace-regexp-in-string (concat "[" math-read-subscripts "]+")
"_(\\&)" str))
(let ((rep-list math-read-replacement-list))
(while rep-list
;; consing like a mad-man here:
(setq str
(replace-regexp-in-string (nth 0 (car rep-list))
(nth 1 (car rep-list)) str))
(setq rep-list (cdr rep-list))))
str)
I would like to propose using a temp buffer instead of kneading the
string into submission:
(defun math-read-preprocess-string (str)
"Replace some substrings of STR by Calc equivalents."
(with-temp-buffer
(insert str)
(goto-char 0)
(while (re-search-forward (concat "[" math-read-superscripts "]+") nil t)
(replace-match "^(\\&)"))
(goto-char 0)
(while (re-search-forward (concat "[" math-read-subscripts "]+") nil t)
(replace-match "_(\\&)"))
(goto-char 0)
(let ((rep-list math-read-replacement-list))
(while rep-list
(while (re-search-forward (nth 0 (car rep-list)) nil t)
(replace-match (nth 1 (car rep-list))))
(goto-char 0)
(setq rep-list (cdr rep-list))))
(buffer-string)))
With this replacement, the profiler shows much less memory usage on the
same org table:
...
30,411,804 91% - org-ctrl-c-ctrl-c
30,407,324 91% - org-table-calc-current-TBLFM
30,363,932 91% - funcall-interactively
30,331,900 91% - org-table-recalculate
20,430,223 61% - org-table-eval-formula
6,751,852 20% + org-table-justify-field-maybe
4,598,523 13% - calc-eval
4,586,091 13% - calc-do-calc-eval
4,569,619 13% - calc-do-calc-eval
4,547,971 13% - math-read-exprs
2,297,453 6% + math-read-expr-list
1,347,377 4% + math-read-preprocess-string
7,296 0% math-read-token
...
Only I am not sure the replacement is correct in all possible edge cases
(or whether it uses so much less memory because I have overlooked
something).
What do you think? Can a calc-mode expert weigh in here?
Regards,
Raffael
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Thu, 30 Nov 2023 07:01:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 67536 <at> debbugs.gnu.org (full text, mbox):
> From: Raffael Stocker <r.stocker <at> mnet-mail.de>
> Date: Wed, 29 Nov 2023 22:29:38 +0100
>
>
> Org table re-calculation is very slow, partly due to
> math-read-preprocess-string of calc mode consing unnecessarily. For
> example, in one (large) table, I get the following memory usage from the
> profiler:
>
> ...
> 60,252,646 96% - org-ctrl-c-ctrl-c
> 60,248,166 96% - org-table-calc-current-TBLFM
> 60,216,431 96% - funcall-interactively
> 60,205,119 96% - org-table-recalculate
> 49,094,651 78% - org-table-eval-formula
> 32,624,631 52% - calc-eval
> 32,624,631 52% - calc-do-calc-eval
> 32,620,487 52% - calc-do-calc-eval
> 32,611,151 52% - math-read-exprs
> 29,388,838 47% + math-read-preprocess-string
> 2,343,257 3% + math-read-expr-list
This is not memory usage, this is CPU usage measured by using
memory-allocation functions as triggers to probe for the function that
is being executed. So its evidence about consing and GC pressure is
indirect at best.
Can you instead look at the values of gcs-done and gc-elapsed before
and after running the offending code, and show the delta of each one
of them, with and without your proposed changes? Then we will see a
much more direct evidence about the number of GC cycles and the time
spent in GC, and could make the decision about how to improve the
situation.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Thu, 30 Nov 2023 19:03:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 67536 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> ...
>> 60,252,646 96% - org-ctrl-c-ctrl-c
>> 60,248,166 96% - org-table-calc-current-TBLFM
>> 60,216,431 96% - funcall-interactively
>> 60,205,119 96% - org-table-recalculate
>> 49,094,651 78% - org-table-eval-formula
>> 32,624,631 52% - calc-eval
>> 32,624,631 52% - calc-do-calc-eval
>> 32,620,487 52% - calc-do-calc-eval
>> 32,611,151 52% - math-read-exprs
>> 29,388,838 47% + math-read-preprocess-string
>> 2,343,257 3% + math-read-expr-list
>
> This is not memory usage, this is CPU usage measured by using
> memory-allocation functions as triggers to probe for the function that
> is being executed. So its evidence about consing and GC pressure is
> indirect at best.
>
> Can you instead look at the values of gcs-done and gc-elapsed before
> and after running the offending code, and show the delta of each one
> of them, with and without your proposed changes? Then we will see a
> much more direct evidence about the number of GC cycles and the time
> spent in GC, and could make the decision about how to improve the
> situation.
I defined the following advice:
--8<---------------cut here---------------start------------->8---
(defun my-gc-status (orig-fun &rest args)
(let* ((done-bf gcs-done)
(elapsed-bf gc-elapsed)
(res (apply orig-fun args))
(done-af gcs-done)
(elapsed-af gc-elapsed))
(message "before:\n\tgcs-done: %d, gc-elapsed: %f" done-bf elapsed-bf)
(message "after:\n\tgcs-done: %d, difference: %d\n\tgc-elapsed: %f, difference: %f"
done-af (- done-af done-bf)
elapsed-af (- elapsed-af elapsed-bf))
res))
(advice-add 'org-table-recalculate :around #'my-gc-status)
--8<---------------cut here---------------end--------------->8---
I had to put it around ‘org-table-recalculate’ instead of
‘math-read-preprocess-string’ as all the functions below
org-table-recalculate are called very often and have small individual
contributions. With the original ‘math-read-preprocess-string’ I get the
following typical result:
--8<---------------cut here---------------start------------->8---
before:
gcs-done: 854, gc-elapsed: 170.601313
after:
gcs-done: 859, difference: 5
gc-elapsed: 171.671042, difference: 1.069729
--8<---------------cut here---------------end--------------->8---
I ran the command about twenty times and almost always got the
gcs-done difference of 5, with the occasional 4. The gc-elapsed is
fairly consistent at 1.07 for the 5 GC runs.
The modified version yields this typical output:
--8<---------------cut here---------------start------------->8---
before:
gcs-done: 906, gc-elapsed: 181.417292
after:
gcs-done: 908, difference: 2
gc-elapsed: 181.847972, difference: 0.430679
--8<---------------cut here---------------end--------------->8---
Again, the gcs-done difference is quite stable at 2, with the occasional
3, the gc-elapsed is also fairly consistent at 0.43 for the 2 GC runs.
So we have about a factor of 2.5 between the elapsed GC times for the
two versions.
Regards,
Raffael
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Thu, 30 Nov 2023 19:16:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 67536 <at> debbugs.gnu.org (full text, mbox):
> From: Raffael Stocker <r.stocker <at> mnet-mail.de>
> Cc: 67536 <at> debbugs.gnu.org
> Date: Thu, 30 Nov 2023 19:28:27 +0100
>
>
> (defun my-gc-status (orig-fun &rest args)
> (let* ((done-bf gcs-done)
> (elapsed-bf gc-elapsed)
> (res (apply orig-fun args))
> (done-af gcs-done)
> (elapsed-af gc-elapsed))
> (message "before:\n\tgcs-done: %d, gc-elapsed: %f" done-bf elapsed-bf)
> (message "after:\n\tgcs-done: %d, difference: %d\n\tgc-elapsed: %f, difference: %f"
> done-af (- done-af done-bf)
> elapsed-af (- elapsed-af elapsed-bf))
> res))
> (advice-add 'org-table-recalculate :around #'my-gc-status)
>
> --8<---------------cut here---------------end--------------->8---
>
> I had to put it around ‘org-table-recalculate’ instead of
> ‘math-read-preprocess-string’ as all the functions below
> org-table-recalculate are called very often and have small individual
> contributions. With the original ‘math-read-preprocess-string’ I get the
> following typical result:
>
> --8<---------------cut here---------------start------------->8---
>
> before:
> gcs-done: 854, gc-elapsed: 170.601313
> after:
> gcs-done: 859, difference: 5
> gc-elapsed: 171.671042, difference: 1.069729
>
> --8<---------------cut here---------------end--------------->8---
>
> I ran the command about twenty times and almost always got the
> gcs-done difference of 5, with the occasional 4. The gc-elapsed is
> fairly consistent at 1.07 for the 5 GC runs.
>
> The modified version yields this typical output:
>
> --8<---------------cut here---------------start------------->8---
>
> before:
> gcs-done: 906, gc-elapsed: 181.417292
> after:
> gcs-done: 908, difference: 2
> gc-elapsed: 181.847972, difference: 0.430679
>
> --8<---------------cut here---------------end--------------->8---
>
> Again, the gcs-done difference is quite stable at 2, with the occasional
> 3, the gc-elapsed is also fairly consistent at 0.43 for the 2 GC runs.
>
> So we have about a factor of 2.5 between the elapsed GC times for the
> two versions.
Thanks, sounds like a good optimization to me.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Fri, 01 Dec 2023 17:41:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
> Thanks, sounds like a good optimization to me.
Splendid!
I cleaned it up a bit and ran a few tests against the original function
using an empty string, a string without anything to replace and a string
requiring many replacements. It seems to behave just as the original.
I use cl-flet and (eval-when-compile (require 'cl-lib)). I hope that is
ok.
Regards,
Raffael
[0001-lisp-calc-calc-aent.el-math-read-preprocess-string-c.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Fri, 01 Dec 2023 18:14:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 67536 <at> debbugs.gnu.org (full text, mbox):
> From: Raffael Stocker <r.stocker <at> mnet-mail.de>
> Cc: 67536 <at> debbugs.gnu.org
> Date: Fri, 01 Dec 2023 18:34:10 +0100
>
> I cleaned it up a bit and ran a few tests against the original function
> using an empty string, a string without anything to replace and a string
> requiring many replacements. It seems to behave just as the original.
Thanks.
> I use cl-flet and (eval-when-compile (require 'cl-lib)). I hope that is
> ok.
I'd prefer a simple internal function, or a lambda.
> + (with-temp-buffer
> + (cl-flet ((replace-all (regexp replacement)
> + (goto-char 0)
> + (while (re-search-forward regexp nil t)
> + (replace-match replacement))))
> + (insert str)
> + (replace-all (concat "[" math-read-superscripts "]+") "^(\\&)")
> + (replace-all (concat "[" math-read-subscripts "]+") "_(\\&)")
> + (dolist (rep-elem math-read-replacement-list)
> + (replace-all (car rep-elem) (cadr rep-elem)))
> + (buffer-string))))
I think buffer-substring-no-properties would be better than
buffer-string, as we don't need to copy any text properties, right?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Fri, 01 Dec 2023 21:16:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> I use cl-flet and (eval-when-compile (require 'cl-lib)). I hope that is
>> ok.
>
> I'd prefer a simple internal function, or a lambda.
I went with the lambda, it is the more concise choice.
>> + (buffer-string))))
>
> I think buffer-substring-no-properties would be better than
> buffer-string, as we don't need to copy any text properties, right?
Yes, indeed. I fixed that too.
Regards,
Raffael
[0001-lisp-calc-calc-aent.el-math-read-preprocess-string-c.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sat, 02 Dec 2023 08:05:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 67536 <at> debbugs.gnu.org (full text, mbox):
> From: Raffael Stocker <r.stocker <at> mnet-mail.de>
> Cc: 67536 <at> debbugs.gnu.org
> Date: Fri, 01 Dec 2023 22:10:22 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> I use cl-flet and (eval-when-compile (require 'cl-lib)). I hope that is
> >> ok.
> >
> > I'd prefer a simple internal function, or a lambda.
>
> I went with the lambda, it is the more concise choice.
>
> >> + (buffer-string))))
> >
> > I think buffer-substring-no-properties would be better than
> > buffer-string, as we don't need to copy any text properties, right?
>
> Yes, indeed. I fixed that too.
Thanks.
Mattias, any comments, or should I install this?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sat, 02 Dec 2023 14:58:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
2 dec. 2023 kl. 09.03 skrev Eli Zaretskii <eliz <at> gnu.org>:
> Mattias, any comments, or should I install this?
Well, the patch doesn't look too unreasonable so installing it would leave us better off than before.
Of course we may want to try to do better if this is really a bottleneck. The big job was to detect and locate the inefficiency -- thank you, Raffael! -- so his efforts were instrumental in any case.
There are minor points that could be addressed: `mapc` is often better replaced with `dolist`; the first position of a buffer is 1, not 0; and perhaps iterating through all elements of math-read-replacement-list isn't ideal.
Here's a variant which computes a single regexp to do the job and should cons a bit less.
Raffael, maybe you could see if this makes a difference in Org table performance.
[math-read-preprocess-string.diff (application/octet-stream, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sat, 02 Dec 2023 19:42:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 67536 <at> debbugs.gnu.org (full text, mbox):
Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
> There are minor points that could be addressed: `mapc` is often better
> replaced with `dolist`;
Is this the case? As ‘mapc’ is implemented directly in C and ‘dolist’
falls back to ‘while’, I thought it would be the other way around. But
of course I never compared them.
> the first position of a buffer is 1, not 0;
Yes, I could have done that one better. I was to lazy to check...
> and perhaps iterating through all elements of math-read-replacement-list isn't ideal.
> Here's a variant which computes a single regexp to do the job and should cons a bit less.
I thought about the possibility of doing this, but I didn't want to open
that can of worms. Not iterating definitely sounds right, though.
However, I checked your proposed version and there is an issue with
replacement of sub- and superscripts. Here are a few test strings I
used on my version, with the comparison strings supplied by the
original:
--8<---------------cut here---------------start------------->8---
(string= (math-read-preprocess-string "±⁷⁽⁽⁽⁽₄₄ds₇⅟⅟l⅛⅛µ3¾⁶⁴₍∞≤")
"+/-^(7(((()_(44)ds_(7)1:1:l(1:8)(1:8)μ3(3:4)^(64)_(()inf<=")
(string= (math-read-preprocess-string "dsfjlsajflk klfsld flsd fkls fkl jfjls")
"dsfjlsajflk klfsld flsd fkls fkl jfjls")
(string= (math-read-preprocess-string "") "")
--8<---------------cut here---------------end--------------->8---
The last two tests work fine (as there are no replacements), but for the
first, your function produces this incorrect result:
"+/-^(7)^(()^(()^(()^(()_(4)_(4)ds_(7)1:1:l(1:8)(1:8)μ3(3:4)^(6)^(4)_(()inf<="
> Raffael, maybe you could see if this makes a difference in Org table performance.
I'm looking forward to doing so.
Regards,
Raffael
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sun, 03 Dec 2023 10:44:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 67536 <at> debbugs.gnu.org (full text, mbox):
2 dec. 2023 kl. 20.26 skrev Raffael Stocker <r.stocker <at> mnet-mail.de>:
> Is this the case? As ‘mapc’ is implemented directly in C and ‘dolist’
> falls back to ‘while’, I thought it would be the other way around.
It's the function calls that are expensive. `dolist` just expands to a loop.
It's even worse if the lambda expression accesses variables outside (which wasn't the case here) because that forces creation of a closure.
> However, I checked your proposed version and there is an issue with
> replacement of sub- and superscripts.
Look at that, I got the semantics wrong. Sorry about that. Here's a new patch.
A lot less pretty this time.
In any case, make sure to include unit tests in your final patch.
The whole problem is compounded by the public variables (math-read-replacement-list etc) that we somehow feel a need to keep unchanged just in case some user modifies them, despite this almost certainly never happens.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sun, 03 Dec 2023 11:15:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 67536 <at> debbugs.gnu.org (full text, mbox):
Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
> Look at that, I got the semantics wrong. Sorry about that. Here's a new patch.
Is it possible you forgot the attachment?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sun, 03 Dec 2023 11:59:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> Is it possible you forgot the attachment?
How dare you suggest such a thing!
[math-read-preprocess-string.diff (application/octet-stream, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Tue, 05 Dec 2023 19:30:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
> Here's a new patch.
> A lot less pretty this time.
>
> In any case, make sure to include unit tests in your final patch.
Ok, here it comes.
I have constructed two org tables for the test (see test-input.org) that
are reasonably long and contain a few not too unreasonable formulas.
The nature of the formulas is not too important for the tests, I just
took them from my original "offending" table.
I also added unit tests for the function, as requested.
One of the tables has no special characters, so
‘math-read-preprocess-string’ just has to walk through without doing any
real work (arguably the most common case). The other table is full of
replaceable stuff. I used these two tables to compare the original with
the new version of ‘math-read-preprocess-string’. The results are in
test-results.org.
As before, I compared ‘gcs-done’ and ‘gc-elapsed’ (with standard
settings for ‘gc-cons-threshold’ and ‘gc-cons-percentage’) with both
versions and tables. I also instrumented ‘org-table-recalculate’ and
‘math-read-preprocess-string’ using elp. Then I set ‘gc-cons-threshold’
to a large value and ran the same tests again. For the latter test, I
only report ‘elp-results’ (as no GC was triggered).
I recalculated every table three times per reported elp result, as it
takes three evaluations for the calculation to converge.
Some interesting results are:
- time spent in GC reduces from 2.11 s to 0.78 s for the second table
(with replacements)
- elp report of average time decreases from 4.6316224e-3 s to
3.0294569e-4 s for the second table
- with GC prevented, elp reports avg. time 3.4286452e-4 s vs. 4.9444e-5 s
- elp report of elapsed time for ‘org-table-recalculate’ for three evaluations
(what the user sees) decreases from 8.36 s to 4.11 s (with standard GC
settings again)
This is a real win especially for large tables. Needless to say, I am
very excited about this optimization.
(Just for completeness, I quickly compared my version to Mattias'
version, the latter is about a factor of two faster according to elp.)
Regards,
Raffael
[0001-lisp-calc-calc-aent.el-math-read-preprocess-string-o.patch (text/x-patch, attachment)]
[test-input.org (text/x-org, attachment)]
[test-results.org (text/x-org, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Sat, 16 Dec 2023 09:42:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 67536 <at> debbugs.gnu.org (full text, mbox):
Mattias, is this okay with you? Should I install these patches?
> From: Raffael Stocker <r.stocker <at> mnet-mail.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
> 67536 <at> debbugs.gnu.org
> Date: Tue, 05 Dec 2023 19:14:36 +0100
>
> Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
>
> > Here's a new patch.
> > A lot less pretty this time.
> >
> > In any case, make sure to include unit tests in your final patch.
>
> Ok, here it comes.
>
> I have constructed two org tables for the test (see test-input.org) that
> are reasonably long and contain a few not too unreasonable formulas.
> The nature of the formulas is not too important for the tests, I just
> took them from my original "offending" table.
>
> I also added unit tests for the function, as requested.
>
> One of the tables has no special characters, so
> ‘math-read-preprocess-string’ just has to walk through without doing any
> real work (arguably the most common case). The other table is full of
> replaceable stuff. I used these two tables to compare the original with
> the new version of ‘math-read-preprocess-string’. The results are in
> test-results.org.
>
> As before, I compared ‘gcs-done’ and ‘gc-elapsed’ (with standard
> settings for ‘gc-cons-threshold’ and ‘gc-cons-percentage’) with both
> versions and tables. I also instrumented ‘org-table-recalculate’ and
> ‘math-read-preprocess-string’ using elp. Then I set ‘gc-cons-threshold’
> to a large value and ran the same tests again. For the latter test, I
> only report ‘elp-results’ (as no GC was triggered).
>
> I recalculated every table three times per reported elp result, as it
> takes three evaluations for the calculation to converge.
>
> Some interesting results are:
>
> - time spent in GC reduces from 2.11 s to 0.78 s for the second table
> (with replacements)
> - elp report of average time decreases from 4.6316224e-3 s to
> 3.0294569e-4 s for the second table
> - with GC prevented, elp reports avg. time 3.4286452e-4 s vs. 4.9444e-5 s
> - elp report of elapsed time for ‘org-table-recalculate’ for three evaluations
> (what the user sees) decreases from 8.36 s to 4.11 s (with standard GC
> settings again)
>
> This is a real win especially for large tables. Needless to say, I am
> very excited about this optimization.
>
> (Just for completeness, I quickly compared my version to Mattias'
> version, the latter is about a factor of two faster according to elp.)
>
> Regards,
> Raffael
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Mon, 18 Dec 2023 10:56:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
16 dec. 2023 kl. 10.40 skrev Eli Zaretskii <eliz <at> gnu.org>:
> Mattias, is this okay with you? Should I install these patches?
So very sorry for the hiatus. I've been a bit poorly and getting back will take some time.
Raffael, thank you for your dogged measurements. Of course my clumsy code didn't preserve the possibility for `math-read-replacement-list` to translate strings longer than a single character but that's what I get for sending off-cuff patches that way.
Here's another off-cuff patch that might work better (demonstrating that I've learned nothing).
Again it's most likely that no user ever changes `math-read-replacement-list ` and the code could be simplified a lot.
[calc-aent.diff (application/octet-stream, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Mon, 18 Dec 2023 11:46:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 67536 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
> Of course my clumsy code didn't preserve the possibility for
> `math-read-replacement-list` to translate strings longer than a single
> character but that's what I get for sending off-cuff patches that way.
...and my test did not catch that edge case. I extended the test with
this and an empty ‘math-read-replacement-list’ for good measure
(although I don't quite know which use case that might be). The test
fails for the previous version and succeeds for the original and this
new one.
Have I missed any other edge cases in the test?
I appended the updated patch.
[0001-lisp-calc-calc-aent.el-math-read-preprocess-string-o.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Tue, 19 Dec 2023 16:17:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 67536 <at> debbugs.gnu.org (full text, mbox):
18 dec. 2023 kl. 12.39 skrev Raffael Stocker <r.stocker <at> mnet-mail.de>:
> ...and my test did not catch that edge case. I extended the test with
> this and an empty ‘math-read-replacement-list’ for good measure
> (although I don't quite know which use case that might be). The test
> fails for the previous version and succeeds for the original and this
> new one.
Thanks, now pushed to master.
The code could be sped up further if necessary but I suppose this will do for now. Otherwise, let us know.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67536; Package
emacs.
(Tue, 19 Dec 2023 17:11:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 67536 <at> debbugs.gnu.org (full text, mbox):
Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
> 18 dec. 2023 kl. 12.39 skrev Raffael Stocker <r.stocker <at> mnet-mail.de>:
>
>> ...and my test did not catch that edge case. I extended the test with
>> this and an empty ‘math-read-replacement-list’ for good measure
>> (although I don't quite know which use case that might be). The test
>> fails for the previous version and succeeds for the original and this
>> new one.
>
> Thanks, now pushed to master.
> The code could be sped up further if necessary but I suppose this will do for now. Otherwise, let us know.
Yes, I think this will be fine. Thanks!
Reply sent
to
Mattias Engdegård <mattias.engdegard <at> gmail.com>:
You have taken responsibility.
(Tue, 19 Dec 2023 18:16:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Raffael Stocker <r.stocker <at> mnet-mail.de>:
bug acknowledged by developer.
(Tue, 19 Dec 2023 18:16:02 GMT)
Full text and
rfc822 format available.
Message #64 received at 67536-done <at> debbugs.gnu.org (full text, mbox):
19 dec. 2023 kl. 18.09 skrev Raffael Stocker <r.stocker <at> mnet-mail.de>:
> Yes, I think this will be fine. Thanks!
Closing.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Wed, 17 Jan 2024 12:24:11 GMT)
Full text and
rfc822 format available.
This bug report was last modified 1 year and 115 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.