Redundant calls leads to slow performance in forge-dispatch

[CeleritasCelery]

Thank you for creating this package. I have been using forge on a large repo, and in the magit dispatch popup it will typically take about 15 seconds after I hit N (forge-dispatch) before the transient menu will show up. I wondered why it took so long, so I enabled magit-process-extreme-logging to see the commands. Here is the result:

Details

$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel
$ git … rev-parse --git-dir
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false config -z --get-all --include forge.remote
$ git … --no-pager --literal-pathspecs -c core.preloadindex\=true -c log.showSignature\=false -c color.ui\=false -c color.diff\=false remote get-url origin
$ git … rev-parse --show-toplevel
$ git … rev-parse --show-cdup [2 times]
$ git … rev-parse --show-toplevel

There are 176 calls to git, but only 6 unique ones. Each set of operations is getting called 22 times. And that is just to open the dispatch menu. It looks like these are part of calls to forge-get-repository. That function does use magit--with-refresh-cache, but the cache is never initialized so it never gets used. I was going to try and initialize magit--refresh-cache myself but I couldn't figure out how to to do that in transient-define-prefix.

Magit v4.1.0-11-g7370a47a, Transient 0.7.4, Forge 0.4.4, Git 2.46.0, Emacs 29.4, darwin

[tarsius]

Yes, I am aware. But after working on Transient and Forge for a long time I wanted to work on other things for a while. That will allow me to find good solutions once I return my focus here.

I have some experimental work that allows storing information in an environment that can be shared by all code that sets up a prefix. Kinda like let, but that cannot be used here, we do have to pass along an environment.

[tarsius]

Yes, I am aware.

By that I meant that I am aware that some work is being done multiple times, not that I observe the exact same behavior as you do, or even just some noticeable slogginess, much less such an extreme slowdown.

For me invoking forge-dispatch from the Magit status buffer of a repository that is already being tracked by Forge does not invoke git even once. forge--get-repository:tracked? does get called seven times, sure, but every time we get the value from forge-buffer-repository, which returns the buffer-local value (without having to call git).

Maybe you call forge-dispatch from a buffer that doesn't have forge-buffer-repository set. But even when I do that, all these calls to git are not an issue in my case. But I am on Linux and it is known that calling git (or any executable) and accessing the file system are much slower macOS, especially when virus scanners and such are doing their thing in the background.

That of course doesn't mean that I shouldn't avoid unnecessary repeated work, but something else is at play here, which is bound to slow down all your interactions with Magit.

The additional cache should arrive shortly (but I am posting this message a bit earlier, to avoid accidentally losing it and then having to write it again).

[CeleritasCelery]

I am glad you are thinking about good potential solutions. In the meantime, I am using this code to workaround the issue:

(with-eval-after-load 'magit
  (defun transient-setup-cache (fn &rest args)
    (let ((magit--refresh-cache (list (cons 0 0))))
      (apply fn args)))
  (advice-add 'transient-setup :around #'transient-setup-cache))

This makes forge-dispatch nearly instantaneous.

[tarsius]

My approach uses the same basic idea, but limits the effect to Forge's menus and is not limited to just the initial setup but also kicks in for later refreshes.

For now it only uses Magit's refresh cache, but it could be extended along these lines:

(defun forge--menu-environment (fn)
  (let* ((magit--refresh-cache (list (cons 0 0)))
         (topic   (forge-current-topic))
         (issue   (forge-current-issue))
         (pullreq (forge-current-pullreq)))
    (cl-letf (((symbol-function #'forge-current-topic)   (lambda () topic))
              ((symbol-function #'forge-current-issue)   (lambda () issue))
              ((symbol-function #'forge-current-pullreq) (lambda () pullreq)))
      (funcall fn))))

You will have to update Transient and Forge.