bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2475 bytes --]

On 11/7/2021 10:37 AM, Michael Albinus wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
> 
>> Thanks for the pointers. I've attached a new version of the patch,
>> along with updated benchmark results. When abbreviating Tramp files,
>> not only is this version faster than my previous patch, it's also 2-4x
>> faster(!) than Emacs trunk.
> 
> Thanks, it looks very promising. According to the benchmarks I'm not
> surprised, because you use Tramp caches.

Hmm, actually it turns out that my patch was only this fast because I 
forgot to check whether the host has case-sensitive file names or not. 
Adding that check back in slows things down again. How I update my 
previous patch will depend on whether we can make 
`file-name-case-insensitive-p' fast for Tramp files, so I'll just focus 
on this part for now and then follow up on the other parts of your 
message after we've decided on what to do here.

Currently on case-sensitive hosts, 
`tramp-handle-file-name-case-insensitive-p' performs its checks on the 
connection every time this function is called. The beginning of tramp.el 
says the following:

   * `tramp-case-insensitive'
     Whether the remote file system handles file names case insensitive.
     Only a non-nil value counts, the default value nil means to
     perform further checks on the remote host.  See
     `tramp-connection-properties' for a way to overwrite this.

I interpret this to mean that Tramp *intentionally* performs checks on 
the host every time if the result is nil. Is there a reason this is 
necessary? Are there any systems out there where the check would return 
nil, but it's still case-insensitive in some cases? Even if there are, I 
imagine that *most* of the time, this check is reliable, and it would be 
nice if we could cache the result for case-sensitive hosts.

I've attached the beginnings of a patch to do this. What do you think? 
If the general idea makes sense, I'll finish it up and file a separate 
bug to track it. If Tramp needs to perform the checks every time for 
some remote hosts, maybe the user could set `tramp-case-insensitive' to 
`never-cache' for those connections?

> Thanks. I've kept that patch on hold for a while. During my illness, it
> got applied, and so you did the dirty task to rearrange everything. I've
> pushed it in your name to the master branch. Thanks.

I hope your health is doing better now. Thanks again for taking a look 
at this patch (and merging the two smaller ones).

[-- Attachment #2: tramp-cache-case-sensitive.patch --]
[-- Type: text/plain, Size: 5432 bytes --]

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 740cb23ebe..9393cfe589 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -1469,21 +1469,22 @@ tramp-file-name-equal-p
        (equal (tramp-file-name-unify vec1)
 	      (tramp-file-name-unify vec2))))
 
-(defun tramp-get-method-parameter (vec param)
+(defun tramp-get-method-parameter (vec param &optional default)
   "Return the method parameter PARAM.
 If VEC is a vector, check first in connection properties.
 Afterwards, check in `tramp-methods'.  If the `tramp-methods'
-entry does not exist, return nil."
+entry does not exist, return DEFAULT."
   (let ((hash-entry
 	 (replace-regexp-in-string "^tramp-" "" (symbol-name param))))
     (if (tramp-connection-property-p vec hash-entry)
 	;; We use the cached property.
-	(tramp-get-connection-property vec hash-entry nil)
+	(tramp-get-connection-property vec hash-entry default)
       ;; Use the static value from `tramp-methods'.
-      (when-let ((methods-entry
-		  (assoc
-		   param (assoc (tramp-file-name-method vec) tramp-methods))))
-	(cadr methods-entry)))))
+      (if-let ((methods-entry
+		(assoc
+		 param (assoc (tramp-file-name-method vec) tramp-methods))))
+	  (cadr methods-entry)
+        default))))
 
 ;; The localname can be quoted with "/:".  Extract this.
 (defun tramp-file-name-unquote-localname (vec)
@@ -3482,51 +3483,57 @@ tramp-handle-file-name-case-insensitive-p
   ;; tests will be added in case they are needed.
   (setq filename (expand-file-name filename))
   (with-parsed-tramp-file-name filename nil
-    (or ;; Maybe there is a default value.
-     (tramp-get-method-parameter v 'tramp-case-insensitive)
-
-     ;; There isn't.  So we must check, in case there's a connection already.
-     (and (file-remote-p filename nil 'connected)
-          (with-tramp-connection-property v "case-insensitive"
-	    (ignore-errors
-	      (with-tramp-progress-reporter v 5 "Checking case-insensitive"
-		;; The idea is to compare a file with lower case
-		;; letters with the same file with upper case letters.
-		(let ((candidate
-		       (tramp-compat-file-name-unquote
-			(directory-file-name filename)))
-		      case-fold-search
-		      tmpfile)
-		  ;; Check, whether we find an existing file with
-		  ;; lower case letters.  This avoids us to create a
-		  ;; temporary file.
-		  (while (and (string-match-p
-			       "[[:lower:]]" (tramp-file-local-name candidate))
-			      (not (file-exists-p candidate)))
-		    (setq candidate
-			  (directory-file-name
-			   (file-name-directory candidate))))
-		  ;; Nothing found, so we must use a temporary file
-		  ;; for comparison.  `make-nearby-temp-file' is added
-		  ;; to Emacs 26+ like `file-name-case-insensitive-p',
-		  ;; so there is no compatibility problem calling it.
-		  (unless (string-match-p
-			   "[[:lower:]]" (tramp-file-local-name candidate))
-		    (setq tmpfile
-			  (let ((default-directory
-				  (file-name-directory filename)))
-			    (tramp-compat-funcall
-			     'make-nearby-temp-file "tramp."))
-			  candidate tmpfile))
-		  ;; Check for the existence of the same file with
-		  ;; upper case letters.
-		  (unwind-protect
-		      (file-exists-p
-		       (concat
-			(file-remote-p candidate)
-			(upcase (tramp-file-local-name candidate))))
-		    ;; Cleanup.
-		    (when tmpfile (delete-file tmpfile)))))))))))
+    (let ((case-insensitive
+	   (tramp-get-method-parameter v 'tramp-case-insensitive 'unset)))
+      (if (not (eq case-insensitive 'unset))
+	  ;; Maybe there is a default value.
+	  case-insensitive
+	;; There isn't.  So we must check, in case there's a
+	;; connection already.
+	(and (file-remote-p filename nil 'connected)
+	     (with-tramp-connection-property v "case-insensitive"
+	       (ignore-errors
+		 (with-tramp-progress-reporter v 5 "Checking case-insensitive"
+		   ;; The idea is to compare a file with lower case
+		   ;; letters with the same file with upper case
+		   ;; letters.
+		   (let ((candidate
+			  (tramp-compat-file-name-unquote
+			   (directory-file-name filename)))
+			 case-fold-search
+			 tmpfile)
+		     ;; Check, whether we find an existing file with
+		     ;; lower case letters.  This avoids us to create
+		     ;; a temporary file.
+		     (while (and (string-match-p
+				  "[[:lower:]]"
+				  (tramp-file-local-name candidate))
+				 (not (file-exists-p candidate)))
+		       (setq candidate
+			     (directory-file-name
+			      (file-name-directory candidate))))
+		     ;; Nothing found, so we must use a temporary file
+		     ;; for comparison.  `make-nearby-temp-file' is
+		     ;; added to Emacs 26+ like
+		     ;; `file-name-case-insensitive-p', so there is no
+		     ;; compatibility problem calling it.
+		     (unless (string-match-p
+			      "[[:lower:]]" (tramp-file-local-name candidate))
+		       (setq tmpfile
+			     (let ((default-directory
+				     (file-name-directory filename)))
+			       (tramp-compat-funcall
+				'make-nearby-temp-file "tramp."))
+			     candidate tmpfile))
+		     ;; Check for the existence of the same file with
+		     ;; upper case letters.
+		     (unwind-protect
+			 (file-exists-p
+			  (concat
+			   (file-remote-p candidate)
+			   (upcase (tramp-file-local-name candidate))))
+		       ;; Cleanup.
+		       (when tmpfile (delete-file tmpfile))))))))))))
 
 (defun tramp-handle-file-name-completion
   (filename directory &optional predicate)