Subject: Re: bug#70036: a fix that

From: Ihor Radchenko <[email-masked]>
Date: Tue, 30 Apr 2024 11:30:39 +0000

Eli Zaretskii <[email-masked]> writes:

>> There are some easy things that can be done to improve `file-truename'
>> performance somewhat. For example, the number of calls to
>> `file-name-nondirectory' can be trivially reduced in the `file-truename'
>> code - it is called up to three times in a row.
>> 
>> (see my earlier message in https://yhetil.org/emacs-bugs/87jzlmd831.fsf@localhost/)
>> 
>> Will it be of interest?
>
> Yes, of course.  Those kinds of changes are no-brainers, really.

See the attached patch.

>From cde58b309588008707cc8b00919eb24801e42eb6 Mon Sep 17 00:00:00 2001
Message-ID: <[email-masked]>
From: Ihor Radchenko <[email-masked]>
Date: Tue, 30 Apr 2024 14:27:04 +0300
Subject: [PATCH] Improve performance of `file-truename' (bug#70036)

* lisp/files.el (file-truename): Avoid repetitive calls to
`file-name-nondirectory'.  These calls contribute significantly to CPU
time.  See the benchmarks in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70036#47
---
 lisp/files.el | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 7dec67c5cf0..b7ebb727c72 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1504,27 +1504,28 @@ file-truename
 			(new (file-name-as-directory (file-truename dirfile counter prev-dirs))))
 		    (setcar prev-dirs (cons (cons old new) (car prev-dirs)))
 		    (setq dir new))))
-	    (if (equal ".." (file-name-nondirectory filename))
-		(setq filename
-		      (directory-file-name (file-name-directory (directory-file-name dir)))
-		      done t)
-	      (if (equal "." (file-name-nondirectory filename))
-		  (setq filename (directory-file-name dir)
-			done t)
-		;; Put it back on the file name.
-		(setq filename (concat dir (file-name-nondirectory filename)))
-		;; Is the file name the name of a link?
-		(setq target (file-symlink-p filename))
-		(if target
-		    ;; Yes => chase that link, then start all over
-		    ;; since the link may point to a directory name that uses links.
-		    ;; We can't safely use expand-file-name here
-		    ;; since target might look like foo/../bar where foo
-		    ;; is itself a link.  Instead, we handle . and .. above.
-		    (setq filename (files--splice-dirname-file dir target)
-			  done nil)
-		  ;; No, we are done!
-		  (setq done t))))))))
+            (let ((filename-no-dir (file-name-nondirectory filename)))
+	      (if (equal ".." filename-no-dir)
+		  (setq filename
+		        (directory-file-name (file-name-directory (directory-file-name dir)))
+		        done t)
+	        (if (equal "." filename-no-dir)
+		    (setq filename (directory-file-name dir)
+			  done t)
+		  ;; Put it back on the file name.
+		  (setq filename (concat dir filename-no-dir))
+		  ;; Is the file name the name of a link?
+		  (setq target (file-symlink-p filename))
+		  (if target
+		      ;; Yes => chase that link, then start all over
+		      ;; since the link may point to a directory name that uses links.
+		      ;; We can't safely use expand-file-name here
+		      ;; since target might look like foo/../bar where foo
+		      ;; is itself a link.  Instead, we handle . and .. above.
+		      (setq filename (files--splice-dirname-file dir target)
+			    done nil)
+		    ;; No, we are done!
+		    (setq done t)))))))))
     filename))
 
 (defun file-chase-links (filename &optional limit)
-- 
2.44.0

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>