On Mon, Feb 16, 2009 at 3:32 AM, Sebastian Nowicki <sebnow@gmail.com> wrote:
On 16/02/2009, at 5:24 PM, Xavier wrote:
On Mon, Feb 16, 2009 at 4:17 AM, Dan McGee <dpmcgee@gmail.com> wrote:
On Mon, Feb 9, 2009 at 1:17 PM, Xavier <shiningxc@gmail.com> wrote:
For testing, I added all the packages in my cache to a database with and without this patch, and the resulting databases are identical so we should be safe. However, we have twice as many forks as before now, so the time went from 1:36 to 3:14 (for adding 493 packages). I don't know if we should care or not, I guess we usually add a small numbers of packages. If we care, we should have a look at all the slowest operations in repo-add.
It is actually way worse than 2x the number of forks- we now fork once per file *per line* in our .PKGINFO file, whereas before we only forked once per file.
New patch incoming on the ML that should fork as many times as the old version, reducing that bottleneck a lot.
Wow indeed, I always read the old version wrongly, I just realized the old for was split on two lines :
- for line in $(bsdtar -xOf "$pkgfile" .PKGINFO | \ - grep -v "^#" | sed 's|\(\w*\)\s*=\s*\(.*\)|\1="\2"|'); do
How about keeping that part, and just `read`ing the values in:
for line in $(bsdtar -xOf "$pkgfile" .PKGINFO | \ grep -v "^#" | sed 's|\(\w*\)\s*=\s*\(.*\)|\1 "\2"|'); do echo $line | read var val declare $var="$val" # Rest is just as in the patch done
I haven't actually tested it since the regex is somewhat faulty on Mac OS X for some reason, and I'm too lazy to boot Arch up :P.
When you do test it, I would put my money on it being broken the same way the original one is. :)
Note: The substitution now results in "pkgname foo" instead of "pkgname=foo".
This is more or less what I did in my revised patch, if you notice. However, quote marks in the sed call itself are the whole crux of the issue here, so we shouldn't have them there at all. -Dan