From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Raffael Stocker Newsgroups: gmane.emacs.bugs Subject: bug#67536: 29.1; Calc mode's math-read-preprocess-string conses unnecessarily Date: Mon, 18 Dec 2023 12:39:01 +0100 Message-ID: References: <83h6l367zx.fsf@gnu.org> <83jzpzaw9s.fsf@gnu.org> <834jh1aj1d.fsf@gnu.org> <83lead81zq.fsf@gnu.org> <1B1320A5-9FE9-4747-8A07-6F7FF71E8840@gmail.com> <83y1dumqnu.fsf@gnu.org> <11C36862-05F6-47B2-AFEB-D51B969EAAA5@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13487"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: mu4e 1.10.8; emacs 29.1 Cc: Eli Zaretskii , monnier@iro.umontreal.ca, 67536@debbugs.gnu.org To: Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Dec 18 12:46:40 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rFC52-0003JO-9C for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 18 Dec 2023 12:46:40 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rFC4h-00081x-RA; Mon, 18 Dec 2023 06:46:19 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rFC4O-0007w5-GT for bug-gnu-emacs@gnu.org; Mon, 18 Dec 2023 06:46:09 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rFC4N-00068O-Rm for bug-gnu-emacs@gnu.org; Mon, 18 Dec 2023 06:46:00 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rFC4P-0003Zx-Ls for bug-gnu-emacs@gnu.org; Mon, 18 Dec 2023 06:46:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Raffael Stocker Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 18 Dec 2023 11:46:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 67536 X-GNU-PR-Package: emacs Original-Received: via spool by 67536-submit@debbugs.gnu.org id=B67536.17028999228573 (code B ref 67536); Mon, 18 Dec 2023 11:46:01 +0000 Original-Received: (at 67536) by debbugs.gnu.org; 18 Dec 2023 11:45:22 +0000 Original-Received: from localhost ([127.0.0.1]:59539 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rFC3l-0002Ck-Bo for submit@debbugs.gnu.org; Mon, 18 Dec 2023 06:45:22 -0500 Original-Received: from mail-out.m-online.net ([212.18.0.9]:33241) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rFC3j-00024E-0W for 67536@debbugs.gnu.org; Mon, 18 Dec 2023 06:45:20 -0500 Original-Received: from frontend01.mail.m-online.net (unknown [192.168.8.182]) by mail-out.m-online.net (Postfix) with ESMTP id 4StygH0YmJz1qsP8; Mon, 18 Dec 2023 12:45:14 +0100 (CET) Original-Received: from localhost (dynscan1.mnet-online.de [192.168.6.68]) by mail.m-online.net (Postfix) with ESMTP id 4StygG5gd0z1qqlW; Mon, 18 Dec 2023 12:45:14 +0100 (CET) X-Virus-Scanned: amavis at mnet-online.de Original-Received: from mail.mnet-online.de ([192.168.8.182]) by localhost (dynscan1.mail.m-online.net [192.168.6.68]) (amavis, port 10024) with ESMTP id KHWmxU53ck8V; Mon, 18 Dec 2023 12:45:13 +0100 (CET) X-Auth-Info: P682YascQw/Er2gprlxQ0OS/tvg4K+BJt727CVjZowaO/pp5+URrVR1dw0b8h/FS Original-Received: from Whiteflame (ppp-212-114-182-217.dynamic.mnet-online.de [212.114.182.217]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.mnet-online.de (Postfix) with ESMTPSA; Mon, 18 Dec 2023 12:45:13 +0100 (CET) In-reply-to: <11C36862-05F6-47B2-AFEB-D51B969EAAA5@gmail.com> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:276459 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mattias Engdeg=C3=A5rd 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 =E2=80=98math-read-replacement-list=E2=80=99 for good mea= sure (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. --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: attachment; filename=0001-lisp-calc-calc-aent.el-math-read-preprocess-string-o.patch Content-Transfer-Encoding: quoted-printable Content-Description: 0001-lisp-calc-calc-aent.el-math-read-preprocess-string-o.patch >From 66f63f993d87161cfeaf07366eb9dc68738b5891 Mon Sep 17 00:00:00 2001 From: Raffael Stocker Date: Mon, 18 Dec 2023 12:35:33 +0100 Subject: [PATCH] * lisp/calc/calc-aent.el (math-read-preprocess-string): optimize (bug#67536) MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit This is an optimized version of =E2=80=98math-read-preprocess-string=E2=80= =99 by Mattias Engdeg=C3=A5rd , with unit tests. This function is called by calc-eval, which in turn is called repeatedly when re-calculating org-mode tables. It was one of the main bottlenecks there and this optimization addresses this by not repeatedly allocating new strings while iterating through the replacement list, but instead working through the argument string only once, using a "flattened" and cached regexp and only one call to =E2=80=98replace-regexp-in-string=E2=80=99. --- lisp/calc/calc-aent.el | 45 +++++++++++++++++++++++++----------- test/lisp/calc/calc-tests.el | 37 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/lisp/calc/calc-aent.el b/lisp/calc/calc-aent.el index 66ede3295ae..ae04a850f5d 100644 --- a/lisp/calc/calc-aent.el +++ b/lisp/calc/calc-aent.el @@ -547,22 +547,41 @@ math-read-subscripts "=E2=82=80=E2=82=81=E2=82=82=E2=82=83=E2=82=84=E2=82=85=E2=82=86=E2=82= =87=E2=82=88=E2=82=89=E2=82=8A=E2=82=8B=E2=82=8D=E2=82=8E" ; 0123456789+-() "A string consisting of the subscripts allowed by Calc.") =20 +(defvar math--read-preprocess-re-cache nil + "Cached regexp and tag: (REGEXP REPLACEMENTS SUPERSCRIPTS SUBSCRIPTS)") + ;;;###autoload (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 - (setq str - (replace-regexp-in-string (nth 0 (car rep-list)) - (nth 1 (car rep-list)) str)) - (setq rep-list (cdr rep-list)))) - str) + (unless (and (eq (nth 1 math--read-preprocess-re-cache) + math-read-replacement-list) + (eq (nth 2 math--read-preprocess-re-cache) + math-read-superscripts) + (eq (nth 3 math--read-preprocess-re-cache) + math-read-subscripts)) + ;; Cache invalid, recompute. + (setq math--read-preprocess-re-cache + (list (rx-to-string + `(or (or (+ (in ,math-read-superscripts)) + (group (+ (in ,math-read-subscripts)))) + (group (or ,@(mapcar #'car math-read-replacement-lis= t)))) + t) + math-read-replacement-list + math-read-superscripts + math-read-subscripts))) + (replace-regexp-in-string + (nth 0 math--read-preprocess-re-cache) + (lambda (s) + (if (match-beginning 2) + (cadr (assoc s math-read-replacement-list)) + (concat (if (match-beginning 1) "_" "^") + "(" + (mapconcat (lambda (c) + (cadr (assoc (char-to-string c) + math-read-replacement-list))) + s) + ")"))) + str t)) =20 ;; The next few variables are local to math-read-exprs (and math-read-expr ;; in calc-ext.el), but are set in functions they call. diff --git a/test/lisp/calc/calc-tests.el b/test/lisp/calc/calc-tests.el index 5b11dd950ba..e49df216019 100644 --- a/test/lisp/calc/calc-tests.el +++ b/test/lisp/calc/calc-tests.el @@ -816,5 +816,42 @@ calc-nth-root (x (calc-tests--calc-to-number (math-pow 8 '(frac 1 6))))) (should (< (abs (- x (sqrt 2.0))) 1.0e-10)))) =20 +(ert-deftest calc-math-read-preprocess-string () + "Test replacement of allowed special Unicode symbols." + ;; ... doesn't change an empty string + (should (string=3D "" (math-read-preprocess-string ""))) + ;; ... doesn't change a string without characters from + ;; =E2=80=98math-read-replacement-list=E2=80=99 + (let ((str "don't replace here")) + (should (string=3D str (math-read-preprocess-string str)))) + ;; ... replaces irrespective of position in input string + (should (string=3D "^(1)" (math-read-preprocess-string "=C2=B9"))) + (should (string=3D "some^(1)" (math-read-preprocess-string "some=C2=B9")= )) + (should (string=3D "^(1)time" (math-read-preprocess-string "=C2=B9time")= )) + (should (string=3D "some^(1)else" (math-read-preprocess-string "some=C2= =B9else"))) + ;; ... replaces every element of =E2=80=98math-read-replacement-list=E2= =80=99 correctly, + ;; in particular combining consecutive super-/subscripts into one + ;; exponent/subscript + (should (string=3D (concat "+/-*:-/*inf<=3D>=3D<=3D>=3D=CE=BC(1:4)(1:2)(= 3:4)(1:3)(2:3)" + "(1:5)(2:5)(3:5)(4:5)(1:6)(5:6)" + "(1:8)(3:8)(5:8)(7:8)1:^(0123456789+-()ni)" + "_(0123456789+-())") + (math-read-preprocess-string (mapconcat #'car + math-read-repla= cement-list + "")))) + ;; ... replaces strings of more than a single character correctly + (let ((math-read-replacement-list (append + math-read-replacement-list + '(("=F0=9D=9A=A4=F0=9D=9A=A5" "ij")) + '(("=C2=BC=C2=BD" "(1:4)(1:2)"))))) + (should (string=3D "(1:4)(1:2)ij" + (math-read-preprocess-string "=C2=BC=C2=BD=F0=9D=9A= =A4=F0=9D=9A=A5")))) + ;; ... handles an empty replacement list gracefully + (let ((math-read-replacement-list '())) + (should (string=3D "=C2=BC" (math-read-preprocess-string "=C2=BC")))) + ;; ... signals an error if the argument is not a string + (should-error (math-read-preprocess-string nil)) + (should-error (math-read-preprocess-string 42))) + (provide 'calc-tests) ;;; calc-tests.el ends here --=20 2.43.0 --=-=-=--