# [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call 13 messages from 2007-12-13 to 2007-12-13. Participants: David Kågedal, Catalin Marinas, Karl Hasselström, Junio C Hamano. Thread: https://gitlist.dev/t/11273 ## David Kågedal, 2007-12-13 13:38 Subject: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <20071213133653.13925.89254.stgit@krank> URL: https://gitlist.dev/e/20071213133653.13925.89254.stgit%40krank ``` This isn't a real patch yet, but it is good enough for my usage. I have a fair amount of branches, and I noticed that "stg branch -l" takes ridiculously long to finish. The problem is that it creates stack objects for all branches, and indiviudally extract information about them one after one. With 20 branches, it took almost 2 seconds to run. Compare that with the 0.01 seconds it takes to run "git branch". I made a patch that uses "git config --get-regexp" to get the description and stgit.stackformatversion options for all branches at once, and ignore the "protected" flag that I don't use. With this change, I'm almost down to half a second, which almost makes it usable. There are still a bunch of redundant invokations of git, but python startup times are hard to get around. Maybe someone can help me find a quicker replacement for the get_protected call? --- stgit/commands/branch.py | 39 ++++++++++++++++++++++++++++++--------- 1 files changed, 30 insertions(+), 9 deletions(-) diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py index 50684bb..f4b0c33 100644 --- a/stgit/commands/branch.py +++ b/stgit/commands/branch.py @@ -72,21 +72,21 @@ options = [make_option('-c', '--create', def __is_current_branch(branch_name): return crt_series.get_name() == branch_name -def __print_branch(branch_name, length): +def __print_branch(branch_name, description, version, length): initialized = ' ' current = ' ' protected = ' ' - branch = stack.Series(branch_name) + #branch = stack.Series(branch_name) - if branch.is_initialised(): + if version != None: initialized = 's' if __is_current_branch(branch_name): current = '>' - if branch.get_protected(): - protected = 'p' + #if branch.get_protected(): + # protected = 'p' out.stdout(current + ' ' + initialized + protected + '\t' - + branch_name.ljust(length) + ' | ' + branch.get_description()) + + branch_name.ljust(length) + ' | ' + (description or '')) def __delete_branch(doomed_name, force = False): doomed = stack.Series(doomed_name) @@ -100,6 +100,23 @@ def __delete_branch(doomed_name, force = False): doomed.delete(force) out.done() +class FormatException(StgException): + pass + +def __get_all_branch_config(key): + key = re.escape(key) + lines = git.GRun('config', '--get-regexp', + r'branch\..*\.'+key).returns([0,1]).output_lines() + val_re = re.compile(r'branch\.(.*)\.%s (.*)' % key) + result = {} + for line in lines: + m = val_re.match(line) + if not m: + raise FormatException("unknown output from git config") + branch, data = m.groups() + result[branch] = data + return result + def func(parser, options, args): if options.create: @@ -198,11 +215,15 @@ def func(parser, options, args): branches = git.get_heads() branches.sort() + descriptions = __get_all_branch_config('description') + versions = __get_all_branch_config('stgit.stackformatversion') + if branches: out.info('Available branches:') - max_len = max([len(i) for i in branches]) - for i in branches: - __print_branch(i, max_len) + max_len = max(len(i) for i in branches) + for branch in branches: + __print_branch(branch, descriptions.get(branch), + versions.get(branch), max_len) else: out.info('No branches') return ``` ## David Kågedal, 2007-12-13 13:42 Subject: git config --get-regexp exit status Message-ID: <871w9qsoc8.fsf@lysator.liu.se> URL: https://gitlist.dev/e/871w9qsoc8.fsf%40lysator.liu.se In-Reply-To: <20071213133653.13925.89254.stgit@krank> ``` David Kågedal writes: > I made a patch that uses "git config --get-regexp" to get the > description and stgit.stackformatversion options for all branches at > once, and ignore the "protected" flag that I don't use. With this > change, I'm almost down to half a second, which almost makes it > usable. One thing that annoyed me what that "git config --get-regexp" will return zero, one, or more matches, which are all valid reponses. But it treats the zero-match special and return an exit status of 1. Is that a conscious choice, or just an effect of how "git config --get" works? Since zero matches isn't really an error, I would like the exit status to be 0. At least for this use case :-) -- David Kågedal ``` ## Catalin Marinas, 2007-12-13 14:04 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: URL: https://gitlist.dev/e/b0943d9e0712130604r6daf05d5n7afbadfe23831839%40mail.gmail.com In-Reply-To: <20071213133653.13925.89254.stgit@krank> ``` On 13/12/2007, David Kågedal wrote: > I have a fair amount of branches, and I noticed that "stg branch -l" > takes ridiculously long to finish. I have the same problem. > Maybe someone can help me find a quicker replacement for the > get_protected call? We can have the standard --list command which ignores the protected flag or even the stgit.formatversion. Just a simple listing of the branches (that's what I need most of the time). To get the description, the 's' and 'p' flags, we could use --list-full or something similar and wait a bit longer. This would also improve the bash completion of commands taking branch names as arguments. We had a similar issue in the past with 'series' as it was checking whether the patch is empty. We ended up adding a '--empty' option for this case. -- Catalin ``` ## David Kågedal, 2007-12-13 14:15 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <87odcur88x.fsf@lysator.liu.se> URL: https://gitlist.dev/e/87odcur88x.fsf%40lysator.liu.se In-Reply-To: ``` "Catalin Marinas" writes: > On 13/12/2007, David Kågedal wrote: >> I have a fair amount of branches, and I noticed that "stg branch -l" >> takes ridiculously long to finish. > > I have the same problem. > >> Maybe someone can help me find a quicker replacement for the >> get_protected call? > > We can have the standard --list command which ignores the protected > flag or even the stgit.formatversion. Just a simple listing of the > branches (that's what I need most of the time). To get the > description, the 's' and 'p' flags, we could use --list-full or > something similar and wait a bit longer. This would also improve the > bash completion of commands taking branch names as arguments. If you remove everything but the branch name, you will have identical output to "git branch", but much slower. Then there is no reason to use it. The reason I used "stg branch -l" was that I found it interesting to know which branches I had initialized with stgit, and I could see uses for the description as well. -- David Kågedal ``` ## David Kågedal, 2007-12-13 14:31 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <87lk7yr7ib.fsf@lysator.liu.se> URL: https://gitlist.dev/e/87lk7yr7ib.fsf%40lysator.liu.se In-Reply-To: ``` "Catalin Marinas" writes: > On 13/12/2007, David Kågedal wrote: >> I have a fair amount of branches, and I noticed that "stg branch -l" >> takes ridiculously long to finish. > > I have the same problem. > >> Maybe someone can help me find a quicker replacement for the >> get_protected call? Hey, why not put the "protected" flag in the config? Then we can get it the same way as the other stuff. Protecting a branch is a configuration action, so it makes sense to put it in the config. -- David Kågedal ``` ## Catalin Marinas, 2007-12-13 15:38 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: URL: https://gitlist.dev/e/b0943d9e0712130738s18ad303fu17a8412097377900%40mail.gmail.com In-Reply-To: <87lk7yr7ib.fsf@lysator.liu.se> ``` On 13/12/2007, David Kågedal wrote: > "Catalin Marinas" writes: > > > On 13/12/2007, David Kågedal wrote: > >> I have a fair amount of branches, and I noticed that "stg branch -l" > >> takes ridiculously long to finish. > > > > I have the same problem. > > > >> Maybe someone can help me find a quicker replacement for the > >> get_protected call? > > Hey, why not put the "protected" flag in the config? Then we can get > it the same way as the other stuff. > > Protecting a branch is a configuration action, so it makes sense to > put it in the config. Yes, I'm OK with this. The only problem I see is that we have to change the stgit.formatversion and provide an upgrade in the Series object. However, 'stg branch -l' no longer initialises the Series objects and the upgrade won't happen. The branch command would have to check format version and force the upgrade if it isn't the required one. BTW, have you run stg-prof to check where it spends most of the time? Is it caused by Python object creation or GIT calls invoked during the Series objects initialisation. If the latter, we can turn some variables into properties and access them lazily. -- Catalin ``` ## Karl Hasselström, 2007-12-13 16:04 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <20071213160432.GA30693@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20071213160432.GA30693%40diana.vm.bytemark.co.uk In-Reply-To: ``` On 2007-12-13 14:04:26 +0000, Catalin Marinas wrote: > On 13/12/2007, David Kågedal wrote: > > > Maybe someone can help me find a quicker replacement for the > > get_protected call? > > We can have the standard --list command which ignores the protected > flag Exactly what is the p flag useful for anyway? -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Karl Hasselström, 2007-12-13 16:08 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <20071213160853.GB30693@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20071213160853.GB30693%40diana.vm.bytemark.co.uk In-Reply-To: <87lk7yr7ib.fsf@lysator.liu.se> ``` On 2007-12-13 15:31:24 +0100, David Kågedal wrote: > Hey, why not put the "protected" flag in the config? Then we can get > it the same way as the other stuff. > > Protecting a branch is a configuration action, so it makes sense to > put it in the config. I agree; if we are to have such a flag, the config is the right place for it. -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## David Kågedal, 2007-12-13 16:08 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <87bq8ur2zs.fsf@lysator.liu.se> URL: https://gitlist.dev/e/87bq8ur2zs.fsf%40lysator.liu.se In-Reply-To: ``` "Catalin Marinas" writes: > BTW, have you run stg-prof to check where it spends most of the time? > Is it caused by Python object creation or GIT calls invoked during the > Series objects initialisation. If the latter, we can turn some > variables into properties and access them lazily. I ran with STG_SUBPROCESS_LOG=debug and noticed that there were four invokations of git for each branch it listed, in addition to the ten invokation before it even starts to list the branches. So I focused on replacing those 4xN invokations with 2 invokations, and it helped a lot. But I'm sure there are still lots of things to improve. Actually, I didn't know about stg-prof. -- David Kågedal ``` ## Catalin Marinas, 2007-12-13 16:10 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: URL: https://gitlist.dev/e/b0943d9e0712130810p35b33e6aj7756b1af1922992b%40mail.gmail.com In-Reply-To: <20071213160432.GA30693@diana.vm.bytemark.co.uk> ``` On 13/12/2007, Karl Hasselström wrote: > On 2007-12-13 14:04:26 +0000, Catalin Marinas wrote: > > > On 13/12/2007, David Kågedal wrote: > > > > > Maybe someone can help me find a quicker replacement for the > > > get_protected call? > > > > We can have the standard --list command which ignores the protected > > flag > > Exactly what is the p flag useful for anyway? It was added so that you don't rebase the stack by mistake. Yann suggested to have a specific policy for this and make the protected flag freeze the stack completely. -- Catalin ``` ## David Kågedal, 2007-12-13 16:39 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <878x3yr1ka.fsf@lysator.liu.se> URL: https://gitlist.dev/e/878x3yr1ka.fsf%40lysator.liu.se In-Reply-To: ``` "Catalin Marinas" writes: > On 13/12/2007, Karl Hasselström wrote: >> On 2007-12-13 14:04:26 +0000, Catalin Marinas wrote: >> >> > On 13/12/2007, David Kågedal wrote: >> > >> > > Maybe someone can help me find a quicker replacement for the >> > > get_protected call? >> > >> > We can have the standard --list command which ignores the protected >> > flag >> >> Exactly what is the p flag useful for anyway? > > It was added so that you don't rebase the stack by mistake. Yann > suggested to have a specific policy for this and make the protected > flag freeze the stack completely. I'd be much more interested in a flag that prevents me from running "git rebase" on a stg-controlled branch by mistake... -- David Kågedal ``` ## Karl Hasselström, 2007-12-13 17:04 Subject: Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call Message-ID: <20071213170414.GA32171@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20071213170414.GA32171%40diana.vm.bytemark.co.uk In-Reply-To: <878x3yr1ka.fsf@lysator.liu.se> ``` On 2007-12-13 17:39:49 +0100, David Kågedal wrote: > I'd be much more interested in a flag that prevents me from running > "git rebase" on a stg-controlled branch by mistake... In the long run, we probably want to figure out a good set of hooks in git that would let us know when interesting stuff happens. And then add them. The challenge will be to keep the number of hooks down. In the short run, we need a general "undo" command. (I've got one on the drawing board. Just need to set aside some time ...) -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ``` ## Junio C Hamano, 2007-12-13 18:06 Subject: Re: git config --get-regexp exit status Message-ID: <7vfxy631w6.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfxy631w6.fsf%40gitster.siamese.dyndns.org In-Reply-To: <871w9qsoc8.fsf@lysator.liu.se> ``` David Kågedal writes: > David Kågedal writes: > ... > One thing that annoyed me what that "git config --get-regexp" will > return zero, one, or more matches, which are all valid reponses. But > it treats the zero-match special and return an exit status of 1. > > Is that a conscious choice, or just an effect of how "git config > --get" works? > > Since zero matches isn't really an error, I would like the exit status > to be 0. At least for this use case :-) I would imagine "matching variant" may want to signal "I did not match anything" as a special event, just like grep does. As long as the caller can distinguish "no match" from real failure cases (say, not in a git repository and hence no configuration file), that is one valid use of status code. I haven't been annoyed by that myself, but I see what you mean. "give me all the matching ones" callers would want to consider zero match and seven matches both the same way as non errors. And similarity to "grep" does not extend very far because there is no "--get-regexp" variant that is quiet (i.e. "no output, just checking if there is a match"). So I do not mind if --get-regexp considered zero matches as non-error myself. If an existing script did something like: if list=$(git config --get-regexp '$pattern') then # use list, without verifying if it is empty ... fi such a change would break it, though. But the script should have been doing: LF=' ' if list=$(git config --get-regexp '$pattern') then IFS="$LF" for $list do ... done fi anyway, so... ```