Consider replacing shell-command-to-string invocations with direct call-process invocations

[colonelpanic8]

Projectile makes extensive use of the shell-command-to-string elisp function when it uses the 'alien indexing method. The advantage of this is, I presume, that the appropriate binary will automatically be detected based on the users shell configuration.

However, for people with non-trivial shell startup times there is a somewhat steep performance penalty to this approach. I have an extensive zsh configuration, but I have made an effort to keep it as lean as possible, and I still see a significant performance penalty to shell-command-to-string.

I used this measure-time macro to investigate this

(defmacro measure-time (&rest body)
  "Measure and return the running time of the code block."
  (declare (indent defun))
  (let ((start (make-symbol "start")))
    `(let ((,start (float-time)))
       ,@body
       (- (float-time) ,start))))

Unfortunately, because of the way that git works, we need to invoke the git-ls-files command directly. (supplying the ls-files argument doesnt work.

(measure-time
  (call-process "/usr/local/opt/git/libexec/git-core/git-ls-files" nil "git-ls-output" "-zco" "--exclude-standard")) => 0.02278304100036621
(measure-time
  (shell-command-to-string projectile-git-command)) => 0.5086848735809326

This is an order of magnitude difference, and because projectile actually invokes MULTIPLE shell commands for each emacs command, this penalty is paid several times.

I realize that rearchitechting this would be somewhat difficult, but would you be open to a change to use call-process when the path to git subcommands is explicitly specified?

Projectile version information

Projectile version: 0.13

Emacs version

E.g. 25.1

Operating system

OSX Yosemite

[colonelpanic8]

Actually, I was able to get something working that optionally skips shell-command-to-string if emacs can find the relevant binary itself.

(defun projectile-call-process-to-string (program &rest args)
  (with-temp-buffer
     (apply 'call-process program nil (current-buffer) nil args)
     (buffer-string)))

(defun projectile-shell-command-to-string (command)
  (cl-destructuring-bind
      (the-command . args) (split-string command " ")
    (let ((binary-path (eshell-search-path the-command)))
      (if binary-path
          (apply 'projectile-call-process-to-string binary-path args)
        (shell-command-to-string command)))))

(defun projectile-files-via-ext-command (command)
  "Get a list of relative file names in the project root by executing COMMAND."
  (split-string (projectile-shell-command-to-string command) "\0" t))

This will obviously only work if (getenv "PATH") returns what it needs to for the given command.

This change makes things MUCH MUCH faster for me.

The downside to this approach is that it means that projectile will need to depend on eshell.

An alternative approach could be to simply include the eshell search path function in projectile itself. The function is pretty short:

(defun eshell-parse-colon-path (path-env)
  "Split string with `parse-colon-path'.
Prepend remote identification of `default-directory', if any."
  (let ((remote (file-remote-p default-directory)))
    (if remote
    (mapcar
     (lambda (x) (concat remote x))
     (parse-colon-path path-env))
      (parse-colon-path path-env))))

(defun eshell-search-path (name)
  "Search the environment path for NAME."
  (if (file-name-absolute-p name)
      name
    (let ((list (eshell-parse-colon-path eshell-path-env))
      suffixes n1 n2 file)
      (if (eshell-under-windows-p)
          (push "." list))
      (while list
    (setq n1 (concat (car list) name))
    (setq suffixes eshell-binary-suffixes)
    (while suffixes
      (setq n2 (concat n1 (car suffixes)))
      (if (and (or (file-executable-p n2)
               (and eshell-force-execution
                (file-readable-p n2)))
           (not (file-directory-p n2)))
          (setq file n2 suffixes nil list nil))
      (setq suffixes (cdr suffixes)))
    (setq list (cdr list)))
      file)))

[patbl]

Should this be reopened?

[colonelpanic8]

@patbl yeah, I think so. I think it would probably be possible to replace all external calls with call-process calls

[patbl]

I switched to counsel-git (part of Ivy), which doesn't have this performance issue. The downside is that you have to put spaces between the words in file names. (There's an option not to, but it's impracticably slow in large projects.)