{"thread":{"id":"16218","subject":"[PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","startedAt":"2008-11-08T03:22:48Z","lastAt":"2008-11-12T11:54:50Z","messageCount":11,"participants":["John Chapman","David Symonds","Arafangion","Jakub Narebski","Junio C Hamano","Han-Wen Nienhuys","Steve Frécinaux","Simon Hausmann"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"95191","messageId":"1226114569-8506-1-git-send-email-thestar@fussycoder.id.au","threadId":"16218","inReplyTo":null,"subject":"[PATCH 1/2] Added support for purged files and also optimised memory usage.","fromName":"John Chapman","fromEmail":"thestar@fussycoder.id.au","sentAt":"2008-11-08T03:22:48Z","receivedAt":"2008-11-08T03:22:48Z","isPatch":true,"sender":{"key":"thestar@fussycoder.id.au","avatar":null},"body":"Purged files are handled as if they are merely deleted, which is not\nentirely optimal, but I don't know of any other way to handle them.\nFile data is deleted from memory as early as they can, and they are more\nefficiently handled, at (significant) cost to CPU usage.\n\nStill need to handle p4 branches with spaces in their names.\nStill need to make git-p4 clone more reliable.\n - Perhaps with a --continue option. (Sometimes the p4 server kills\n the connection)\n\nSigned-off-by: John Chapman \n---\n contrib/fast-import/git-p4 | 14 +++++++-------\n 1 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 2216cac..38d1a17 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -946,7 +946,7 @@ class P4Sync(Command):\n \n if includeFile:\n filesForCommit.append(f)\n- if f['action'] != 'delete':\n+ if f['action'] not in ('delete', 'purge'):\n filesToRead.append(f)\n \n filedata = []\n@@ -965,11 +965,11 @@ class P4Sync(Command):\n while j < len(filedata):\n stat = filedata[j]\n j += 1\n- text = [];\n+ text = ''\n while j < len(filedata) and filedata[j]['code'] in ('text', 'unicode', 'binary'):\n- text.append(filedata[j]['data'])\n+ text += filedata[j]['data']\n+ del filedata[j]['data']\n j += 1\n- text = ''.join(text)\n \n if not stat.has_key('depotFile'):\n sys.stderr.write(\"p4 print fails with: %s\\n\" % repr(stat))\n@@ -1038,7 +1038,7 @@ class P4Sync(Command):\n continue\n \n relPath = self.stripRepoPath(file['path'], branchPrefixes)\n- if file[\"action\"] == \"delete\":\n+ if file[\"action\"] in (\"delete\", \"purge\"):\n self.gitStream.write(\"D %s\\n\" % relPath)\n else:\n data = file['data']\n@@ -1077,7 +1077,7 @@ class P4Sync(Command):\n \n cleanedFiles = {}\n for info in files:\n- if info[\"action\"] == \"delete\":\n+ if info[\"action\"] in (\"delete\", \"purge\"):\n continue\n cleanedFiles[info[\"depotFile\"]] = info[\"rev\"]\n \n@@ -1400,7 +1400,7 @@ class P4Sync(Command):\n if change > newestRevision:\n newestRevision = change\n \n- if info[\"action\"] == \"delete\":\n+ if info[\"action\"] in (\"delete\", \"purge\"):\n # don't increase the file cnt, otherwise details[\"depotFile123\"] will have gaps!\n #fileCnt = fileCnt + 1\n continue\n-- \n1.6.0.3.643.g233db\n"},{"id":"95190","messageId":"1226114569-8506-2-git-send-email-thestar@fussycoder.id.au","threadId":"16218","inReplyTo":"1226114569-8506-1-git-send-email-thestar@fussycoder.id.au","subject":"[PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"John Chapman","fromEmail":"thestar@fussycoder.id.au","sentAt":"2008-11-08T03:22:49Z","receivedAt":"2008-11-08T03:22:49Z","isPatch":true,"sender":{"key":"thestar@fussycoder.id.au","avatar":null},"body":"\nSigned-off-by: John Chapman \n---\n contrib/fast-import/git-p4 | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 38d1a17..9f0a5f9 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -316,8 +316,11 @@ def gitBranchExists(branch):\n stderr=subprocess.PIPE, stdout=subprocess.PIPE);\n return proc.wait() == 0;\n \n+_gitConfig = {}\n def gitConfig(key):\n- return read_pipe(\"git config %s\" % key, ignore_error=True).strip()\n+ if not _gitConfig.has_key(key):\n+ _gitConfig[key] = read_pipe(\"git config %s\" % key, ignore_error=True).strip()\n+ return _gitConfig[key]\n \n def p4BranchesInGit(branchesAreInRemotes = True):\n branches = {}\n-- \n1.6.0.3.643.g233db\n"},{"id":"95193","messageId":"ee77f5c20811072119y65738f54o7e6792fb405c142c@mail.gmail.com","threadId":"16218","inReplyTo":"1226114569-8506-2-git-send-email-thestar@fussycoder.id.au","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2008-11-08T05:19:49Z","receivedAt":"2008-11-08T05:19:49Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On Fri, Nov 7, 2008 at 7:22 PM, John Chapman wrote:\n\n> +_gitConfig = {}\n> def gitConfig(key):\n> - return read_pipe(\"git config %s\" % key, ignore_error=True).strip()\n> + if not _gitConfig.has_key(key):\n> + _gitConfig[key] = read_pipe(\"git config %s\" % key, ignore_error=True).strip()\n> + return _gitConfig[key]\n\nIf this is truly a noticeable bottleneck on Windows, something like\nthe following might be even better: (completely untested!)\n\n_gitConfig = None\ndef gitConfig(key):\n if _gitConfig is None:\n lines = read_pipe(\"git config -l\", ignore_error=True).readlines():\n _gitConfig = dict([l.strip().split('=', 1) for l in lines])\n return _gitConfig.get(key, None)\n\n\n\nDave.\n"},{"id":"95202","messageId":"1226127130.8252.6.camel@therock.nsw.bigpond.net.au","threadId":"16218","inReplyTo":"ee77f5c20811072119y65738f54o7e6792fb405c142c@mail.gmail.com","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Arafangion","fromEmail":"thestar@fussycoder.id.au","sentAt":"2008-11-08T06:52:10Z","receivedAt":"2008-11-08T06:52:10Z","isPatch":true,"sender":{"key":"thestar@fussycoder.id.au","avatar":null},"body":"On Fri, 2008-11-07 at 21:19 -0800, David Symonds wrote:\n\n> _gitConfig = None\n> def gitConfig(key):\n> if _gitConfig is None:\n> lines = read_pipe(\"git config -l\", ignore_error=True).readlines():\n> _gitConfig = dict([l.strip().split('=', 1) for l in lines])\n> return _gitConfig.get(key, None)\n\nThat certainly is better, if one can assume that git's configuration is\nsmall. (And relative to the memory usage of the script, it will\ndefinetly be small).\n\nI shall give that a go, although the change won't make it even faster -\nI suspect that much of the performance penalty in windows is the\npathetic fork() performance, particularly as the memory usage of the\nscript increases. (If subprocess does fork() and exec() in order to open\nanother process, in cygwin).\n\nThankyou.\n"},{"id":"95206","messageId":"m3mygaeda0.fsf@localhost.localdomain","threadId":"16218","inReplyTo":"ee77f5c20811072119y65738f54o7e6792fb405c142c@mail.gmail.com","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-08T10:13:32Z","receivedAt":"2008-11-08T10:13:32Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\"David Symonds\" writes:\n\n> On Fri, Nov 7, 2008 at 7:22 PM, John Chapman wrote:\n> \n> > +_gitConfig = {}\n> > def gitConfig(key):\n> > - return read_pipe(\"git config %s\" % key, ignore_error=True).strip()\n> > + if not _gitConfig.has_key(key):\n> > + _gitConfig[key] = read_pipe(\"git config %s\" % key, ignore_error=True).strip()\n> > + return _gitConfig[key]\n> \n> If this is truly a noticeable bottleneck on Windows, something like\n> the following might be even better: (completely untested!)\n> \n> _gitConfig = None\n> def gitConfig(key):\n> if _gitConfig is None:\n> lines = read_pipe(\"git config -l\", ignore_error=True).readlines():\n> _gitConfig = dict([l.strip().split('=', 1) for l in lines])\n> return _gitConfig.get(key, None)\n\nWouldn't it be better to use \"git config -l -z\", split lines at \"\\0\"\n(NUL), and split key from value at first \"\\N\" (CR)? This format was\nmeant for scripts.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"95272","messageId":"7vr65kagvm.fsf@gitster.siamese.dyndns.org","threadId":"16218","inReplyTo":"1226114569-8506-2-git-send-email-thestar@fussycoder.id.au","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-09T18:33:49Z","receivedAt":"2008-11-09T18:33:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"These are patches to fast-import/git-p4, which you two seem to in charge\nof.\n\n From:\tJohn Chapman \n Subject: [PATCH 1/2] Added support for purged files and also optimised memory usage.\n Date:\tSat, 8 Nov 2008 14:22:48 +1100\n Message-Id: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>\n\n From:\tJohn Chapman \n Subject: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.\n Date:\tSat, 8 Nov 2008 14:22:49 +1100\n Message-Id: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>\n\nIt was unfortunately not immediately obvious from the Subject: line what\nthese patches are about, and I am guessing you missed them because of that.\n"},{"id":"95301","messageId":"gf8b2t$5v0$1@ger.gmane.org","threadId":"16218","inReplyTo":"7vr65kagvm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@xs4all.nl","sentAt":"2008-11-10T03:50:52Z","receivedAt":"2008-11-10T03:50:52Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"Hi Junio,\n\nI haven't been involved with git-p4 for a long time. I'm not really fit \nfor judging these patches.\n\n\nJunio C Hamano escreveu:\n> These are patches to fast-import/git-p4, which you two seem to in charge\n> of.\n> \n> From:\tJohn Chapman \n> Subject: [PATCH 1/2] Added support for purged files and also optimised memory usage.\n> Date:\tSat, 8 Nov 2008 14:22:48 +1100\n> Message-Id: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>\n> \n> From:\tJohn Chapman \n> Subject: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.\n> Date:\tSat, 8 Nov 2008 14:22:49 +1100\n> Message-Id: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>\n> \n> It was unfortunately not immediately obvious from the Subject: line what\n> these patches are about, and I am guessing you missed them because of that.\n\n\n-- \n Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen\n"},{"id":"95313","messageId":"4917F2F3.8000408@gmail.com","threadId":"16218","inReplyTo":"1226127130.8252.6.camel@therock.nsw.bigpond.net.au","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Steve Frécinaux","fromEmail":"nudrema@gmail.com","sentAt":"2008-11-10T08:38:11Z","receivedAt":"2008-11-10T08:38:11Z","isPatch":true,"sender":{"key":"nudrema@gmail.com","avatar":null},"body":"Arafangion wrote:\n> On Fri, 2008-11-07 at 21:19 -0800, David Symonds wrote:\n> \n>> _gitConfig = None\n>> def gitConfig(key):\n>> if _gitConfig is None:\n>> lines = read_pipe(\"git config -l\", ignore_error=True).readlines():\n>> _gitConfig = dict([l.strip().split('=', 1) for l in lines])\n>> return _gitConfig.get(key, None)\n> \n> That certainly is better, if one can assume that git's configuration is\n> small. (And relative to the memory usage of the script, it will\n> definetly be small).\n\nWhat about using git config --get-regexp to only get the p4-related \nsettings ?\n\nI don't really know the options used by git-p4, but something like this \nseems like a good candidate to address every trade-off concerns?\n\ngit-config --get-regexp '^(p4|user)\\.'\n"},{"id":"95315","messageId":"200811101046.01543.simon@lst.de","threadId":"16218","inReplyTo":"7vr65kagvm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2008-11-10T09:46:01Z","receivedAt":"2008-11-10T09:46:01Z","isPatch":true,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Sunday 09 November 2008 Junio C Hamano, wrote:\n> These are patches to fast-import/git-p4, which you two seem to in charge\n> of.\n>\n> From:\tJohn Chapman \n> Subject: [PATCH 1/2] Added support for purged files and also optimised\n> memory usage. Date:\tSat, 8 Nov 2008 14:22:48 +1100\n> Message-Id: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>\n>\n> From:\tJohn Chapman \n> Subject: [PATCH 2/2] Cached the git configuration, which is now\n> noticibly faster on windows. Date:\tSat, 8 Nov 2008 14:22:49 +1100\n> Message-Id: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>\n>\n> It was unfortunately not immediately obvious from the Subject: line what\n> these patches are about, and I am guessing you missed them because of that.\n\nAck on both patches. The second one could be done better, as suggested in the \nfollow-ups, but both are clearly an improvement :)\n\n\nSimon\n"},{"id":"95516","messageId":"7vzlk5u5rq.fsf@gitster.siamese.dyndns.org","threadId":"16218","inReplyTo":"200811101046.01543.simon@lst.de","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-12T00:50:17Z","receivedAt":"2008-11-12T00:50:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Simon Hausmann writes:\n\n> On Sunday 09 November 2008 Junio C Hamano, wrote:\n>> These are patches to fast-import/git-p4, which you two seem to in charge\n>> of.\n>>\n>> From:\tJohn Chapman \n>> Subject: [PATCH 1/2] Added support for purged files and also optimised\n>> memory usage. Date:\tSat, 8 Nov 2008 14:22:48 +1100\n>> Message-Id: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>\n>>\n>> From:\tJohn Chapman \n>> Subject: [PATCH 2/2] Cached the git configuration, which is now\n>> noticibly faster on windows. Date:\tSat, 8 Nov 2008 14:22:49 +1100\n>> Message-Id: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>\n>>\n>> It was unfortunately not immediately obvious from the Subject: line what\n>> these patches are about, and I am guessing you missed them because of that.\n>\n> Ack on both patches. The second one could be done better, as suggested\n> in the follow-ups, but both are clearly an improvement :)\n\nThanks, both of you. Will apply.\n"},{"id":"95558","messageId":"1226490890.10685.3.camel@therock.nsw.bigpond.net.au","threadId":"16218","inReplyTo":"7vzlk5u5rq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.","fromName":"Arafangion","fromEmail":"thestar@fussycoder.id.au","sentAt":"2008-11-12T11:54:50Z","receivedAt":"2008-11-12T11:54:50Z","isPatch":true,"sender":{"key":"thestar@fussycoder.id.au","avatar":null},"body":"Thanks for making those patches more visible, however I do feel the need\nto mention one thing I expected to have been raised during patch review:\n1) The memory optimisation may cause significant slowdown, which wasn't\na big issue on my machine, perhaps because I'm _still_ trying to get it\nto work on my particular repo. (It's still using too much memory), and I\nhave a very fast machine. It switches git-p4 from a RAM-intensive app\nto a somewhat-less-RAM-intensive app at the cost of also becomming much\nmore CPU-intensive.\n\n\n\nOn Tue, 2008-11-11 at 16:50 -0800, Junio C Hamano wrote:\n> Simon Hausmann writes:\n> \n> > On Sunday 09 November 2008 Junio C Hamano, wrote:\n> >> These are patches to fast-import/git-p4, which you two seem to in charge\n> >> of.\n> >>\n> >> From:\tJohn Chapman \n> >> Subject: [PATCH 1/2] Added support for purged files and also optimised\n> >> memory usage. Date:\tSat, 8 Nov 2008 14:22:48 +1100\n> >> Message-Id: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>\n> >>\n> >> From:\tJohn Chapman \n> >> Subject: [PATCH 2/2] Cached the git configuration, which is now\n> >> noticibly faster on windows. Date:\tSat, 8 Nov 2008 14:22:49 +1100\n> >> Message-Id: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>\n> >>\n> >> It was unfortunately not immediately obvious from the Subject: line what\n> >> these patches are about, and I am guessing you missed them because of that.\n> >\n> > Ack on both patches. The second one could be done better, as suggested\n> > in the follow-ups, but both are clearly an improvement :)\n> \n> Thanks, both of you. Will apply.\n> \n"}]}