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

On large repos, status mode > refresh is very slow #341

Closed
tsbriggs02 opened this issue Oct 23, 2014 · 5 comments
Closed

On large repos, status mode > refresh is very slow #341

tsbriggs02 opened this issue Oct 23, 2014 · 5 comments

Comments

@tsbriggs02
Copy link

Hello -- I'm new to tig, but very impressed so far :-)

I'm having a problem though: I'm using tig 2.0.2 on a large repo (about 133,000 commits). Similar to #310 and #324 , I've found a place where it becomes very slow: in the status mode, hitting 'R' to refresh takes about 20 seconds. (Doing "git status" at the command line takes less than 1 second.)

@dcfranca
Copy link

Hi, related to this performance improvement.
We found a performance penalty in src/refdb.c:174, looping through refs.
There's a comparison: strcmp(name, refs[pos]->name);
but struct ref->name is a 1 char lenght c-string, while name is the full name of the ref, does this comparison make sense? It seems that there's room for improvement there.

Let me know if it impacts the code to remove this comparison (and skip the loop in the case type != REFERENCE_REPLACE)

@tsbriggs02
Copy link
Author

It seems to me like the reason that refresh is slow in 'status' mode is that it tries to re-read the entire list of refs (it calls load_refs(TRUE)). But it seems like the only thing tig wants to do is update the name of the current branch, to display at the top of the screen. So, I propose replacing that load_refs() with code to get and set the current branch name.

(I'm looking through the tig codebase now, to see if there's already a function that does that. I haven't found it yet -- I could add it to git.h.)

Edit: maybe in status.c:
case REQ_REFRESH:
/* Load the current branch information and then the view. */
load_repo_info();
status_update_onbranch();

jonas added a commit that referenced this issue Nov 2, 2014
Add a test for the different on-branch "modes" based on whether
git-rebase, git-am or git-merge is in progress or whether a detached
head has been checked out.

This also changes status view refreshing to only reload the current
branch information and not all refs as suggested in #341. Note that it
actually reloads all repo information to get both the symbolic ref
pointed to by HEAD as well as the HEAD id.
@jonas
Copy link
Owner

jonas commented Nov 2, 2014

@tsbriggs02 Is there a public repo available in which I can reproduce this? I find it difficult to understand that the status refresh is bound by the number of commits. It would make more sense if it is bound by the number of files since refreshing executes git update-index -q --unmerged --refresh which in worst case will iterate over all tracked files in the repo and rehash them.

For now, per your suggestion I've changed status refresh to no longer reload the refs. I am curious if this fixes the issue for you.

@DanielFranca As you correctly point out the refdb is quite stupid. This actually goes for most things in tig, since it mainly uses arrays or single-linked lists. The changes to the revision graph pulled in a hash implementation, so it would be worth looking into converting the various caches to use a hash map instead of an array to speed things up. This would of course come at the expense of more memory usage.

@tsbriggs02
Copy link
Author

@jonas -- It's funny... I assumed that this was a problem with large repos in general. But I tried just now with the Linux kernel repo, and it didn't have the problem. (Status > 'R' became responsive again in about 1 second.) The kernel repo has more files, and more commits, than my company's repo which has the problem.

At any rate, your recent commits fixed it -- the refresh time on my company's repo has gone from 15-20 seconds, to 1. Thank you! :-)

@jonas
Copy link
Owner

jonas commented Nov 4, 2014

Thanks for confirming that this is now fixed.

dcfranca pushed a commit to jerojasro-booking/tig that referenced this issue Nov 27, 2014
Add a test for the different on-branch "modes" based on whether
git-rebase, git-am or git-merge is in progress or whether a detached
head has been checked out.

This also changes status view refreshing to only reload the current
branch information and not all refs as suggested in jonas#341. Note that it
actually reloads all repo information to get both the symbolic ref
pointed to by HEAD as well as the HEAD id.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants