{"thread":{"id":"11273","subject":"[StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","startedAt":"2007-12-13T13:38:36Z","lastAt":"2007-12-13T18:06:33Z","messageCount":13,"participants":["David Kågedal","Catalin Marinas","Karl Hasselström","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"63011","messageId":"20071213133653.13925.89254.stgit@krank","threadId":"11273","inReplyTo":null,"subject":"[StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-12-13T13:38:36Z","receivedAt":"2007-12-13T13:38:36Z","isPatch":false,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"This isn't a real patch yet, but it is good enough for my usage.\n\nI have a fair amount of branches, and I noticed that \"stg branch -l\"\ntakes ridiculously long to finish. The problem is that it creates\nstack objects for all branches, and indiviudally extract information\nabout them one after one.  With 20 branches, it took almost 2 seconds\nto run.  Compare that with the 0.01 seconds it takes to run \"git\nbranch\".\n\nI made a patch that uses \"git config --get-regexp\" to get the\ndescription and stgit.stackformatversion options for all branches at\nonce, and ignore the \"protected\" flag that I don't use. With this\nchange, I'm almost down to half a second, which almost makes it\nusable.\n\nThere are still a bunch of redundant invokations of git, but python\nstartup times are hard to get around.\n\nMaybe someone can help me find a quicker replacement for the\nget_protected call?\n\n---\n\n stgit/commands/branch.py |   39 ++++++++++++++++++++++++++++++---------\n 1 files changed, 30 insertions(+), 9 deletions(-)\n\n\ndiff --git a/stgit/commands/branch.py b/stgit/commands/branch.py\nindex 50684bb..f4b0c33 100644\n--- a/stgit/commands/branch.py\n+++ b/stgit/commands/branch.py\n@@ -72,21 +72,21 @@ options = [make_option('-c', '--create',\n def __is_current_branch(branch_name):\n     return crt_series.get_name() == branch_name\n \n-def __print_branch(branch_name, length):\n+def __print_branch(branch_name, description, version, length):\n     initialized = ' '\n     current = ' '\n     protected = ' '\n \n-    branch = stack.Series(branch_name)\n+    #branch = stack.Series(branch_name)\n \n-    if branch.is_initialised():\n+    if version != None:\n         initialized = 's'\n     if __is_current_branch(branch_name):\n         current = '>'\n-    if branch.get_protected():\n-        protected = 'p'\n+    #if branch.get_protected():\n+    #    protected = 'p'\n     out.stdout(current + ' ' + initialized + protected + '\\t'\n-               + branch_name.ljust(length) + '  | ' + branch.get_description())\n+               + branch_name.ljust(length) + '  | ' + (description or ''))\n \n def __delete_branch(doomed_name, force = False):\n     doomed = stack.Series(doomed_name)\n@@ -100,6 +100,23 @@ def __delete_branch(doomed_name, force = False):\n     doomed.delete(force)\n     out.done()\n \n+class FormatException(StgException):\n+    pass\n+\n+def __get_all_branch_config(key):\n+    key = re.escape(key)\n+    lines = git.GRun('config', '--get-regexp',\n+                     r'branch\\..*\\.'+key).returns([0,1]).output_lines()\n+    val_re = re.compile(r'branch\\.(.*)\\.%s (.*)' % key)\n+    result = {}\n+    for line in lines:\n+        m = val_re.match(line)\n+        if not m:\n+            raise FormatException(\"unknown output from git config\")\n+        branch, data = m.groups()\n+        result[branch] = data\n+    return result\n+\n def func(parser, options, args):\n \n     if options.create:\n@@ -198,11 +215,15 @@ def func(parser, options, args):\n         branches = git.get_heads()\n         branches.sort()\n \n+        descriptions = __get_all_branch_config('description')\n+        versions = __get_all_branch_config('stgit.stackformatversion')\n+\n         if branches:\n             out.info('Available branches:')\n-            max_len = max([len(i) for i in branches])\n-            for i in branches:\n-                __print_branch(i, max_len)\n+            max_len = max(len(i) for i in branches)\n+            for branch in branches:\n+                __print_branch(branch, descriptions.get(branch),\n+                               versions.get(branch), max_len)\n         else:\n             out.info('No branches')\n         return\n"},{"id":"63013","messageId":"871w9qsoc8.fsf@lysator.liu.se","threadId":"11273","inReplyTo":"20071213133653.13925.89254.stgit@krank","subject":"git config --get-regexp exit status","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-12-13T13:42:31Z","receivedAt":"2007-12-13T13:42:31Z","isPatch":false,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"David Kågedal <davidk@lysator.liu.se> writes:\n\n> I made a patch that uses \"git config --get-regexp\" to get the\n> description and stgit.stackformatversion options for all branches at\n> once, and ignore the \"protected\" flag that I don't use. With this\n> change, I'm almost down to half a second, which almost makes it\n> usable.\n\nOne thing that annoyed me what that \"git config --get-regexp\" will\nreturn zero, one, or more matches, which are all valid reponses. But\nit treats the zero-match special and return an exit status of 1.\n\nIs that a conscious choice, or just an effect of how \"git config\n--get\" works?\n\nSince zero matches isn't really an error, I would like the exit status\nto be 0. At least for this use case :-)\n\n-- \nDavid Kågedal\n"},{"id":"63018","messageId":"b0943d9e0712130604r6daf05d5n7afbadfe23831839@mail.gmail.com","threadId":"11273","inReplyTo":"20071213133653.13925.89254.stgit@krank","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-12-13T14:04:26Z","receivedAt":"2007-12-13T14:04:26Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n> I have a fair amount of branches, and I noticed that \"stg branch -l\"\n> takes ridiculously long to finish.\n\nI have the same problem.\n\n> Maybe someone can help me find a quicker replacement for the\n> get_protected call?\n\nWe can have the standard --list command which ignores the protected\nflag or even the stgit.formatversion. Just a simple listing of the\nbranches (that's what I need most of the time). To get the\ndescription, the 's' and 'p' flags, we could use --list-full or\nsomething similar and wait a bit longer. This would also improve the\nbash completion of commands taking branch names as arguments.\n\nWe had a similar issue in the past with 'series' as it was checking\nwhether the patch is empty. We ended up adding a '--empty' option for\nthis case.\n\n-- \nCatalin\n"},{"id":"63019","messageId":"87odcur88x.fsf@lysator.liu.se","threadId":"11273","inReplyTo":"b0943d9e0712130604r6daf05d5n7afbadfe23831839@mail.gmail.com","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-12-13T14:15:26Z","receivedAt":"2007-12-13T14:15:26Z","isPatch":false,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"\"Catalin Marinas\" <catalin.marinas@gmail.com> writes:\n\n> On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n>> I have a fair amount of branches, and I noticed that \"stg branch -l\"\n>> takes ridiculously long to finish.\n>\n> I have the same problem.\n>\n>> Maybe someone can help me find a quicker replacement for the\n>> get_protected call?\n>\n> We can have the standard --list command which ignores the protected\n> flag or even the stgit.formatversion. Just a simple listing of the\n> branches (that's what I need most of the time). To get the\n> description, the 's' and 'p' flags, we could use --list-full or\n> something similar and wait a bit longer. This would also improve the\n> bash completion of commands taking branch names as arguments.\n\nIf you remove everything but the branch name, you will have identical\noutput to \"git branch\", but much slower.  Then there is no reason to\nuse it. The reason I used \"stg branch -l\" was that I found it\ninteresting to know which branches I had initialized with stgit, and I\ncould see uses for the description as well.\n\n-- \nDavid Kågedal\n"},{"id":"63021","messageId":"87lk7yr7ib.fsf@lysator.liu.se","threadId":"11273","inReplyTo":"b0943d9e0712130604r6daf05d5n7afbadfe23831839@mail.gmail.com","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-12-13T14:31:24Z","receivedAt":"2007-12-13T14:31:24Z","isPatch":false,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"\"Catalin Marinas\" <catalin.marinas@gmail.com> writes:\n\n> On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n>> I have a fair amount of branches, and I noticed that \"stg branch -l\"\n>> takes ridiculously long to finish.\n>\n> I have the same problem.\n>\n>> Maybe someone can help me find a quicker replacement for the\n>> get_protected call?\n\nHey, why not put the \"protected\" flag in the config? Then we can get\nit the same way as the other stuff.\n\nProtecting a branch is a configuration action, so it makes sense to\nput it in the config.\n\n-- \nDavid Kågedal\n"},{"id":"63024","messageId":"b0943d9e0712130738s18ad303fu17a8412097377900@mail.gmail.com","threadId":"11273","inReplyTo":"87lk7yr7ib.fsf@lysator.liu.se","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-12-13T15:38:32Z","receivedAt":"2007-12-13T15:38:32Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n> \"Catalin Marinas\" <catalin.marinas@gmail.com> writes:\n>\n> > On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n> >> I have a fair amount of branches, and I noticed that \"stg branch -l\"\n> >> takes ridiculously long to finish.\n> >\n> > I have the same problem.\n> >\n> >> Maybe someone can help me find a quicker replacement for the\n> >> get_protected call?\n>\n> Hey, why not put the \"protected\" flag in the config? Then we can get\n> it the same way as the other stuff.\n>\n> Protecting a branch is a configuration action, so it makes sense to\n> put it in the config.\n\nYes, I'm OK with this. The only problem I see is that we have to\nchange the stgit.formatversion and provide an upgrade in the Series\nobject. However, 'stg branch -l' no longer initialises the Series\nobjects and the upgrade won't happen.\n\nThe branch command would have to check format version and force the\nupgrade if it isn't the required one.\n\nBTW, have you run stg-prof to check where it spends most of the time?\nIs it caused by Python object creation or GIT calls invoked during the\nSeries objects initialisation. If the latter, we can turn some\nvariables into properties and access them lazily.\n\n-- \nCatalin\n"},{"id":"63026","messageId":"20071213160432.GA30693@diana.vm.bytemark.co.uk","threadId":"11273","inReplyTo":"b0943d9e0712130604r6daf05d5n7afbadfe23831839@mail.gmail.com","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-12-13T16:04:32Z","receivedAt":"2007-12-13T16:04:32Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-12-13 14:04:26 +0000, Catalin Marinas wrote:\n\n> On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n>\n> > Maybe someone can help me find a quicker replacement for the\n> > get_protected call?\n>\n> We can have the standard --list command which ignores the protected\n> flag\n\nExactly what is the p flag useful for anyway?\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"63028","messageId":"20071213160853.GB30693@diana.vm.bytemark.co.uk","threadId":"11273","inReplyTo":"87lk7yr7ib.fsf@lysator.liu.se","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-12-13T16:08:53Z","receivedAt":"2007-12-13T16:08:53Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-12-13 15:31:24 +0100, David Kågedal wrote:\n\n> Hey, why not put the \"protected\" flag in the config? Then we can get\n> it the same way as the other stuff.\n>\n> Protecting a branch is a configuration action, so it makes sense to\n> put it in the config.\n\nI agree; if we are to have such a flag, the config is the right place\nfor it.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"63027","messageId":"87bq8ur2zs.fsf@lysator.liu.se","threadId":"11273","inReplyTo":"b0943d9e0712130738s18ad303fu17a8412097377900@mail.gmail.com","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-12-13T16:08:55Z","receivedAt":"2007-12-13T16:08:55Z","isPatch":false,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"\"Catalin Marinas\" <catalin.marinas@gmail.com> writes:\n\n> BTW, have you run stg-prof to check where it spends most of the time?\n> Is it caused by Python object creation or GIT calls invoked during the\n> Series objects initialisation. If the latter, we can turn some\n> variables into properties and access them lazily.\n\nI ran with STG_SUBPROCESS_LOG=debug and noticed that there were four\ninvokations of git for each branch it listed, in addition to the ten\ninvokation before it even starts to list the branches.\n\nSo I focused on replacing those 4xN invokations with 2 invokations,\nand it helped a lot.  But I'm sure there are still lots of things to\nimprove.\n\nActually, I didn't know about stg-prof.\n\n-- \nDavid Kågedal\n"},{"id":"63029","messageId":"b0943d9e0712130810p35b33e6aj7756b1af1922992b@mail.gmail.com","threadId":"11273","inReplyTo":"20071213160432.GA30693@diana.vm.bytemark.co.uk","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-12-13T16:10:10Z","receivedAt":"2007-12-13T16:10:10Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 13/12/2007, Karl Hasselström <kha@treskal.com> wrote:\n> On 2007-12-13 14:04:26 +0000, Catalin Marinas wrote:\n>\n> > On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n> >\n> > > Maybe someone can help me find a quicker replacement for the\n> > > get_protected call?\n> >\n> > We can have the standard --list command which ignores the protected\n> > flag\n>\n> Exactly what is the p flag useful for anyway?\n\nIt was added so that you don't rebase the stack by mistake. Yann\nsuggested to have a specific policy for this and make the protected\nflag freeze the stack completely.\n\n-- \nCatalin\n"},{"id":"63036","messageId":"878x3yr1ka.fsf@lysator.liu.se","threadId":"11273","inReplyTo":"b0943d9e0712130810p35b33e6aj7756b1af1922992b@mail.gmail.com","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-12-13T16:39:49Z","receivedAt":"2007-12-13T16:39:49Z","isPatch":false,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"\"Catalin Marinas\" <catalin.marinas@gmail.com> writes:\n\n> On 13/12/2007, Karl Hasselström <kha@treskal.com> wrote:\n>> On 2007-12-13 14:04:26 +0000, Catalin Marinas wrote:\n>>\n>> > On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:\n>> >\n>> > > Maybe someone can help me find a quicker replacement for the\n>> > > get_protected call?\n>> >\n>> > We can have the standard --list command which ignores the protected\n>> > flag\n>>\n>> Exactly what is the p flag useful for anyway?\n>\n> It was added so that you don't rebase the stack by mistake. Yann\n> suggested to have a specific policy for this and make the protected\n> flag freeze the stack completely.\n\nI'd be much more interested in a flag that prevents me from running\n\"git rebase\" on a stg-controlled branch by mistake...\n\n-- \nDavid Kågedal\n"},{"id":"63038","messageId":"20071213170414.GA32171@diana.vm.bytemark.co.uk","threadId":"11273","inReplyTo":"878x3yr1ka.fsf@lysator.liu.se","subject":"Re: [StGit RFC] Make \"stg branch -l\" faster by getting all git config information in one call","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-12-13T17:04:14Z","receivedAt":"2007-12-13T17:04:14Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-12-13 17:39:49 +0100, David Kågedal wrote:\n\n> I'd be much more interested in a flag that prevents me from running\n> \"git rebase\" on a stg-controlled branch by mistake...\n\nIn the long run, we probably want to figure out a good set of hooks in\ngit that would let us know when interesting stuff happens. And then\nadd them. The challenge will be to keep the number of hooks down.\n\nIn the short run, we need a general \"undo\" command. (I've got one on\nthe drawing board. Just need to set aside some time ...)\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"63041","messageId":"7vfxy631w6.fsf@gitster.siamese.dyndns.org","threadId":"11273","inReplyTo":"871w9qsoc8.fsf@lysator.liu.se","subject":"Re: git config --get-regexp exit status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-13T18:06:33Z","receivedAt":"2007-12-13T18:06:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kågedal <davidk@lysator.liu.se> writes:\n\n> David Kågedal <davidk@lysator.liu.se> writes:\n> ...\n> One thing that annoyed me what that \"git config --get-regexp\" will\n> return zero, one, or more matches, which are all valid reponses. But\n> it treats the zero-match special and return an exit status of 1.\n>\n> Is that a conscious choice, or just an effect of how \"git config\n> --get\" works?\n>\n> Since zero matches isn't really an error, I would like the exit status\n> to be 0. At least for this use case :-)\n\nI would imagine \"matching variant\" may want to signal \"I did not match\nanything\" as a special event, just like grep does.  As long as the\ncaller can distinguish \"no match\" from real failure cases (say, not in a\ngit repository and hence no configuration file), that is one valid use\nof status code.\n\nI haven't been annoyed by that myself, but I see what you mean.  \"give\nme all the matching ones\" callers would want to consider zero match and\nseven matches both the same way as non errors.  And similarity to \"grep\"\ndoes not extend very far because there is no \"--get-regexp\" variant that\nis quiet (i.e. \"no output, just checking if there is a match\").\n\nSo I do not mind if --get-regexp considered zero matches as non-error\nmyself.  If an existing script did something like:\n\n\tif list=$(git config --get-regexp '$pattern')\n        then\n        \t# use list, without verifying if it is empty\n                ...\n\tfi\n\nsuch a change would break it, though.  But the script should have been\ndoing:\n\n\tLF='\n\t'\n\tif list=$(git config --get-regexp '$pattern')\n        then\n\t\tIFS=\"$LF\"\n\t\tfor $list\n                do\n                \t...\n\t\tdone\n\tfi\n\nanyway, so...\n"}]}