Subject: Re: bug#28227: 26.0.50; Tramp tests are slow

From: Gemini Lasswell <[email-masked]>
Date: Wed, 30 Aug 2017 18:20:08 -0700

Hi Michael,

Thank you for the test running tips. The reason I noticed that the Tramp
tests were an outlier in terms of time to execute was not that I was
running the entire set of Tramp tests. My pet project is getting as much
as possible of Emacs and its tests to run with Edebug/Testcover code
coverage turned on in order to generate code coverage reports. The
volume of data structures created for code coverage cause garbage
collection to be expensive. Everything runs slower with code coverage
on, but expensive garbage collection makes even the "make check" set of
Tramp tests run extremely slowly.

Looking at the code coverage reports gave me some insight into why Tramp
does so much garbage collection. tramp-file-name-structure gets called
over 2 million times during the full run of Tramp tests, and creates
strings and builds a list each time, but only constructs one of three
possible results depending on the value of tramp-syntax. The result
lists are not modified by the code they are returned to.

In the attached patch I have modified tramp-file-name-structure and its
subroutines so that the three results are calculated once and then
looked up when needed. With this patch the full set of Tramp tests takes
1/3 as long to complete as before.

It's not my use case, but it seems possible that this patch would be a
noticable performance improvement to someone who keeps a long running
Emacs process with lots of data loaded (hence costly garbage collection)
and also uses Tramp heavily over fast network connections.

Besides the patch I'm attaching two coverage reports which you may find
interesting. The code coverage data were created by running the full
set of Tramp tests, one without the patch and one with the patch. The
reports were generated with genhtml from a lcov info file written by a
modified version of Testcover which collects execution counts. The
number between the line number and the code on each line is the maximum
of the execution counts of the forms in that line of code.

Best regards,
Gemini

>From 88d7660ddecd215e48a9dfd7a1bc6efa3f226efc Mon Sep 17 00:00:00 2001
From: Gemini Lasswell <[email-masked]>
Date: Wed, 30 Aug 2017 07:11:41 -0700
Subject: [PATCH] Reduce Tramp's memory usage

Construct Tramp syntax strings and regular expressions once instead
of every time they are used, and store them in alists keyed by Tramp
syntax.
* tramp.el (tramp-build-remote-file-name-spec-regexp)
(tramp-build-file-name-structure): New functions.
(tramp-prefix-format-alist, tramp-prefix-regexp-alist)
(tramp-prefix-method-regexp-alist)
(tramp-postfix-method-format-alist)
(tramp-postfix-method-regexp-alist)
(tramp-prefix-ipv6-format-alist, tramp-prefix-ipv6-regexp-alist)
(tramp-postfix-ipv6-format-alist)
(tramp-postfix-ipv6-regexp-alist)
(tramp-postfix-host-format-alist)
(tramp-postfix-host-regexp-alist)
(tramp-remote-file-name-spec-regexp-alist)
(tramp-file-name-structure-alist): New constants.
(tramp-lookup-syntax): New macro.
(tramp-prefix-format, tramp-prefix-regexp, tramp-method-regexp)
(tramp-postfix-method-format, tramp-postfix-method-regexp)
(tramp-prefix-ipv6-format, tramp-prefix-ipv6-regexp)
(tramp-postfix-ipv6-format, tramp-postfix-ipv6-regexp)
(tramp-postfix-host-format, tramp-postfix-host-regexp)
(tramp-remote-file-name-spec-regexp, tramp-file-name-structure):
Use it.
---
 lisp/net/tramp.el | 173 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 128 insertions(+), 45 deletions(-)

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 1a5cda7e20..3a1d4d843f 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -700,40 +700,70 @@ tramp-syntax-values
     (setq values (mapcar 'last values)
 	  values (mapcar 'car values))))
 
+(defmacro tramp-lookup-syntax (alist)
+  "Look up a syntax string in ALIST according to `tramp-compat-tramp-syntax.'
+Raise an error if `tramp-syntax' is invalid."
+  `(let ((result (cdr (assq (tramp-compat-tramp-syntax) ,alist))))
+     (or result
+         (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+
+(defconst tramp-prefix-format-alist
+  '((default    . "/")
+    (simplified . "/")
+    (separate   . "/["))
+  "Alist mapping Tramp syntax to strings beginning Tramp file names.")
+
 (defun tramp-prefix-format ()
   "String matching the very beginning of Tramp file names.
 Used in `tramp-make-tramp-file-name'."
-  (cond ((eq (tramp-compat-tramp-syntax) 'default) "/")
-	((eq (tramp-compat-tramp-syntax) 'simplified) "/")
-	((eq (tramp-compat-tramp-syntax) 'separate) "/[")
-	(t (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+  (tramp-lookup-syntax tramp-prefix-format-alist))
+
+(defconst tramp-prefix-regexp-alist
+  `((default    . ,(concat "^" (regexp-quote "/")))
+    (simplified . ,(concat "^" (regexp-quote "/")))
+    (separate   . ,(concat "^" (regexp-quote "/["))))
+  "Alist of regexps matching the beginnings of Tramp file names.
+Keyed by Tramp syntax. Derived from `tramp-prefix-format-alist'.")
 
 (defun tramp-prefix-regexp ()
   "Regexp matching the very beginning of Tramp file names.
 Should always start with \"^\". Derived from `tramp-prefix-format'."
-  (concat "^" (regexp-quote (tramp-prefix-format))))
+  (tramp-lookup-syntax tramp-prefix-regexp-alist))
+
+(defconst tramp-prefix-method-regexp-alist
+  '((default    . "[a-zA-Z0-9-]+")
+    (simplified . "")
+    (separate   . "[a-zA-Z0-9-]*"))
+  "Alist mapping Tramp syntax to regexps matching methods identifiers.")
 
 (defun tramp-method-regexp ()
   "Regexp matching methods identifiers.
 The `ftp' syntax does not support methods."
-  (cond ((eq (tramp-compat-tramp-syntax) 'default) "[a-zA-Z0-9-]+")
-	((eq (tramp-compat-tramp-syntax) 'simplified) "")
-	((eq (tramp-compat-tramp-syntax) 'separate) "[a-zA-Z0-9-]*")
-	(t (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+  (tramp-lookup-syntax tramp-prefix-method-regexp-alist))
+
+(defconst tramp-postfix-method-format-alist
+  '((default    . ":")
+    (simplified . "")
+    (separate   . "/"))
+  "Alist mapping Tramp syntax to the delimiter after the method.")
 
 (defun tramp-postfix-method-format ()
   "String matching delimiter between method and user or host names.
 The `ftp' syntax does not support methods.
 Used in `tramp-make-tramp-file-name'."
-  (cond ((eq (tramp-compat-tramp-syntax) 'default) ":")
-	((eq (tramp-compat-tramp-syntax) 'simplified) "")
-	((eq (tramp-compat-tramp-syntax) 'separate) "/")
-	(t (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+  (tramp-lookup-syntax tramp-postfix-method-format-alist))
+
+(defconst tramp-postfix-method-regexp-alist
+  `((default    . ,(regexp-quote ":"))
+    (simplified . ,(regexp-quote ""))
+    (separate   . ,(regexp-quote "/")))
+  "Alist mapping Tramp syntax to regexp matching delimiter after method.
+Derived from `tramp-postfix-method-format-alist'.")
 
 (defun tramp-postfix-method-regexp ()
   "Regexp matching delimiter between method and user or host names.
 Derived from `tramp-postfix-method-format'."
-  (regexp-quote (tramp-postfix-method-format)))
+  (tramp-lookup-syntax tramp-postfix-method-regexp-alist))
 
 (defconst tramp-user-regexp "[^/|: \t]+"
   "Regexp matching user names.")
@@ -769,18 +799,28 @@ tramp-postfix-user-regexp
 (defconst tramp-host-regexp "[a-zA-Z0-9_.-]+"
   "Regexp matching host names.")
 
+(defconst tramp-prefix-ipv6-format-alist
+  '((default    . "[")
+    (simplified . "[")
+    (separate   . ""))
+  "Alist mapping Tramp syntax to strings prefixing IPv6 addresses.")
+
 (defun tramp-prefix-ipv6-format ()
   "String matching left hand side of IPv6 addresses.
 Used in `tramp-make-tramp-file-name'."
-  (cond ((eq (tramp-compat-tramp-syntax) 'default) "[")
-	((eq (tramp-compat-tramp-syntax) 'simplified) "[")
-	((eq (tramp-compat-tramp-syntax) 'separate) "")
-	(t (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+  (tramp-lookup-syntax tramp-prefix-ipv6-format-alist))
+
+(defconst tramp-prefix-ipv6-regexp-alist
+  `((default    . ,(regexp-quote "["))
+    (simplified . ,(regexp-quote "["))
+    (separate   . ,(regexp-quote "")))
+  "Alist mapping Tramp syntax to regexp matching prefix of IPv6 addresses.
+Derived from `tramp-prefix-ipv6-format-alist'")
 
 (defun tramp-prefix-ipv6-regexp ()
   "Regexp matching left hand side of IPv6 addresses.
 Derived from `tramp-prefix-ipv6-format'."
-  (regexp-quote (tramp-prefix-ipv6-format)))
+  (tramp-lookup-syntax tramp-prefix-ipv6-regexp-alist))
 
 ;; The following regexp is a bit sloppy.  But it shall serve our
 ;; purposes.  It covers also IPv4 mapped IPv6 addresses, like in
@@ -789,18 +829,28 @@ tramp-ipv6-regexp
   "\\(?:\\(?:[a-zA-Z0-9]+\\)?:\\)+[a-zA-Z0-9.]+"
   "Regexp matching IPv6 addresses.")
 
+(defconst tramp-postfix-ipv6-format-alist
+  '((default    . "]")
+    (simplified . "]")
+    (separate   . ""))
+  "Alist mapping Tramp syntax to suffix for IPv6 addresses.")
+
 (defun tramp-postfix-ipv6-format ()
   "String matching right hand side of IPv6 addresses.
 Used in `tramp-make-tramp-file-name'."
-  (cond ((eq (tramp-compat-tramp-syntax) 'default) "]")
-	((eq (tramp-compat-tramp-syntax) 'simplified) "]")
-	((eq (tramp-compat-tramp-syntax) 'separate) "")
-	(t (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+  (tramp-lookup-syntax tramp-postfix-ipv6-format-alist))
+
+(defconst tramp-postfix-ipv6-regexp-alist
+  `((default    . ,(regexp-quote "]"))
+    (simplified . ,(regexp-quote "]"))
+    (separate   . ,(regexp-quote "")))
+  "Alist mapping Tramp syntax to regexps matching IPv6 suffixes.
+Derived from `tramp-postfix-ipv6-format-alist'.")
 
 (defun tramp-postfix-ipv6-regexp ()
   "Regexp matching right hand side of IPv6 addresses.
 Derived from `tramp-postfix-ipv6-format'."
-  (regexp-quote (tramp-postfix-ipv6-format)))
+  (tramp-lookup-syntax tramp-postfix-ipv6-format-alist))
 
 (defconst tramp-prefix-port-format "#"
   "String matching delimiter between host names and port numbers.")
@@ -827,18 +877,28 @@ tramp-postfix-hop-regexp
   "Regexp matching delimiter after ad-hoc hop definitions.
 Derived from `tramp-postfix-hop-format'.")
 
+(defconst tramp-postfix-host-format-alist
+  '((default    . ":")
+    (simplified . ":")
+    (separate   . "]"))
+  "Alist mapping Tramp syntax to strings between host and local names.")
+
 (defun tramp-postfix-host-format ()
   "String matching delimiter between host names and localnames.
 Used in `tramp-make-tramp-file-name'."
-  (cond ((eq (tramp-compat-tramp-syntax) 'default) ":")
-	((eq (tramp-compat-tramp-syntax) 'simplified) ":")
-	((eq (tramp-compat-tramp-syntax) 'separate) "]")
-	(t (error "Wrong `tramp-syntax' %s" tramp-syntax))))
+  (tramp-lookup-syntax tramp-postfix-host-format-alist))
+
+(defconst tramp-postfix-host-regexp-alist
+  `((default    . ,(regexp-quote ":"))
+    (simplified . ,(regexp-quote ":"))
+    (separate   . ,(regexp-quote "]")))
+  "Alist mapping Tramp syntax to regexp matching name delimiters.
+Derived from `tramp-postfix-host-format-alist'.")
 
 (defun tramp-postfix-host-regexp ()
   "Regexp matching delimiter between host names and localnames.
 Derived from `tramp-postfix-host-format'."
-  (regexp-quote (tramp-postfix-host-format)))
+  (tramp-lookup-syntax tramp-postfix-host-regexp-alist))
 
 (defconst tramp-localname-regexp ".*$"
   "Regexp matching localnames.")
@@ -851,16 +911,46 @@ tramp-unknown-id-integer
 
 ;;; File name format:
 
+(defun tramp-build-remote-file-name-spec-regexp (syntax)
+  "Construct a regexp matching a Tramp file name for a Tramp SYNTAX."
+  (let ((tramp-syntax syntax))
+    (concat
+     "\\("   (tramp-method-regexp) "\\)" (tramp-postfix-method-regexp)
+     "\\(?:" "\\("   tramp-user-regexp     "\\)" tramp-postfix-user-regexp "\\)?"
+     "\\("   "\\(?:" tramp-host-regexp     "\\|"
+     (tramp-prefix-ipv6-regexp)
+     "\\(?:" tramp-ipv6-regexp "\\)?"
+     (tramp-postfix-ipv6-regexp) "\\)?"
+     "\\(?:" tramp-prefix-port-regexp    tramp-port-regexp "\\)?" "\\)?")))
+
+(defconst tramp-remote-file-name-spec-regexp-alist
+  `((default    . ,(tramp-build-remote-file-name-spec-regexp 'default))
+    (simplified . ,(tramp-build-remote-file-name-spec-regexp 'simplified))
+    (separate   . ,(tramp-build-remote-file-name-spec-regexp 'separate)))
+  "Alist mapping Tramp syntax to regexps matching Tramp file names.")
+
 (defun tramp-remote-file-name-spec-regexp ()
   "Regular expression matching a Tramp file name between prefix and postfix."
-  (concat
-           "\\("   (tramp-method-regexp) "\\)" (tramp-postfix-method-regexp)
-   "\\(?:" "\\("   tramp-user-regexp     "\\)" tramp-postfix-user-regexp "\\)?"
-   "\\("   "\\(?:" tramp-host-regexp     "\\|"
-	           (tramp-prefix-ipv6-regexp)
-		   "\\(?:" tramp-ipv6-regexp "\\)?"
-                   (tramp-postfix-ipv6-regexp) "\\)?"
-	   "\\(?:" tramp-prefix-port-regexp    tramp-port-regexp "\\)?" "\\)?"))
+  (tramp-lookup-syntax tramp-remote-file-name-spec-regexp-alist))
+
+(defun tramp-build-file-name-structure (syntax)
+  "Construct the Tramp file name structure for SYNTAX.
+See `tramp-file-name-structure'."
+  (let ((tramp-syntax syntax))
+    (list
+     (concat
+      (tramp-prefix-regexp)
+      "\\(" "\\(?:" (tramp-remote-file-name-spec-regexp)
+      tramp-postfix-hop-regexp "\\)+" "\\)?"
+      (tramp-remote-file-name-spec-regexp) (tramp-postfix-host-regexp)
+      "\\(" tramp-localname-regexp "\\)")
+     5 6 7 8 1)))
+
+(defconst tramp-file-name-structure-alist
+  `((default    . ,(tramp-build-file-name-structure 'default))
+    (simplified . ,(tramp-build-file-name-structure 'simplified))
+    (separate   . ,(tramp-build-file-name-structure 'separate)))
+  "Alist mapping Tramp syntax to the file name structure for that syntax.")
 
 (defun tramp-file-name-structure ()
   "List of six elements (REGEXP METHOD USER HOST FILE HOP), detailing \
@@ -881,14 +971,7 @@ tramp-file-name-structure
 means the opening parentheses are counted to identify the pair.
 
 See also `tramp-file-name-regexp'."
-  (list
-   (concat
-    (tramp-prefix-regexp)
-    "\\(" "\\(?:" (tramp-remote-file-name-spec-regexp)
-                  tramp-postfix-hop-regexp "\\)+" "\\)?"
-    (tramp-remote-file-name-spec-regexp) (tramp-postfix-host-regexp)
-    "\\(" tramp-localname-regexp "\\)")
-   5 6 7 8 1))
+  (tramp-lookup-syntax tramp-file-name-structure-alist))
 
 (defun tramp-file-name-regexp ()
   "Regular expression matching file names handled by Tramp.
-- 
2.12.2