-
Notifications
You must be signed in to change notification settings - Fork 10
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
Slowness relative to regular magit #9
Comments
Hi @Macavirus, sorry to be slow here. I don't think it does: the overhead of calling delta and converting the ANSI escape sequences to emacs text properties is, I believe, small/negligible relative to the multiple external process calls that magit makes. My personal emacs setup does have some annoying performance issues involving magit which I haven't tracked down: in particular, I find that up/down arrow ( |
(I do need to double-check that still occurs without magit-delta.) It sounds like you might have had something specific in mind yourself though? |
What does the profiler show? |
Good question. I think I looked at this before, but I'll try to do it again when it next happens. |
OK it happened again. It seems to be saying that the time is spent in
(The percentages are low because it captured |
The performance issue I saw was staging and unstaging hunks when there is a huge diff in the repo. For example, famous |
@braineo how is performance in the situation you describe when using magit without magit-delta? |
emacs 27.1.5 build from source, CPU usage is quite different only by showing
With magit-deltaWithout magit-delta(use-package magit-delta
:disabled
:if (executable-find "delta")
:hook
((magit-mode . (lambda () (magit-delta-mode +1))))
:config
;; Cannot use line number feature of delta in magit. refer to https://github.com/dandavison/magit-delta/issues/13
(setq magit-delta-delta-args (append magit-delta-delta-args '("--features" "magit-delta"))))
|
Thanks, interesting! Is it a public repo? If so a commit / any other instructions to reproduce would be great. |
That will be trivial Any nodejs projects will do, for example
If it freezes emacs just change how many commits you want to leap back 😛 |
It really slowed down my magit, even though the same stuff seems to work fine on command line git. had to disable the mode (these were some not-so-large .tfstate files) |
I had to disable it too. Having |
The workaround I'm doing now is enabling (defvar nth/magit-delta-point-max 50000)
;; Disable mode if there are too many characters
(advice-add 'magit-delta-call-delta-and-convert-ansi-escape-sequences :around
(defun nth/magit-delta-colorize-maybe-a (fn &rest args)
(if (<= (point-max) nth/magit-delta-point-max)
(apply fn args)
(magit-delta-mode -1))))
;; Re-enable mode after `magit-refresh' if there aren't too many characters
(add-hook 'magit-post-refresh-hook
(defun nth/magit-enable-magit-delta-maybe-h (&rest _args)
(when (and (not magit-delta-mode)
(<= (point-max) nth/magit-delta-point-max))
(magit-delta-mode +1)))) |
Sorry for not giving this much attention. It's not because I'm unaffected -- I use magit-delta daily and I encounter performance issues too (moving point in @mpereira can you give instructions for profiling? It sounds like you have done this much more successfully than me -- I have so far failed to profile in a way that points to any relevant function. |
No worries @dandavison, and no pressure 🙂
This happens to me sporadically, even without using delta or magit-delta. I consistently fix it by
What I did was
That should show you the function tree with the number of cycles samples collected in each function. In my case I could see |
Thanks a lot @mpereira. So here's the thing. I see that (defun time-magit-status ()
(interactive)
(let ((start-time (time-to-seconds (current-time))))
(magit-status)
(message "magit-status took %.1f seconds"
(- (time-to-seconds (current-time)) start-time)))) Now, here are the results of various sequences of repeated invocations of
So there are certainly some things to explain. Why do successive calls get slower? Why is the second call in plain magit 3x slower?? And is there not a case for magit to wash these diffs lazily, and start up with the file diffs folded when there are many changed files? But, I need someone to show how to reproduce |
Here's perhaps a slightly better way to time this:
Then in a git repo with a large outstanding diff, either
Would anyone here be able to gather some timings on your machine, either using this script, or just in a normal emacs session using the With a diff of 638 changed files, I'm getting the same as above: consistently slightly faster with |
Thanks @dandavison for looking into this. My diff is
|
Thanks a lot for doing that @naistran. I tried it on someone else's machine last night (and a smaller diff) and also saw slower times for |
Here are some reproducible steps I did to get a large amount of diffs:
results:
|
Thanks @willbush, I ran your commands and can reproduce the slower timing with magit-delta. Here's some quick profiling results: with magit-delta without magit-delta Here's one of the key magit functions involved: (defun magit-diff-wash-diffs (args &optional limit)
(run-hooks 'magit-diff-wash-diffs-hook) ;; <=== magit-delta-call-delta-and-convert-ansi-escape-sequences
(when (member "--show-signature" args)
(magit-diff-wash-signature))
(when (member "--stat" args)
(magit-diff-wash-diffstat))
(when (re-search-forward magit-diff-headline-re limit t)
(goto-char (line-beginning-position))
(magit-wash-sequence (apply-partially 'magit-diff-wash-diff args))
(insert ?\n))) I haven't looked into this properly but I believe that the profiling results indicate that the slowness is occurring when magit's elisp code is executing (especially |
I'm fairly new to emacs, but my setup is Doom Emacs on an Intel Macbook Pro and I notice considerably slower performance when magit-delta is enabled. I would love to use magit-delta, but I had to turn it off for some work because it was taking much longer to even pass over the (closed section) package-lock and package json files. Mostly here to +1 the issue, but I will see if I can get some performance metrics |
Hi @tarsius, do you have a moment to advise on the performance issue we're discussing here, and maybe glance at the profiling output above (#9 (comment))? The tentative conclusion I've reached is
To remind you of the background, magit-delta was made possible by the addition of |
I also find it very plausible that the overlays are to be blamed. |
Thanks a lot @tarsius. Supposing that's the case, do you have any hunches or ideas about ways to improve the performance? For example, one thing that's coming to my mind is temporarily removing the overlays and then reinstating them after magit has done its work. (But I haven't looked into that to see whether it's feasible or whether the buffer would have changed too much in the interim.) Another thought is processing the diffs more asynchronously, e.g. using aio.el. Perhaps I could feed diff hunks one by one to the delta process, instead of processing the entire multi-file diff buffer in one go. But I don't know whether such a change could be achieved as inobtrusively as we have currently with the single hook When developing magit-delta I did try using text-properties for the colors but IIRC I found that these interacted with text properties used by magit itself (perhaps for the hunk divider styling/folding) and thus caused a broken diff buffer. On the other hand using overlays seemed to work perfectly with essentially no work, so I went with overlays. |
I wanted to experiment a bit with this myself and get back to you after that. But I am busy with the numerous requests concerning my own packages that keep streaming in, I don't think I will get to this more complicated case any time soon. The idea I wanted to try was doing the magit-delta thing after magit's regular diff washing instead of before. Whether that is at all feasible of course depends no whether delta adds control characters in places where it messes up regular diff washing. |
I'm having some performance issues with it right now, tracking issue for it: dandavison/magit-delta#9
@tarsius @dandavison any further thoughts on this? This is still happening. |
At the least, it appears this issue won't be solved for a long time. In the mean time would it be possible to have a configuration to disable magit-delta for diffs larger than a certain size? I believe that would make it so that many users such as myself could adopt magit-delta for most diffs without negative perfomance impact. |
@ParetoOptimalDev FWIW, with large diffs, when magit says something like "Fontifying, press C-g to cancel" I often take it up on that offer. Usually what I'm trying to do is stage or commit; I wouldn't try to look at a very large diff in magit-delta, I'd always use the command line for that. |
Personally I leave delta off and toggle it on when needed: (defun my/magit-delta-toggle ()
"Toggle magit-delta-mode and refresh magit."
(interactive)
(progn
(call-interactively 'magit-delta-mode)
(magit-refresh))) |
I'm sorry there hasn't been any progress here! I just wanted to mention that I still use magit-delta all the time and, when it becomes slow, I restart Emacs, and that fixes it. For me it is mainly |
I sometimes have many tabs opened up and don't have a stable session restoration function working yet, so that's not a good option for me at the moment. I wonder if forcing a garbage collection is what's needed after emacs becomes slow from magit-delta though. |
Ah, good suggestion. I'll try it next time it happens and report back. |
I'm finding that |
@dandavison Are you saying that after viewing a lot of changes with magit-delta-mode it slows down the rest of Emacs even after you kill all buffers and garbage-collect? |
No, it's just operations in the
Yes, |
To confirm, the gradual decrease in responsiveness that I am experiencing can be fixed by killing the |
I use magit extensively and wondering if this makes any subjective performance difference vs. regular magit behaviour. If so, that would be great to see in the readme!
I think the slowdown I see regularly in magit might have something to do with the emacs fontifying/long lines issues that have been reported elswehere and nothing to do with magit itself, but unsure.
The text was updated successfully, but these errors were encountered: