# git-p4 fails when cloning a p4 depo. 14 messages from 2007-06-08 to 2007-06-17. Participants: Benjamin Sergeant, Scott Lamb, Simon Hausmann, Han-Wen Nienhuys. Thread: https://gitlist.dev/t/8505 ## Benjamin Sergeant, 2007-06-08 16:41 Subject: git-p4 fails when cloning a p4 depo. Message-ID: <1621f9fa0706080941k67d2878dud8cf06436c67aea0@mail.gmail.com> URL: https://gitlist.dev/e/1621f9fa0706080941k67d2878dud8cf06436c67aea0%40mail.gmail.com ``` I attached a lame patch to die without showing the Python Traceback, but I'd rather succeed :) Maybe there is a different mailing list for git-p4. If there is tell me and I'll post there. Benjamin. [bsergean@flanders sandbox]$ rm -rf dev ; git-p4 clone //Work/Users/Capture3D/A3D810/pdfl/Common/a3d/dev Importing from //Work/Users/Capture3D/A3D810/pdfl/Common/a3d/dev into dev Initialized empty Git repository in .git/ Doing initial import of //Work/Users/Capture3D/A3D810/pdfl/Common/a3d/dev/ from revision #head [{'p4ExitCode': 32512}] Traceback (most recent call last): File "/home/bsergean/src/fast-export/git-p4", line 1489, in main() File "/home/bsergean/src/fast-export/git-p4", line 1484, in main if not cmd.run(args): File "/home/bsergean/src/fast-export/git-p4", line 1395, in run if not P4Sync.run(self, depotPaths): File "/home/bsergean/src/fast-export/git-p4", line 1203, in run self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths) File "/home/bsergean/src/fast-export/git-p4", line 744, in commit self.readP4Files(files) File "/home/bsergean/src/fast-export/git-p4", line 722, in readP4Files contents[stat['depotFile']] = text KeyError: 'depotFile' diff --git a/git-p4 b/git-p4 index 36fe69a..3e1a878 100755 --- a/git-p4 +++ b/git-p4 @@ -707,6 +707,9 @@ class P4Sync(Command): f['rev']) for f in files])) + if "p4ExitCode" in filedata[0]: + die("Problems executing p4"); + j = 0; contents = {} while j < len(filedata): ``` ## Benjamin Sergeant, 2007-06-08 18:13 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <1621f9fa0706081113w7bb765ebx74f03a7407b753cb@mail.gmail.com> URL: https://gitlist.dev/e/1621f9fa0706081113w7bb765ebx74f03a7407b753cb%40mail.gmail.com In-Reply-To: <1621f9fa0706080941k67d2878dud8cf06436c67aea0@mail.gmail.com> ``` A perforce command with all the files in the repo is generated to get all the file content. Here is a patch to break it into multiple successive perforce command who uses 4K of parameter max, and collect the output for later. It works, but not for big depos, because the whole perforce depo content is stored in memory in P4Sync.run(), and it looks like mine is bigger than 2 Gigs, so I had to kill the process. diff --git a/git-p4 b/git-p4 index 36fe69a..906b193 100755 --- a/git-p4 +++ b/git-p4 @@ -703,9 +703,22 @@ class P4Sync(Command): if not files: return - filedata = p4CmdList('print %s' % ' '.join(['"%s#%s"' % (f['path'], - f['rev']) - for f in files])) + # We cannot put all the files on the command line + # OS have limitations on the max lenght of arguments + # POSIX says it's 4096 bytes, default for Linux seems to be 130 K. + # and all OS from the table below seems to be higher than POSIX. + # See http://www.in-ulm.de/~mascheck/various/argmax/ + chunk = '' + filedata = [] + for i in xrange(len(files)): + f = files[i] + chunk += '"%s#%s" ' % (f['path'], f['rev']) + if len(chunk) > 4000 or i == len(files)-1: + data = p4CmdList('print %s' % chunk) + if "p4ExitCode" in data[0]: + die("Problems executing p4. Error: [%d]." % (data[0]['p4ExitCode'])); + filedata.extend(data) + chunk = '' j = 0; contents = {} @@ -1486,3 +1499,5 @@ def main(): if __name__ == '__main__': main() + +# vim: set filetype=python sts=4 sw=4 et si : On 6/8/07, Benjamin Sergeant wrote: > I attached a lame patch to die without showing the Python Traceback, > but I'd rather succeed :) > Maybe there is a different mailing list for git-p4. If there is tell > me and I'll post there. > > Benjamin. > > [bsergean@flanders sandbox]$ rm -rf dev ; git-p4 clone > //Work/Users/Capture3D/A3D810/pdfl/Common/a3d/dev > Importing from //Work/Users/Capture3D/A3D810/pdfl/Common/a3d/dev into dev > Initialized empty Git repository in .git/ > Doing initial import of > //Work/Users/Capture3D/A3D810/pdfl/Common/a3d/dev/ from revision #head > [{'p4ExitCode': 32512}] > Traceback (most recent call last): > File "/home/bsergean/src/fast-export/git-p4", line 1489, in > main() > File "/home/bsergean/src/fast-export/git-p4", line 1484, in main > if not cmd.run(args): > File "/home/bsergean/src/fast-export/git-p4", line 1395, in run > if not P4Sync.run(self, depotPaths): > File "/home/bsergean/src/fast-export/git-p4", line 1203, in run > self.commit(details, self.extractFilesFromCommit(details), > self.branch, self.depotPaths) > File "/home/bsergean/src/fast-export/git-p4", line 744, in commit > self.readP4Files(files) > File "/home/bsergean/src/fast-export/git-p4", line 722, in readP4Files > contents[stat['depotFile']] = text > KeyError: 'depotFile' > > ``` ## Scott Lamb, 2007-06-08 21:31 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <4669CAB4.5080507@slamb.org> URL: https://gitlist.dev/e/4669CAB4.5080507%40slamb.org In-Reply-To: <1621f9fa0706081113w7bb765ebx74f03a7407b753cb@mail.gmail.com> ``` Benjamin Sergeant wrote: > A perforce command with all the files in the repo is generated to get > all the file content. > Here is a patch to break it into multiple successive perforce command > who uses 4K of parameter max, and collect the output for later. > > It works, but not for big depos, because the whole perforce depo > content is stored in memory in P4Sync.run(), and it looks like mine is > bigger than 2 Gigs, so I had to kill the process. Hmm. I tried git-p4 out on Sunday, and it definitely didn't do this then...this commit must have showed up since: commit 6460cf12df4556f889888b5f0b49e07040747e6f Author: Han-Wen Nienhuys Date: Wed May 23 18:49:35 2007 -0300 Read p4 files in one batch. I believe HEAD at the time was this: commit 458e0545cb3dd03af9cd1a61480cbb764639043a Author: Simon Hausmann Date: Mon May 28 19:24:57 2007 +0200 Fix typo in listExistingP4Branches that broke sync. so you might try checking out an old version, and I'll go RTFM on reading merge history, because I can't figure out when this happened. > > > diff --git a/git-p4 b/git-p4 > index 36fe69a..906b193 100755 > --- a/git-p4 > +++ b/git-p4 > @@ -703,9 +703,22 @@ class P4Sync(Command): > if not files: > return > > - filedata = p4CmdList('print %s' % ' '.join(['"%s#%s"' % > (f['path'], > - f['rev']) > - for f in files])) > + # We cannot put all the files on the command line > + # OS have limitations on the max lenght of arguments > + # POSIX says it's 4096 bytes, default for Linux seems to be 130 K. > + # and all OS from the table below seems to be higher than POSIX. > + # See http://www.in-ulm.de/~mascheck/various/argmax/ No need to hardcode - from Python this is os.sysconf(os.sysconf_names['SC_ARG_MAX']) > + chunk = '' > + filedata = [] > + for i in xrange(len(files)): > + f = files[i] > + chunk += '"%s#%s" ' % (f['path'], f['rev']) > + if len(chunk) > 4000 or i == len(files)-1: > + data = p4CmdList('print %s' % chunk) > + if "p4ExitCode" in data[0]: > + die("Problems executing p4. Error: [%d]." % > (data[0]['p4ExitCode'])); > + filedata.extend(data) > + chunk = '' > > j = 0; > contents = {} > @@ -1486,3 +1499,5 @@ def main(): > > if __name__ == '__main__': > main() > + > +# vim: set filetype=python sts=4 sw=4 et si : ``` ## Scott Lamb, 2007-06-08 21:34 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <4669CB75.7060009@slamb.org> URL: https://gitlist.dev/e/4669CB75.7060009%40slamb.org In-Reply-To: <4669CAB4.5080507@slamb.org> ``` Scott Lamb wrote: > No need to hardcode - from Python this is > os.sysconf(os.sysconf_names['SC_ARG_MAX']) In fact, just os.sysconf('SC_ARG_MAX') will do. ``` ## Benjamin Sergeant, 2007-06-08 22:04 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <1621f9fa0706081504l6106c639oe57c9fd74ebd097a@mail.gmail.com> URL: https://gitlist.dev/e/1621f9fa0706081504l6106c639oe57c9fd74ebd097a%40mail.gmail.com In-Reply-To: <4669CB75.7060009@slamb.org> ``` On 6/8/07, Scott Lamb wrote: > Scott Lamb wrote: > > No need to hardcode - from Python this is > > os.sysconf(os.sysconf_names['SC_ARG_MAX']) > > In fact, just os.sysconf('SC_ARG_MAX') will do. > magic number are lot of fun, why would you want to use the clean method :) So are you saying that in the old days, git-p4 was importing the p4 depo in small slices to not overkill the process memory (in case the depo is big) ? BTW, there is the whole universe in my depot, so using -//Work/Users in my client specification I usually manage to have less megs of code on my disk after a sync. This way the git-p4 clone would not use too much memory. But we would have to change the way git-p4 works, it should be able to read a full client view instead of just a single perforce path. Would you give me the git command to fetch the up git clone --date ? Thanks, Benjamin. ``` ## Benjamin Sergeant, 2007-06-08 22:25 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <1621f9fa0706081525n64e5d3a3i6379030e1f619ef0@mail.gmail.com> URL: https://gitlist.dev/e/1621f9fa0706081525n64e5d3a3i6379030e1f619ef0%40mail.gmail.com In-Reply-To: <1621f9fa0706081504l6106c639oe57c9fd74ebd097a@mail.gmail.com> ``` I got my git-p4 from here BTW. http://repo.or.cz/w/fast-export.git On 6/8/07, Benjamin Sergeant wrote: > On 6/8/07, Scott Lamb wrote: > > Scott Lamb wrote: > > > No need to hardcode - from Python this is > > > os.sysconf(os.sysconf_names['SC_ARG_MAX']) > > > > In fact, just os.sysconf('SC_ARG_MAX') will do. > > > > magic number are lot of fun, why would you want to use the clean method :) > > So are you saying that in the old days, git-p4 was importing the p4 > depo in small slices to not overkill the process memory (in case the > depo is big) ? > > BTW, there is the whole universe in my depot, so using -//Work/Users > in my client specification I usually manage to have less megs of code > on my disk after a sync. > This way the git-p4 clone would not use too much memory. But we would > have to change the way git-p4 works, it should be able to read a full > client view instead of just a single perforce path. > > Would you give me the git command to fetch the up > git clone --date ? > > Thanks, > Benjamin. > ``` ## Simon Hausmann, 2007-06-08 22:38 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <200706090038.33799.simon@lst.de> URL: https://gitlist.dev/e/200706090038.33799.simon%40lst.de In-Reply-To: <1621f9fa0706081113w7bb765ebx74f03a7407b753cb@mail.gmail.com> ``` On Friday 08 June 2007 20:13:55 Benjamin Sergeant wrote: > A perforce command with all the files in the repo is generated to get > all the file content. > Here is a patch to break it into multiple successive perforce command > who uses 4K of parameter max, and collect the output for later. > > It works, but not for big depos, because the whole perforce depo > content is stored in memory in P4Sync.run(), and it looks like mine is > bigger than 2 Gigs, so I had to kill the process. I'd be generally fine with splitting up the "p4 print ..." calls into chunks but you have a good point with the memory usage. The old approach of calling print per file did not have any of those limitations. Han-Wen, what do you think? How much of a performance improvement is the batched print? (I didn't notice any immediate difference, but then I have a very fast connection to the perforce server and usually small changesets) Simon ``` ## Han-Wen Nienhuys, 2007-06-08 23:33 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <4669E73F.2040702@xs4all.nl> URL: https://gitlist.dev/e/4669E73F.2040702%40xs4all.nl In-Reply-To: <1621f9fa0706081504l6106c639oe57c9fd74ebd097a@mail.gmail.com> ``` Benjamin Sergeant escreveu: > So are you saying that in the old days, git-p4 was importing the p4 > depo in small slices to not overkill the process memory (in case the > depo is big) ? no, in the "old days" git-p4 used a separate p4 invocation for each file. -- Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen ``` ## Benjamin Sergeant, 2007-06-09 00:32 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <1621f9fa0706081732k7a31782cv26f3295245057b6f@mail.gmail.com> URL: https://gitlist.dev/e/1621f9fa0706081732k7a31782cv26f3295245057b6f%40mail.gmail.com In-Reply-To: <4669E73F.2040702@xs4all.nl> ``` On 6/8/07, Han-Wen Nienhuys wrote: > Benjamin Sergeant escreveu: > > > So are you saying that in the old days, git-p4 was importing the p4 > > depo in small slices to not overkill the process memory (in case the > > depo is big) ? > > no, in the "old days" git-p4 used a separate p4 invocation for each file. > Anyway, in case you hit command line lenght limit here it is. That might be interesting for the "next days" :) Benjamin. [bsergean@flanders fast-export]$ git format-patch -k -m --stdout origin From 45f2dbdb9a8c0b3beb007ae892613cdc4afab80a Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Fri, 8 Jun 2007 09:58:57 -0700 Subject: Split p4 print call into multiple call to not exceed the command line lenght maximum --- git-p4 | 21 ++++++++++++++++++--- 1 files changed, 18 insertions(+), 3 deletions(-) diff --git a/git-p4 b/git-p4 index 36fe69a..906b193 100755 --- a/git-p4 +++ b/git-p4 @@ -703,9 +703,22 @@ class P4Sync(Command): if not files: return - filedata = p4CmdList('print %s' % ' '.join(['"%s#%s"' % (f['path'], - f['rev']) - for f in files])) + # We cannot put all the files on the command line + # OS have limitations on the max lenght of arguments + # POSIX says it's 4096 bytes, default for Linux seems to be 130 K. + # and all OS from the table below seems to be higher than POSIX. + # See http://www.in-ulm.de/~mascheck/various/argmax/ + chunk = '' + filedata = [] + for i in xrange(len(files)): + f = files[i] + chunk += '"%s#%s" ' % (f['path'], f['rev']) + if len(chunk) > 4000 or i == len(files)-1: + data = p4CmdList('print %s' % chunk) + if "p4ExitCode" in data[0]: + die("Problems executing p4. Error: [%d]." % (data[0]['p4ExitCode'])); + filedata.extend(data) + chunk = '' j = 0; contents = {} @@ -1486,3 +1499,5 @@ def main(): if __name__ == '__main__': main() + +# vim: set filetype=python sts=4 sw=4 et si : -- 1.5.0.4 >From dd9975708433efeec37b608755f54fbeaedf0f3f Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Fri, 8 Jun 2007 10:20:39 -0700 Subject: Use os.sysconf('SC_ARG_MAX') to retrieve the max value, and build the string using join (faster) --- git-p4 | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/git-p4 b/git-p4 index 906b193..8dc1963 100755 --- a/git-p4 +++ b/git-p4 @@ -705,20 +705,23 @@ class P4Sync(Command): # We cannot put all the files on the command line # OS have limitations on the max lenght of arguments - # POSIX says it's 4096 bytes, default for Linux seems to be 130 K. - # and all OS from the table below seems to be higher than POSIX. # See http://www.in-ulm.de/~mascheck/various/argmax/ - chunk = '' + chunks = [] + chunkLenght = 0 filedata = [] + maxlenght = max(int(os.sysconf('SC_ARG_MAX') * 0.90), 4000) + print maxlenght for i in xrange(len(files)): f = files[i] - chunk += '"%s#%s" ' % (f['path'], f['rev']) - if len(chunk) > 4000 or i == len(files)-1: - data = p4CmdList('print %s' % chunk) + chunkLenght += len(f['path']) + len(f['rev']) + chunks.append('"%s#%s" ' % (f['path'], f['rev'])) + if chunkLenght > maxlenght or i == len(files)-1: + data = p4CmdList('print %s' % ' '.join(chunks)) if "p4ExitCode" in data[0]: die("Problems executing p4. Error: [%d]." % (data[0]['p4ExitCode'])); filedata.extend(data) - chunk = '' + chunks = [] + chunkLenght = 0 j = 0; contents = {} -- 1.5.0.4 ``` ## Han-Wen Nienhuys, 2007-06-12 01:07 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <466DF1E1.9060408@xs4all.nl> URL: https://gitlist.dev/e/466DF1E1.9060408%40xs4all.nl In-Reply-To: <200706090038.33799.simon@lst.de> ``` Simon Hausmann escreveu: > On Friday 08 June 2007 20:13:55 Benjamin Sergeant wrote: >> A perforce command with all the files in the repo is generated to get >> all the file content. >> Here is a patch to break it into multiple successive perforce command >> who uses 4K of parameter max, and collect the output for later. >> >> It works, but not for big depos, because the whole perforce depo >> content is stored in memory in P4Sync.run(), and it looks like mine is >> bigger than 2 Gigs, so I had to kill the process. > > I'd be generally fine with splitting up the "p4 print ..." calls into chunks > but you have a good point with the memory usage. The old approach of calling > print per file did not have any of those limitations. Han-Wen, what do you > think? How much of a performance improvement is the batched print? One unscientific measurement (getting 1 file vs. 30 files) indicates that this is about 5x times faster. -- Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen ``` ## Han-Wen Nienhuys, 2007-06-12 01:08 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <466DF1FB.6010001@xs4all.nl> URL: https://gitlist.dev/e/466DF1FB.6010001%40xs4all.nl In-Reply-To: <200706090038.33799.simon@lst.de> ``` Simon Hausmann escreveu: > On Friday 08 June 2007 20:13:55 Benjamin Sergeant wrote: >> A perforce command with all the files in the repo is generated to get >> all the file content. >> Here is a patch to break it into multiple successive perforce command >> who uses 4K of parameter max, and collect the output for later. >> >> It works, but not for big depos, because the whole perforce depo >> content is stored in memory in P4Sync.run(), and it looks like mine is >> bigger than 2 Gigs, so I had to kill the process. > > I'd be generally fine with splitting up the "p4 print ..." calls into chunks > but you have a good point with the memory usage. The old approach of calling > print per file did not have any of those limitations. Han-Wen, what do you > think? How much of a performance improvement is the batched print? One unscientific measurement (getting 1 file vs. 30 files) indicates that this is about 5x times faster. -- Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen ``` ## Han-Wen Nienhuys, 2007-06-12 01:13 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <466DF32D.802@xs4all.nl> URL: https://gitlist.dev/e/466DF32D.802%40xs4all.nl In-Reply-To: <1621f9fa0706081113w7bb765ebx74f03a7407b753cb@mail.gmail.com> ``` Benjamin Sergeant escreveu: > A perforce command with all the files in the repo is generated to get > all the file content. > Here is a patch to break it into multiple successive perforce command > who uses 4K of parameter max, and collect the output for later. > > It works, but not for big depos, because the whole perforce depo > content is stored in memory in P4Sync.run(), and it looks like mine is > bigger than 2 Gigs, so I had to kill the process. General idea of the patch is ok. some nits: > + chunk = '' > + filedata = [] > + for i in xrange(len(files)): why not for f in files: ? > + f = files[i] > + chunk += '"%s#%s" ' % (f['path'], f['rev']) > + if len(chunk) > 4000 or i == len(files)-1: 4k seems reasonable enough, but can you take the min() with os.sysconf('SC_ARG_MAX') ? Can you address this and resend so we can apply the patch? Thanks. -- Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen ``` ## Simon Hausmann, 2007-06-17 08:11 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <200706171011.52492.simon@lst.de> URL: https://gitlist.dev/e/200706171011.52492.simon%40lst.de In-Reply-To: <466DF32D.802@xs4all.nl> ``` On Tuesday 12 June 2007 03:13:17 Han-Wen Nienhuys wrote: > Benjamin Sergeant escreveu: > > A perforce command with all the files in the repo is generated to get > > all the file content. > > Here is a patch to break it into multiple successive perforce command > > who uses 4K of parameter max, and collect the output for later. > > > > It works, but not for big depos, because the whole perforce depo > > content is stored in memory in P4Sync.run(), and it looks like mine is > > bigger than 2 Gigs, so I had to kill the process. > > General idea of the patch is ok. some nits: > > + chunk = '' > > + filedata = [] > > + for i in xrange(len(files)): > > why not > > for f in files: > > ? It seems 'i' is used a bit later. Is there a nicer way to express this in python? > > + f = files[i] > > + chunk += '"%s#%s" ' % (f['path'], f['rev']) > > + if len(chunk) > 4000 or i == len(files)-1: > > 4k seems reasonable enough, but can you take the min() with > os.sysconf('SC_ARG_MAX') ? > > Can you address this and resend so we can apply the patch? > Thanks. Since I ran into the very problem of a too long commandline myself yesterday I took the liberty of adding the SC_ARG_MAX bit to Benjamin's patch and comitting it then. Simon ``` ## Benjamin Sergeant, 2007-06-17 16:09 Subject: Re: git-p4 fails when cloning a p4 depo. Message-ID: <1621f9fa0706170909s779b471fl872c027b8e7b446c@mail.gmail.com> URL: https://gitlist.dev/e/1621f9fa0706170909s779b471fl872c027b8e7b446c%40mail.gmail.com In-Reply-To: <200706171011.52492.simon@lst.de> ``` On 6/17/07, Simon Hausmann wrote: > On Tuesday 12 June 2007 03:13:17 Han-Wen Nienhuys wrote: > > Benjamin Sergeant escreveu: > > > A perforce command with all the files in the repo is generated to get > > > all the file content. > > > Here is a patch to break it into multiple successive perforce command > > > who uses 4K of parameter max, and collect the output for later. > > > > > > It works, but not for big depos, because the whole perforce depo > > > content is stored in memory in P4Sync.run(), and it looks like mine is > > > bigger than 2 Gigs, so I had to kill the process. > > > > General idea of the patch is ok. some nits: > > > + chunk = '' > > > + filedata = [] > > > + for i in xrange(len(files)): > > > > why not > > > > for f in files: > > > > ? > > It seems 'i' is used a bit later. Is there a nicer way to express this in > python? > > > > + f = files[i] > > > + chunk += '"%s#%s" ' % (f['path'], f['rev']) > > > + if len(chunk) > 4000 or i == len(files)-1: > > > > 4k seems reasonable enough, but can you take the min() with > > os.sysconf('SC_ARG_MAX') ? > > > > Can you address this and resend so we can apply the patch? > > Thanks. > > Since I ran into the very problem of a too long commandline myself yesterday I > took the liberty of adding the SC_ARG_MAX bit to Benjamin's patch and > comitting it then. > Cool. (probably useless but) For what it's worth ; Here is a tar file with 2 patchs: - The original one - The second one that adds the SC_ARG_MAX BTW, doing a += on a string is not supposed to be fast, appending elem to a sequence and then using ' '.join on them to build the big string is said to be faster (I did not timed it thought). (in the second patch also) Thanks, Benjamin. > > Simon > > ```