{"thread":{"id":"4240","subject":"[PATCH] cvsimport: introduce -L option to workaround memory leaks","startedAt":"2006-05-22T11:38:08Z","lastAt":"2006-05-26T06:02:30Z","messageCount":10,"participants":["Martin Langhoff","Linus Torvalds","Martin Langhoff (CatalystIT)","Theodore Tso","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"20442","messageId":"11482978883713-git-send-email-martin@catalyst.net.nz","threadId":"4240","inReplyTo":null,"subject":"[PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2006-05-22T11:38:08Z","receivedAt":"2006-05-22T11:38:08Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Signed-off-by: Martin Langhoff \n\n---\nThis is ugly, but while I work on cleaning up the leak\nthat seems to be somewhere in the commit() sub, we may\nas well set up a workaround.\n\nI am not 100% happy woth including this in git.git. \nIn any case, I hope we can revert it soon.\n\n---\n\n git-cvsimport.perl | 8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\n64ea3c83d8cd176ee972055bd1d11f398655dad8\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex c0ae00b..c1923d1 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -29,7 +29,7 @@ use IPC::Open2;\n $SIG{'PIPE'}=\"IGNORE\";\n $ENV{'TZ'}=\"UTC\";\n \n-our($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S);\n+our($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L);\n my (%conv_author_name, %conv_author_email);\n \n sub usage() {\n@@ -85,7 +85,7 @@ sub write_author_info($) {\n \tclose ($f);\n }\n \n-getopts(\"hivmkuo:d:p:C:z:s:M:P:A:S:\") or usage();\n+getopts(\"hivmkuo:d:p:C:z:s:M:P:A:S:L:\") or usage();\n usage if $opt_h;\n \n @ARGV <= 1 or usage();\n@@ -716,6 +716,7 @@ my $commit = sub {\n \t}\n };\n \n+my $commitcount = 1;\n while() {\n \tchomp;\n \tif($state == 0 and /^-+$/) {\n@@ -849,6 +850,9 @@ #\tVERSION:1.96->1.96.2.1\n \t} elsif($state == 9 and /^\\s*$/) {\n \t\t$state = 10;\n \t} elsif(($state == 9 or $state == 10) and /^-+$/) {\n+\t\tif ($opt_L && $commitcount++ >= $opt_L) {\n+\t\t\tlast;\n+\t\t}\n \t\t&$commit();\n \t\t$state = 1;\n \t} elsif($state == 11 and /^-+$/) {\n-- \n1.3.2.g82000\n"},{"id":"20504","messageId":"Pine.LNX.4.64.0605221926270.3697@g5.osdl.org","threadId":"4240","inReplyTo":"11482978883713-git-send-email-martin@catalyst.net.nz","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-23T02:28:37Z","receivedAt":"2006-05-23T02:28:37Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nThis stupid patch on top of yours seems to make git happier. It's \ndisgusting, I know, but it just repacks things every kilo-commit.\n\nI actually think that I found a real ext3 performance bug from trying to \ndetermine why git sometimes slows down ridiculously when the tree has been \nallowed to go too long without a repack.\n\n\t\tLinus\n\n---\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex fb56278..c141f5e 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -853,10 +853,14 @@ #\tVERSION:1.96->1.96.2.1\n \t} elsif($state == 9 and /^\\s*$/) {\n \t\t$state = 10;\n \t} elsif(($state == 9 or $state == 10) and /^-+$/) {\n-\t\tif ($opt_L && $commitcount++ >= $opt_L) {\n+\t\t$commitcount++;\n+\t\tif ($opt_L && $commitcount > $opt_L) {\n \t\t\tlast;\n \t\t}\n \t\tcommit();\n+\t\tif (($commitcount & 1023) == 0) {\n+\t\t\tsystem(\"git repack -a -d\");\n+\t\t}\n \t\t$state = 1;\n \t} elsif($state == 11 and /^-+$/) {\n \t\t$state = 1;\n"},{"id":"20505","messageId":"44727E5F.5070309@catalyst.net.nz","threadId":"4240","inReplyTo":"Pine.LNX.4.64.0605221926270.3697@g5.osdl.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Martin Langhoff (CatalystIT)","fromEmail":"martin@catalyst.net.nz","sentAt":"2006-05-23T03:15:43Z","receivedAt":"2006-05-23T03:15:43Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Linus Torvalds wrote:\n\n> \n> This stupid patch on top of yours seems to make git happier. It's \n> disgusting, I know, but it just repacks things every kilo-commit.\n> \n> I actually think that I found a real ext3 performance bug from trying to \n> determine why git sometimes slows down ridiculously when the tree has been \n> allowed to go too long without a repack.\n> \n\nAcked (in case anyone cares for such an obvious one), and thanks! I \nthought of doing that last night together with that exact patch, but I \nwas focussing on the leak.\n\ncheers,\n\n\nm\n-- \n-----------------------------------------------------------------------\nMartin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington\nWEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St\nOFFICE: +64(4)916-7224 MOB: +64(21)364-017\n Make things as simple as possible, but no simpler - Einstein\n-----------------------------------------------------------------------\n"},{"id":"20552","messageId":"20060523153636.GA21506@thunk.org","threadId":"4240","inReplyTo":"Pine.LNX.4.64.0605221926270.3697@g5.osdl.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2006-05-23T15:36:36Z","receivedAt":"2006-05-23T15:36:36Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"On Mon, May 22, 2006 at 07:28:37PM -0700, Linus Torvalds wrote:\n> \n> \n> This stupid patch on top of yours seems to make git happier. It's \n> disgusting, I know, but it just repacks things every kilo-commit.\n> \n> I actually think that I found a real ext3 performance bug from trying to \n> determine why git sometimes slows down ridiculously when the tree has been \n> allowed to go too long without a repack.\n\nDo you have dir_index (the hashed btree) feature enabled by any chance?\n\n\t\t\t\t\t\t- Ted\n"},{"id":"20553","messageId":"Pine.LNX.4.64.0605230848060.5623@g5.osdl.org","threadId":"4240","inReplyTo":"20060523153636.GA21506@thunk.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-23T16:05:36Z","receivedAt":"2006-05-23T16:05:36Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 23 May 2006, Theodore Tso wrote:\n> On Mon, May 22, 2006 at 07:28:37PM -0700, Linus Torvalds wrote:\n> > \n> > I actually think that I found a real ext3 performance bug from trying to \n> > determine why git sometimes slows down ridiculously when the tree has been \n> > allowed to go too long without a repack.\n> \n> Do you have dir_index (the hashed btree) feature enabled by any chance?\n\nNo, and I know I probably should, since it would hopefully help git usage.\n\nBut my problem actually happens even with moderately sized directories: \nthey were just 40kB or so in size, and the problem isn't high system CPU \nusage, but tons of extra IO. I ran things on a machine with 2GB of RAM, \nand as far as I could tell, the working set _should_ have fit into memory, \nbut CPU utilization was consistently in the 1% range.\n\nNow, it's possible that I'm just wrong, and it really didn't fit in \nmemory, but I I _suspect_ that the issue is that ext3 directory handling \nstill uses the \"buffer_head\" thing rather than the page cache, and that we \nsimply don't LRU the memory appropriately so we don't let the memory \npressure expand the buffer cache.\n\nNow, using buffer cache in this day and age is insane and horrible \n(there's a reason I suspect the LRU doesn't work that well: the buffer \nheads aren't supposed to be used as a cache, and people are supposed to \nuse the page cache for it these days), but Andrew tells me that the whole \nJBD thing basically requires it. Whatever.\n\nNow, repacking obviously hides it entirely (because then the load becomes \nentirely a page-cache load, and the kernel does _that_ beautifully), but \nI'm a bit bummed that I think I hit an ext3 braindamage.\n\nSo an unpacked git archive on ext3 (but not ext2, I believe: ext2 should \nuse the page cache for directories) ends up being very buffer-cache \nintensive. And the buffer cache is basically deprecated..\n\n\t\tLinus\n\nPS. I'll see if I can figure out the problem, and maybe the good news is \nthat I'll be able to just fix a real kernel performance issue. Still, \nthere's a _reason_ we tried to get away from the buffer heads as a caching \nentity..\n"},{"id":"20554","messageId":"Pine.LNX.4.64.0605230919190.5623@g5.osdl.org","threadId":"4240","inReplyTo":"Pine.LNX.4.64.0605230848060.5623@g5.osdl.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-23T16:25:47Z","receivedAt":"2006-05-23T16:25:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 23 May 2006, Linus Torvalds wrote:\n> \n> So an unpacked git archive on ext3 (but not ext2, I believe: ext2 should \n> use the page cache for directories) ends up being very buffer-cache \n> intensive. And the buffer cache is basically deprecated..\n\nA few notes: I'm not 100% sure things really fit in the 2GB (*), so it \nreally may be IO limited, and the low CPU use is just because that machine \nalso happens to have a dang fast next-gen Intel CPU that hasn't even been \nreleased yet.\n\nAlso, I do realize that hashed directories should actually decrease the \nbuffer cache pressure too, just because we wouldn't need to read all of \nthe directory for a lookup. \n\n\t\tLinus\n\n(*) cvsps itself grows to 1.6GB of the 2GB and while that memory should be \nlargely idle, the problem may simply be that we don't swap it out eagerly \nenough. Allowing filesystem metadata to swap out processes is something \nwe've tuned against, because it tends to result in horrible interactive \nbehaviour after a nightly \"updatedb\" run. So it's entirely possible that \nthis is all normal..\n"},{"id":"20717","messageId":"46a038f90605251742p2435ae23k8bfbb98409a30c1c@mail.gmail.com","threadId":"4240","inReplyTo":"Pine.LNX.4.64.0605221926270.3697@g5.osdl.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-05-26T00:42:09Z","receivedAt":"2006-05-26T00:42:09Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 5/23/06, Linus Torvalds wrote:\n>\n> This stupid patch on top of yours seems to make git happier. It's\n> disgusting, I know, but it just repacks things every kilo-commit.\n\nCall me slow, but I am still running and rerunning that gentoo import. ;-)\n\nThe current import has reached ~200K commits, and .git is 450MB, while\nthe checked out tree is 230MB (680MB with .git). At this stage, git\nrepack -a -d is too memory hungry. I just had to kill it after it took\n2GB and was still growing fast. So I have dropped the -a in my test\nimport for the time being.\n\nOther than that, we are doing ~6K commits per hour on a 2GB 2GHz Opteron.\n\ncheers,\n\n\nmartin\n"},{"id":"20736","messageId":"Pine.LNX.4.64.0605252204590.5623@g5.osdl.org","threadId":"4240","inReplyTo":"46a038f90605251742p2435ae23k8bfbb98409a30c1c@mail.gmail.com","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-26T05:20:39Z","receivedAt":"2006-05-26T05:20:39Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 26 May 2006, Martin Langhoff wrote:\n> \n> Call me slow, but I am still running and rerunning that gentoo import. ;-)\n\nI'm doing it too, just for fun.\n\nOf course, since I'm doing this on a machine that basically has a laptop \ndisk, the \"just for fun\" part is a bit sad. It's waiting for disk about \n25% of the time ;/\n\nAnd it's slow as hell. I really wish we could do better on the CVS import \nfront. \n\n> The current import has reached ~200K commits, and .git is 450MB, while\n> the checked out tree is 230MB (680MB with .git). At this stage, git\n> repack -a -d is too memory hungry.\n\nI've got 2GB in that puppy, and \"repack -a -d\" is fine for me. I'm not \nquite up to 200k commits yet (I'm at 160k), but the repacking is certainly \nfaster than the rest of the import.. Gaah. \n\nIt's \"git-rev-list --objects\" that is the memory sucker for me, the \npacking itself doesn't seem to be too bad.\n\nThe biggest cost seems to be git-write-tree, which is about 0.225 seconds \nfor me on that tree on that machine. Which _should_ mean that we could do \n4 commits a second, but that sure as hell ain't how it works out. It seems \nto do about 1.71 commits a second for me on that tree, which is pretty \ndamn pitiful. Some cvs overhead, and probably some other git overhead too.\n\n(That's a 2GHz Merom, so the fact that you get ~6k commits per hour on \nyour 2GHz Opteron is about the same speed - I suspect you're also at least \npartly limited by disk, our numbers seem to match pretty well).\n\n200k commits at 6k commits per hour is about a day and a half (plus the \noccasional packing load). Taking that long to import a CVS archive is \nhorrible. But I guess it _is_ several years of work, and I guess you \nreally have to do it only once, but still.\n\n\t\t\tLinus\n"},{"id":"20737","messageId":"e563o9$f6a$1@sea.gmane.org","threadId":"4240","inReplyTo":"Pine.LNX.4.64.0605252204590.5623@g5.osdl.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-05-26T05:29:49Z","receivedAt":"2006-05-26T05:29:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Linus Torvalds wrote:\n\n> 200k commits at 6k commits per hour is about a day and a half (plus the \n> occasional packing load). Taking that long to import a CVS archive is \n> horrible. But I guess it _is_ several years of work, and I guess you \n> really have to do it only once, but still.\n\nAnd how parsecvs (which as far as I remember didn't have incremental mode)\ncompares wrt speed to git-cvsimport? It is supposed to be faster...\n\n-- \nJakub Narebski\nWarsaw, Poland\n"},{"id":"20739","messageId":"46a038f90605252302v5ff1a3a8vb4053b7da8eb7a68@mail.gmail.com","threadId":"4240","inReplyTo":"Pine.LNX.4.64.0605252204590.5623@g5.osdl.org","subject":"Re: [PATCH] cvsimport: introduce -L option to workaround memory leaks","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-05-26T06:02:30Z","receivedAt":"2006-05-26T06:02:30Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 5/26/06, Linus Torvalds wrote:\n> I'm doing it too, just for fun.\n\nWell, it's good to not be so alone in our definition of fun ;-)\n\n> Of course, since I'm doing this on a machine that basically has a laptop\n> disk, the \"just for fun\" part is a bit sad. It's waiting for disk about\n> 25% of the time ;/\n\nOuch.\n\n> And it's slow as hell. I really wish we could do better on the CVS import\n> front.\n\nMe too. However, I don't think the perl part is so costly anymore.\nIt's down to waiting on IO. git-write-tree is also prominently there.\nIt takes a lot of memory in some writes -- I had thought it'd be\ncheaper as it takes one tree object at the time...\n\nI also have a trivial patch that I haven't posted yet, that runs cvsps\nto a tempfile, and then reads the file. Serialising the tasks means\nthat we don't carry around cvsps' memory footprint during the import\nitself.\n\n...\n> It's \"git-rev-list --objects\" that is the memory sucker for me, the\n> packing itself doesn't seem to be too bad.\n\n\nNo, you're right, it's git-rev-list that gets called during the\nrepack. But it was pushing everything it could to swap. Once it didn't\nfit in memory, it hit a brick wall :(\n\n> The biggest cost seems to be git-write-tree, which is about 0.225 seconds\n> for me on that tree on that machine. Which _should_ mean that we could do\n> 4 commits a second, but that sure as hell ain't how it works out. It seems\n> to do about 1.71 commits a second for me on that tree, which is pretty\n> damn pitiful. Some cvs overhead, and probably some other git overhead too.\n\nWell, we _have_ to fetch the file. I guess you are thinking of\nextracting if frrom the RCS ,v file directly? One tihng that I found\nthat seemed to speed things up a bit was to declare TMPDIR to be a\ndirectory in the same partition.\n\n> (That's a 2GHz Merom, so the fact that you get ~6k commits per hour on\n> your 2GHz Opteron is about the same speed - I suspect you're also at least\n> partly limited by disk, our numbers seem to match pretty well).\n\nYup. This is _very_ diskbound.\n\n> 200k commits at 6k commits per hour is about a day and a half (plus the\n> occasional packing load). Taking that long to import a CVS archive is\n> horrible. But I guess it _is_ several years of work, and I guess you\n> really have to do it only once, but still.\n\nAnd it's a huge CVS archive too.\n\n\n\nmartin\n"}]}