Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance issues on OSX #2909

Closed
atomontage opened this issue Dec 28, 2016 · 57 comments
Closed

Performance issues on OSX #2909

atomontage opened this issue Dec 28, 2016 · 57 comments

Comments

@atomontage
Copy link

atomontage commented Dec 28, 2016

Edit by @tarsius: The cause of this has been found and it has been fixed on Emacs' master branch. See this for more information, including how to get a backport of that fix.

Edit by @tarsius: The fix is included in Emacs 26.1.


"2.9.0-34-gfee9c17c"
GIT: 2.11.0

magit-stage of a single file with a 5-line diff is perceivably slow, around 1 second.
magit-unstage also suffers from the same perceived slowdown.

Some profiling data:

  • Stage
magit-stage                                       1           0.8872        0.8872
magit-stage-1                                     1           0.887063      0.887063
magit-run-git                                     1           0.885151      0.885151
magit-process-file                                39          0.855452      0.0219346666
magit-refresh                                     1           0.669937      0.669937
magit-refresh-buffer                              1           0.668759      0.668759
magit-status-refresh-buffer                       1           0.666563      0.666563
magit-git-insert                                  14          0.491143      0.0350816428
magit-git-str                                     30          0.3144180000  0.0104806000
magit-rev-parse-safe                              28          0.2852569999  0.0101877499
magit-git-wash                                    4           0.225156      0.056289
magit-call-git                                    1           0.215206      0.215206
magit-toplevel                                    16          0.2069670000  0.0129354375
magit-call-process                                1           0.195608      0.195608
magit-git-items                                   2           0.1731359999  0.0865679999
  • Unstage
magit-unstage                                     1           0.943516      0.943516
magit-unstage-1                                   1           0.943387      0.943387
magit-run-git                                     1           0.924936      0.924936
magit-process-file                                44          0.912173      0.0207312045
magit-refresh                                     1           0.666068      0.666068
magit-refresh-buffer                              1           0.665038      0.665038
magit-status-refresh-buffer                       1           0.662804      0.662804
magit-git-insert                                  14          0.485324      0.0346659999
magit-git-str                                     35          0.3729509999  0.0106557428
magit-rev-parse-safe                              33          0.3445889999  0.0104420909
magit-call-git                                    1           0.258859      0.258859
magit-toplevel                                    17          0.2507959999  0.0147527058
magit-call-process                                1           0.243106      0.243106
magit-git-wash                                    4           0.207833      0.05195825
magit-git-items                                   2           0.165855      0.0829275
magit-insert-unstaged-changes                     1           0.16204       0.16204
magit-insert-status-headers                       1           0.157472      0.157472
magit-insert-untracked-files                      1           0.152695      0.152695
magit-insert-headers                              1           0.143047      0.143047
magit-insert-head-branch-header                   1           0.143032      0.143032
magit-git-string                                  17          0.116647      0.0068615882
magit-insert-remaining-headers                    1           0.114531      0.114531
magit-process-finish                              1           0.112665      0.112665
magit-mode-get-buffers                            2           0.112007      0.0560035
magit-process-buffer                              2           0.111826      0.055913
  • Command line
❯❯ time git add test

real	0m0.014s
user	0m0.002s
sys	0m0.004s

The amount of time spent for such a frequently-used operation is enough to lead to
annoyance and mental anguish on the part of the user :-]

Also note that I have 30 untracked files and 10 unstaged changes in this repo.
Maybe this plays into magit-refresh/magit-refresh-buffer?

@atomontage
Copy link
Author

More profiling results, this time from a new completely empty repo.
Results come from me staging and unstaging a single 5-line file.

  • Stage
magit-stage                                       1           0.487667      0.487667
magit-stage-untracked                             1           0.487638      0.487638
magit-run-git                                     1           0.485922      0.485922
magit-process-file                                27          0.462281      0.0171215185
magit-rev-parse-safe                              26          0.3255129999  0.0125197307
magit-git-str                                     26          0.3253689999  0.0125141923
magit-call-git                                    1           0.271569      0.271569
magit-toplevel                                    8           0.256617      0.032077125
magit-call-process                                1           0.248202      0.248202
magit-refresh                                     1           0.214344      0.214344
magit-refresh-buffer                              1           0.211877      0.211877
magit-status-refresh-buffer                       1           0.211705      0.211705
magit-process-setup                               1           0.154661      0.154661
magit-mode-get-buffers                            2           0.15148       0.07574
magit-git-insert                                  5           0.07983       0.015966
  • Unstage
magit-unstage                                     1           0.568939      0.568939
magit-unstage-1                                   1           0.568908      0.568908
magit-run-git                                     1           0.547663      0.547663
magit-process-file                                32          0.546206      0.0170689375
magit-rev-parse-safe                              31          0.4087369999  0.0131850645
magit-git-str                                     31          0.4085759999  0.0131798709
magit-call-git                                    1           0.33064       0.33064
magit-toplevel                                    9           0.3180239999  0.0353359999
magit-call-process                                1           0.309545      0.309545
magit-refresh                                     1           0.217015      0.217015
magit-refresh-buffer                              1           0.215841      0.215841
magit-status-refresh-buffer                       1           0.215716      0.215716
magit-process-setup                               1           0.157354      0.157354
magit-process-buffer                              2           0.146106      0.073053
magit-mode-get-buffers                            2           0.1431339999  0.0715669999
magit-process-finish                              1           0.132236      0.132236
magit-git-insert                                  5           0.083577      0.0167154

Half a second+ seems excessive for this case, no?

@tarsius
Copy link
Member

tarsius commented Dec 29, 2016

The cause of this probably is System Integrity Protection a new of OS X El Capitan. You should disable that as described here (you might better instructions elsewhere).

@tarsius tarsius added the support User needs some help label Dec 29, 2016
@atomontage
Copy link
Author

atomontage commented Dec 29, 2016

Not sure how SIP plays into this, can you elaborate? Why would SIP affect git being executed through
magit/Emacs and not the same git being executed outside? The profiling results I posted point
to a number of things contributing to the slowdown.

I use my own git executable which is placed in /opt/local, nothing to do with Xcode.

I instrumented some of the frequently called functions to print their arguments.
For the last scenario I described, completely empty new repo, stage/unstage one 5-file file,
magit-process-file is called "27" times for the stage alone. This looks more than excessive
since it will create a new process running git each time.

process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel) [2 times]
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-cdup) [2 times]
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel) [2 times]
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-cdup) [2 times]
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel)
process-file: (/opt/local/bin/git nil *magit-process: test nil --no-pager --literal-pathspecs -c core.preloadindex=true add -- test.c)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-cdup) [2 times]
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel)
process-file: (/opt/local/bin/git nil nil nil --no-pager --literal-pathspecs -c core.preloadindex=true update-index --refresh)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --verify HEAD)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --git-dir)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-cdup)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true config --list -z)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true status -z --porcelain)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true diff --no-ext-diff --no-prefix --)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true diff --cached --no-ext-diff --no-prefix --)
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --verify refs/stash)
process-file: (/opt/local/bin/git nil nil nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse @{upstream})
process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true symbolic-ref --short HEAD)
process-file: (/opt/local/bin/git nil nil nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse @{upstream})

A lot/most of these calls look completely redundant to me.

@tarsius
Copy link
Member

tarsius commented Dec 29, 2016

Not sure how SIP plays into this, can you elaborate?

No, I don't use macOS and I don't know any more about it. It's just that other users had the same issue before and disabling SIP made it go away, so there is a good change that might work for you too.

Why would SIP affect git being executed through magit/Emacs and not the same git being executed outside? The profiling results I posted point to a number of things contributing to the slowdown.

If calling git is slow, that might never-the-less be unnoticeable but when calling it multiple times as is necessary to refresh magit's status buffer, the slowdown accumulates.

I use my own git executable which is placed in /opt/local, nothing to do with Xcode.

Make sure this is really the case. Emacs and your shell might not use the same $PATH.

I instrumented some of the frequently called functions to print their arguments.

How exactly did you do that? Show my the advice or patch.

For the last scenario I described, completely empty new repo, stage/unstage one 5-file file, magit-process-file is called "27" times for the stage alone.

It is called 27 times to do that and to update the status buffer.

This looks more than excessive since it will create a new process running git each time.

Actually it doesn't. Magit uses a cache now and most of these identical calls to git are actually reduced to just one. Git is still being called more than once, so the effect of whatever makes calling git slow still accumulates. (That's why I am asking for the patch/advice: to see whether it takes the cache into account.)

A lot/most of these calls look completely redundant to me.

In many places we need to know where the repository is, so we have to figure it out. But we use a cache because starting subprocesses is slow on Windows. If you get unlucky, it can also be slow on macOS, probably because of the issue I mentioned.

Please give disabling System Integrity Protection a change. Maybe there is another issue, but I would first like to rule this likely cause out.

@atomontage
Copy link
Author

atomontage commented Dec 29, 2016

If calling git is slow, that might never-the-less be unnoticeable but when calling it multiple times as is
necessary to refresh magit's status buffer, the slowdown accumulates.

Calling git once (to stage) through magit-process-file takes about the same amount of time to calling "git
add" outside Emacs. I've posted timings for both in my previous messages, it's 0.017s vs 0.014s which is
about the same. I don't think the root issue here is the time it takes to call git, it's (as you said) the accumulation of all these calls, that are a lot. Of course it could be that normally git-add should take much less than 0.0xx seconds and it doesn't in my case, but that seems improbable. I will nonetheless verify
without SIP and also on different OS versions.

I use my own git executable which is placed in /opt/local, nothing to do with Xcode.

Make sure this is really the case. Emacs and your shell might not use the same $PATH.

From my previous post here:

process-file: (/opt/local/bin/git nil (t nil) nil --no-pager --literal-pathspecs -c core.preloadindex=true rev-parse --show-toplevel) [2 times]

You can clearly see the path is correct.

I instrumented some of the frequently called functions to print their arguments.

How exactly did you do that? Show my the advice or patch.

I added (message "process-file: %s" args) in the beginning of magit-process-file.
You can see the output in my previous post.

This looks more than excessive since it will create a new process running git each time.

Actually it doesn't. Magit uses a cache now and most of these identical calls to git are actually reduced to just one. Git is still being called more than once, so the effect of whatever makes calling git slow still accumulates. (That's why I am asking for the patch/advice: to see whether it takes the cache into account.)

Through printing the arguments of magit-process-file we can see that process-file ends up getting called 27 times for 1 stage operation, each time creating a new git process. Now, AFAIK, process-file does not use a cache, so if you say that Magit is using a cache now that should be in-effect before you end up calling Emacs process-file. Since we can see the latter get called 27 times, that doesn't seem to be the case here. Maybe it's a configuration issue or maybe something else is causing the Magit cache to be bypassed.
Can you give me some pointers as to variables/functions I need to check to help you track this down?

I will disable SIP and get back to you on that too.

@jguenther
Copy link
Contributor

I am also experiencing this problem (OSX El Capitan, with SIP/rootless already disabled, homebrew git@2.11.0, magit-git-executable explicitly set to the homebrew git and not system git). It's the most noticable when staging/unstaging (not sure when it started unfortunately).

This is the top of a profile from elp (only magit instrumented). I staged and unstaged a 1-line file from a magit status buffer, in a relatively small repo:

magit-run-git                                     2           1.725056      0.862528
magit-process-file                                77          1.662533      0.0215913376
magit-refresh                                     2           1.191127      0.5955635
magit-refresh-buffer                              2           1.172525      0.5862625
magit-status-refresh-buffer                       2           1.172009      0.5860045
magit-unstage                                     1           0.882784      0.882784
magit-unstage-1                                   1           0.88274       0.88274
magit-stage                                       1           0.86318       0.86318
magit-stage-untracked                             1           0.86315       0.86315
magit-git-str                                     59          0.8044480000  0.0136347118
magit-git-insert                                  26          0.6937960000  0.0266844615
magit-rev-parse-safe                              55          0.6741869999  0.0122579454
magit-call-git                                    2           0.5339050000  0.2669525000
magit-call-process                                2           0.48829       0.244145
magit-toplevel                                    38          0.4752409999  0.0125063421
magit-insert-status-headers                       2           0.474912      0.237456
magit-insert-headers                              2           0.43959       0.219795
magit-insert-head-branch-header                   2           0.439552      0.219776
magit-insert-remaining-headers                    2           0.366764      0.183382
magit-mode-get-buffers                            4           0.278153      0.06953825
magit-process-setup                               2           0.266046      0.133023
magit-insert-upstream-branch-header               2           0.235569      0.1177845
magit-git-string                                  26          0.2333819999  0.0089762307
magit-git-items                                   4           0.22027       0.0550675
magit-insert-untracked-files                      2           0.189578      0.094789
magit-process-finish                              2           0.152561      0.0762805
magit-process-unset-mode-line                     2           0.152111      0.0760555
magit-process-buffer                              2           0.13961       0.069805
magit-insert-unpushed-to-upstream                 2           0.136475      0.0682375
magit-insert-tags-header                          2           0.130556      0.065278
magit-rev-verify                                  7           0.126569      0.0180812857
magit-process-set-mode-line                       2           0.1261010000  0.0630505000
magit-insert-unstaged-changes                     2           0.125846      0.062923
magit-git-wash                                    8           0.125328      0.015666
magit-git-exit-code                               6           0.1119690000  0.0186615
magit-rev-format                                  6           0.1063379999  0.017723
magit-insert-log                                  4           0.101463      0.02536575
magit-get                                         30          0.100881      0.0033627
magit-get-all                                     30          0.1007709999  0.0033590333
magit-config-get-from-cached-list                 32          0.1006610000  0.0031456562

I do have some customizations to magit hooks, so if necessary I could rerun this from a clean config if it'd be helpful.

@atomontage
Copy link
Author

atomontage commented Dec 29, 2016

jguenther: In your case, magit-process-file ended up being called 77 (!) times.
That's 77 git invocations for a single stage operation.

Surely there's something wrong here.

EDIT: I tried again on my setup with SIP disabled, no difference at all.
Forgot to mention in my previous posts, all the data I've posted is from a stock Emacs 25.1 with
nothing extra loaded except magit itself that is using a stock configuration.

@jguenther
Copy link
Contributor

77 operations for a stage and then unstage, but yeah that does seem excessive. I will try to do this from an empty emacs config with just magit installed and see if it's any different.

@tarsius
Copy link
Member

tarsius commented Dec 29, 2016

(not sure when it started unfortunately)

But it's a somewhat recent development?

The cache I mentioned earlier only covers "refreshing the status buffer, after something made that necessary". It does not cover "save unsaved file-visiting buffers before refresh" (I think, will have to check. Also is might be fairly simple to change that.) And it also doesn't cover "revert file-visiting buffers after files have changed on disc" (using a cache here would be rather difficult).

@tarsius
Copy link
Member

tarsius commented Dec 29, 2016

Let's say there's room for improvement here... I'll look into this in more depth as soon as possible, probably not before the second half of January though. (Unless I find a quick solution during the next few hours.)

@tarsius tarsius removed the support User needs some help label Dec 29, 2016
@atomontage
Copy link
Author

I could swear it wasn't this slow in the past, otherwise I would have noticed.
I'll try bisecting and see if things got considerably worse or if my memory is playing tricks on me.

@tarsius
Copy link
Member

tarsius commented Dec 29, 2016

magit-process-file is called "27" times for the stage alone. This looks more than excessive since it will create a new process running git each time.

Yes and no.

First, this is the result of very old design decisions (in short "just regenerate everything, when something might have changed") and I plan to address that soon. However that will be a major amount of work, as that will involve completely refactoring many central parts of magit.

Looking at the arguments a bit closer I see that rev-parse --show-cdup/--show-toplevel are being called a bit much. That is probably the result of ecc4114 and this would probably fix it:

diff --git a/lisp/magit-git.el b/lisp/magit-git.el
index db830b93..d5130f79 100644
--- a/lisp/magit-git.el
+++ b/lisp/magit-git.el
@@ -1496,7 +1496,7 @@ (defun magit-config-get-from-cached-list (key)
    (--> key
         (replace-regexp-in-string "\\`[^.]+" #'downcase it t t)
         (replace-regexp-in-string "[^.]+\\'" #'downcase it t t))
-   (magit--with-refresh-cache (list 'config (magit-toplevel))
+   (magit--with-refresh-cache (list 'config default-directory)
      (let ((configs (make-hash-table :test 'equal)))
        (dolist (conf (magit-git-items "config" "--list" "-z"))
          (let* ((nl-pos (cl-position ?\n conf))

@npostavs Do you think this is safe (i.e. never slower than no caching at all)? Not doing it kinda defeats the purpose of using config --list (i.e. calling git more in order to call it less).

As for the other calls

# stage the change
add -- test.c

# update the status buffer
rev-parse --verify HEAD
config --list -z
status -z --porcelain
diff --no-ext-diff --no-prefix --
diff --cached --no-ext-diff --no-prefix --
rev-parse --verify refs/stash                   
symbolic-ref --short HEAD
update-index --refresh
rev-parse @{upstream} (okay this is called twice, why?)

there isn't much that can be done about those without major refactoring.

There are multiple strategies for improving performance that I intend to implement, all essentially boiling down to "don't do things that aren't actually necessary". Or in other words make Magit smarter, by investing the time necessary to make that happen (lot's of work). I actually already intended to write about that fairly soon.

I am afraid I don't see a way to improve this significantly any time soon, i.e. before the rewrite. Except for the thing mentioned above. Also if you can find causes for the slowness that are more concrete than "git is being called to often" (I've known that for a long time), then I might be able to squeeze out a bit of performance here and there.

But overall the time for squeezing is over, what's needed now are some major changes.

@atomontage
Copy link
Author

atomontage commented Dec 29, 2016

@tarsius Ok, I suspected as much, it's sorta obvious from the profiling results. I'll still do the bisect and see if there's a specific point where things got noticeably worse.

@npostavs
Copy link
Member

Looking at the arguments a bit closer I see that rev-parse --show-cdup/--show-toplevel are being called a bit much. That is probably the result of ecc4114

Those calls are not from magit-get-from-cached-list (except for the last), so I don't think that code is relevant.

Most of the calls seem to be from magit-maybe-save-repository-buffers, magit-process-setup (magit-process-set-mode-line), magit-process-finish (magit-process-unset-mode-line).

and this would probably fix it:

@@ -1496,7 +1496,7 @@ (defun magit-config-get-from-cached-list (key)
-   (magit--with-refresh-cache (list 'config (magit-toplevel))
+   (magit--with-refresh-cache (list 'config default-directory)

I used (magit-toplevel) under the assumption it would be cached anyway.

@tarsius
Copy link
Member

tarsius commented Dec 30, 2016

Most of the calls seem to be from magit-maybe-save-repository-buffers, magit-process-setup (magit-process-set-mode-line), magit-process-finish (magit-process-unset-mode-line).

Ah, I think we can optimize the process part. We already store the default-directory in the process object, now we just have to use it. But it's to late here for me to look into it right now.

@tarsius
Copy link
Member

tarsius commented Dec 31, 2016

I am closing this because there's not anything I can do right now, it's a known issue, and I am going to address it eventually anyway. But if you have concrete suggestions for tweaks, then keep them coming.

@tarsius tarsius closed this as completed Dec 31, 2016
@atomontage
Copy link
Author

If it's a known issue can't you leave this open (or rename it to be more generic since it applies to other OSes too) so we can track it?

If you're dead-set on closing this, can you at least update the Performance section of the manual to make it obvious that "calling git too much" can be an issue. Right now the manual mentions refreshing buffers and recreating their content from scratch, which could be a different issue entirely, or it could be the root cause of calling git too much or both as implied by the profiling data (refreshing adds its own overhead on top of the time spent by calling out to git).

If you make it obvious that calling out to git a lot is a problem, others will have more information to get going with, assuming they want to dive in and attempt to fix.

@tarsius
Copy link
Member

tarsius commented Dec 31, 2016

Okay we can keep this open.

If you make it obvious that calling out to git a lot is a problem, others will have more information to get going with, assuming they want to dive in and attempt to fix.

As I said that requires major refactoring, so they won't succeed (unless they spend a few months on it, which I will do).

@tarsius tarsius reopened this Dec 31, 2016
@tarsius tarsius added the duplicate This issue or pull request already exists label Dec 31, 2016
@tarsius
Copy link
Member

tarsius commented Dec 31, 2016

Also I closed this with the intention to very soon write about this in more detail. Once I have done that, I will close this issue again.

@jojojames
Copy link

Will throw my hat in the ring and also say that Linux's Emacs / Magit seem to be way faster than on OSX.

Ubuntu inside a VM on my Windows 10 desktop has instantaneous everything whereas every action on OSX has probably a 500-700 ms delay to it.

It's just surprising staging a file doing OSX -> Remote Into Windows -> Ubuntu in Vmware is faster than native OSX staging a file.

Though my desktop is probably much faster than my macbook 6850K processor vs whatever is in the 2013 Macbook.

It's on my TODO list at some point to test a Linux VM from the Macbook and see if it really is just a hardware thing and not something else intrinsic to OSX causing this slowness.

@jsab
Copy link

jsab commented Mar 16, 2017

I have tried disabling SIP and I confirm it does not fix the issue. If someone has a workaround for this it would be greatly appreciated.

I have also tried a blank emacs config with only magit installed and the issue is still there. (Other places mentioned that performance issues my be related to themes or packages.)

My initial awareness of this issues comes from working on the same repo from a 2013 MBP running linux (magit is blazing fast) and from a 2017 MBP running Sierra (annoying lag on just about every magit operation).

@jsab
Copy link

jsab commented Mar 16, 2017

I have installed VirtualBox and setup a Fedora 25 VM to test @jojojames's hypothesis. Although, using that VM (i.e. dragging windows around, typing in terminal) is slow and laggy, magit is snappy and much faster than on the macOS sierra host.

@npostavs
Copy link
Member

Is it git itself that is slower? Can you time some git commands on the VM and compare to macOS?

@tarsius tarsius removed the duplicate This issue or pull request already exists label Mar 16, 2017
@jsab
Copy link

jsab commented Apr 4, 2017

Benchmark of Aquamacs (http://aquamacs.org/):

(benchmark 1 '(call-process "/usr/bin/true" nil nil nil))         "Elapsed time: 0.016182s"

@aaronjensen
Copy link

aaronjensen commented Apr 8, 2017

For me on macOS 10.11 w/ emacs -Q and emacs 25.1 installed via homebrew I get:

Elapsed time: 0.003509s

Strangely, with my full spacemacs config, it doubles:

Elapsed time: 0.007726s

These are both results of running:

(benchmark 1 '(call-process "/usr/bin/true" nil nil nil))  

What could cause that doubling?

EDIT

It has to do with the frame size. The larger the frame, the longer call-process takes. When I did the above tests, my emacs -Q was a much smaller frame. `emacs -Q* is still slightly faster it seems for the same frame size, but only by about 10ms rather than 35+

@aaronjensen
Copy link

aaronjensen commented Apr 8, 2017

FYI, reported upstream:

http://lists.gnu.org/archive/html/bug-gnu-emacs/2017-04/msg00201.html and YAMAMOTO Mitsuharu has an idea.

@tarsius
Copy link
Member

tarsius commented Apr 8, 2017

Thanks @aaronjensen!

@aaronjensen
Copy link

I can confirm that doing what was suggested makes for a significant improvement. I now understand why people consider magit more than fast enough (who do not use macs). Very exciting 😄

I've submitted a pull to add an experimental patch to homebrew-emacs-plus.

If anyone else wants to try it before it is merged, know that it is experimental, it could cause emacs to hang or who knows what else. That said, if you want to:

$ brew tap d12frosted/emacs-plus
$ brew edit emacs-plus

And add (somewhere with the other patches):

patch do
  url "https://gist.githubusercontent.com/aaronjensen/28e26a6c6c0dc767176bc35d5545d10e/raw/42b53ce8cca354f9a6249ffa63b5773abcef958e/GNU-Emacs-25.1-OS-X-enable-vfork.patch"
  sha256 "37d699b42754994c8aa86f245f418b0e26cf7b4b48f29de58117086663e73ac2"
end

Then:

$ brew reinstall emacs-plus

@jojojames
Copy link

jojojames commented Apr 8, 2017

This is great.

Thanks @jsab @npostavs @aaronjensen @tarsius

I wonder what benchmarks we are getting.

I'm only getting 2-3 ms difference.

Yamamoto Port
(benchmark 1 '(call-process "/usr/bin/true" nil nil nil))
"Elapsed time: 0.011376s"
"Elapsed time: 0.011855s"
"Elapsed time: 0.012701s"
"Elapsed time: 0.010775s"
"Elapsed time: 0.011097s"

Built off Master with

-/* The following solves the problem that Emacs hangs when evaluating
-   (make-comint "test0" "/nodir/nofile" nil "") when /nodir/nofile
-   does not exist.  Also, setsid is not allowed in the vfork child's
-   context as of Darwin 9/Mac OS X 10.5.  */
-#undef HAVE_WORKING_VFORK
-#define vfork fork

(benchmark 1 '(call-process "/usr/bin/true" nil nil nil))
"Elapsed time: 0.008599s"
"Elapsed time: 0.009335s"
"Elapsed time: 0.009192s"
"Elapsed time: 0.008738s"
"Elapsed time: 0.008734s"
"Elapsed time: 0.007592s"

@jsab
Copy link

jsab commented Apr 9, 2017

Hallelujah! The @aaronjensen patch works for me:

(benchmark 1 '(call-process "/usr/bin/true" nil nil nil))         "Elapsed time: 0.002732s"

(That is with my full spacemacs config, running emacs -Q yields a gain of about 0.4 ms.)

Note that for people who originally did not install emacs-plus from source you will need to build from source for the patch to take effect after you added the snippet with brew edit:

brew reinstall --build-from-source emacs-plus

In conclusion, this makes magit as fast in GUI emacs than it is in -nw emacs on macOS.

Big thanks to everyone!

@aaronjensen
Copy link

aaronjensen commented Apr 10, 2017

Hey all, I've updated the patch with some fixes from the mailing list.

Also, here are some updated installation instructions until it is merged in:

$ brew uninstall emacs-plus
$ brew tap aaronjensen/emacs-plus
$ brew reinstall aaronjensen/emacs-plus/emacs-plus --with-vfork

As for benchmarks @jojojames I saw a bigger improvement % wise, but my machine must be faster overall. I went from ~5-6ms to ~1-2ms.

Btw, on the bench it takes out some variability (and probably more closely replicates what magit is actually doing) if you run it 100 times (just divide the result by 100 afterwards):

(benchmark 100 '(call-process "/usr/bin/true" nil nil nil))  

Elapsed time: 0.123614s

I don't have saved numbers, but before the patch I was above 0.500s for the same test.

@jojojames
Copy link

You're right, it's a significant speedup (rebuilt Emacs off master).

Without patch
(benchmark 100 '(call-process "/usr/bin/true" nil nil nil))
"Elapsed time: 1.878366s"

With patch
(benchmark 100 '(call-process "/usr/bin/true" nil nil nil))
"Elapsed time: 0.540134s"

d12frosted pushed a commit to d12frosted/homebrew-emacs-plus that referenced this issue Apr 20, 2017
This significantly speeds up `call-process` which is used heavily by magit.
Ultimately, this makes magit, and probably other things more responsive. This is
backported from master a13eaddce2ddbe3ba0b7f4c81715bc0fcdba99f6.

See http://lists.gnu.org/archive/html/bug-gnu-emacs/2017-04/msg00201.html
and magit/magit#2909
@aaronjensen
Copy link

FYI, the patch is now merged to emacs master:

emacs-mirror/emacs@a13eadd

And it's available by default via homebrew w/ emacs-plus:

https://github.com/d12frosted/homebrew-emacs-plus

@aaronjensen
Copy link

@tarsius Not what you think, but you may be able to consider this closed now that there shouldn't be a significant difference between mac and linux perf.

@tarsius
Copy link
Member

tarsius commented Apr 21, 2017

I have added a summary to the manual and updated the original post above to link to it.

Thanks again to everyone helping with this!

@tarsius tarsius closed this as completed Apr 21, 2017
@tarsius
Copy link
Member

tarsius commented Apr 21, 2017

FYI, the patch is now merged to emacs master:

What other commits have to be cherry-picked to the emacs-25 branch for this to work?

That commit by itself applies, but apparently doesn't compile:
https://www.reddit.com/r/emacs/comments/66o45w/patch_emacs_to_speedup_magit_on_macos/dgk6989/.

@aaronjensen
Copy link

I think it'd be a couple commits. There were changes to expose DEV_TTY which emacs-25 doesn't have yet, so I modified the patch:

https://gist.githubusercontent.com/aaronjensen/f45894ddf431ecbff78b1bcf533d3e6b/raw/6a5cd7f57341aba673234348d8b0d2e776f86719/Emacs-25-OS-X-use-vfork.patch

We could probably figure out exactly which commits are needed, but there could be quite a long thread in that sweater.

@tarsius
Copy link
Member

tarsius commented Apr 22, 2017

Thanks, I am now linking to this backport.

@aaronjensen
Copy link

Unfortunately, it appears that the performance of vfork has regressed in macOS Monterey. As far as I can tell, vfork and fork have the same performance, so if you are sensitive to this, you might want to hold off on upgrading to Monterey.

@tarsius
Copy link
Member

tarsius commented Nov 7, 2021

☹️

@aaronjensen
Copy link

aaronjensen commented Nov 8, 2021

Known issue apparently: https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg02223.html

There's a fix in Emacs macport, but not yet on master and it looks like the original conversation died out.

@aaronjensen
Copy link

emacs-plus now has patches in place for 28 and 29. A patch has been proposed upstream, waiting on that.

@sielicki
Copy link

sielicki commented Jul 2, 2023

For those arriving through searches, emacs-plus both landed and then removed patches to accomodate this, as it was marked as upstreamed:

d12frosted/homebrew-emacs-plus#422
d12frosted/homebrew-emacs-plus#424

Unfortunately the commit messages did not mark the upstream commit(s) which implemented this and I'm unable to find it, but it stands to reason that any up-to-date emacs 28+ package you install on mac should have a fix for this, so no special concern needed by users going forward, regardless of flavor.

@aaronjensen
Copy link

Your second link includes in the PR description the commit to emacs: emacs-mirror/emacs@cc4edea

@sielicki
Copy link

sielicki commented Jul 2, 2023

...Whoops. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants