magithub-issue-repo is very slow on my machine #349
Comments
|
Nice catch. I'm wondering if such a cache (probably just a hash table 'root-dir -> value'; local memory only) should go into ghub though so magit can directly benefit as well. @tarsius, what do you think? |
|
Magit already has such a cache, but because Why do you call |
|
It could be that it's the cache that's calling this so many times. When we cache an API call, Code references: Actually, that's not it. It's a lot simpler than that, though some performance improvements could be made to the cache here (since I'm pretty confident the context includes the host). We actually call I don't recall what |
|
The dashboard does bucket issues by repo using this function. Is that the workflow we're seeing it in? Lines 196 to 198 in 8cf40f5 |
|
Here's a complete trace from the profiler: Actually, there are many code paths that end up calling |
|
You can take I feel like we should still cache |
|
I'd be interested to see what's taking |
|
I don't have the trace, but I remember the trace but I remember where the slowness came from. The datetime parsing was actually quite slow: |
Please do that in |
|
Will do. I made some commits last night that implements -- for now -- a simple time-based cache of git-config information. Once I get home, I'll clean those up and push -- we'll see how that affects performance here. I'd like to avoid adding a hard dependency on magit, but I've no issue with using it if available. I'll see what macro-magic I can do tonight. |
|
Hi, I came to your conclusions independently:
For To solve this I came up with the same idea, using a hash table. By changing the code in (defun ghub--host (&optional forge)
(when (not (hash-table-p ghub--host-result))
(setq ghub--host-result (make-hash-table :test 'equal)))
(if-let* ((result (gethash default-directory ghub--host-result nil)))
result
(puthash default-directory
(cl-ecase forge
((nil github)
(or (ignore-errors (car (process-lines "git" "config" "github.host")))
ghub-default-host))
(gitlab
(or (ignore-errors (car (process-lines "git" "config" "gitlab.host")))
(bound-and-true-p glab-default-host)))
(gitea
(or (ignore-errors (car (process-lines "git" "config" "gitea.host")))
(bound-and-true-p gtea-default-host)))
(gogs
(or (ignore-errors (car (process-lines "git" "config" "gogs.host")))
(bound-and-true-p gogs-default-host)))
(bitbucket
(or (ignore-errors (car (process-lines "git" "config" "bitbucket.host")))
(bound-and-true-p buck-default-host))))
ghub--host-result)))
(defvar ghub--host-result (make-hash-table :test string-equal)
"Cache result of ghub--host.")Now, the issue is how to correctly refresh the cache. For I went the very crude way to replace (defun magithub--parse-time-string (iso8601)
"Parse ISO8601 into a time value.
ISO8601 is expected to not have a TZ component."
(let ((year (string-to-number (substring iso8601 0 4)))
(month (string-to-number (substring iso8601 5 7)))
(day (string-to-number (substring iso8601 8 10)))
(hour (string-to-number (substring iso8601 11 13)))
(minute (string-to-number (substring iso8601 14 16)))
(second (string-to-number (substring iso8601 17 19)))
(day-of-week nil)
(dst nil)
(tz 0))
(encode-time second minute hour day month year t)))No error detection is made here, we should check Now again, no performance impact anymore. With those two performance bottlenecks out of the way, the remaining profile trace is: Looks like optimizing |
|
Excellent work @ibizaman! |
|
I've been thinking about where to put the code to cache the result of Caching should be done somewhere in magithub, as magithub is the only one that knows when to refresh the cache. Like proposed by @tarsius, that should be done somewhere in That being said, I glanced over that function and have no idea how to use it to refresh the |
|
Jumping ahead and replacing that whole I skipped a few levels for readability. I don't think there's an easy way around this loading time apart from lazy loading each issue. EDIT: I can come up with a PR where the user can select with a custom variable to choose if the issues and PR info must be lazy loaded or not. EDIT2: This slowness is not only happening on a EDIT3: To be fair, stashing with |
|
When doing a lot of git stuff (as opposed to any github stuff), I find it helpful to quickly turn off the heavy sections completely for those few minutes. I don't have a computer with a working emacs at the moment (part of the reason it's so hard to find time for this project... |
|
The other alternative that we could take would be changing what sections are turned on by default. Filling and fontifying the first parts of the issue body probably isn't cheap (from your profile above, about 23%). It might not give enough value for its cost. |
😱 |
|
@vermiculus that's a good point. In some way, the root cause is magit needing to refresh the whole buffer on a stage operation. |
|
Just discovered this note about performance of magit on macos. https://magit.vc/manual/magit/MacOS-Performance.html I'll see what speed boost I can get with that |
I know :-( my emacs computer is largely my wife's now until hers is fixed. Don't know when that will be. |
magithub-issue-repois a bottleneck on my machine when bringing up the github status.This is likely because of the
ghub--hostcall:It's quite slow to have to shell out to git for every single issue for a large enough list of issues. It would be much better to call this once for the list of issues, or even better to remember this value per repo. It's not very likely that this host value is going to change very often.
The text was updated successfully, but these errors were encountered: