{"thread":{"id":"9784","subject":"Significant performance waste in git-svn and friends","startedAt":"2007-09-05T18:47:10Z","lastAt":"2007-09-07T06:28:17Z","messageCount":11,"participants":["Mike Hommey","Junio C Hamano","David Kastrup","Patrick Doyle","Shawn O. Pearce","Eric Wong","Steven Grimm"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"52609","messageId":"20070905184710.GA3632@glandium.org","threadId":"9784","inReplyTo":null,"subject":"Significant performance waste in git-svn and friends","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-09-05T18:47:10Z","receivedAt":"2007-09-05T18:47:10Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Hi,\n\nBeing a pervert abusing the way subversion doesn't deal with branches\nand tags, I'm actually not a user of git-svn or git-svnimport, because\nthey just can't deal easily with my perversion. So I'm writing a script\nto do the conversion for me, and since I also like to learn new things\nwhen I'm coding, I'm writing it in ruby.\n\nAnyways, one of the things I'm trying to convert is my svk repository\nfor debian packaging of xulrunner (so, a significant subset of the\nmozilla tree), which doesn't involve a lot of revisions (around 280,\nbecause I only imported releases or CVS snapshots), but involves a lot\nof files (roughly 20k).\n\nThe first thing I noticed when twisting around the svk repo so that\ngit-svn could somehow import it a while ago, is that running git-svn\nwas in my case significantly slower than svnadmin dump | svnadmin load\n(more than 2 times slower).\n\nAnd now, with my own script, I got the same kind of \"slowdown\". So I\ninvestigated it, and it didn't take long to realize that replacing\ngit-hash-object by a simple reimplementation in ruby was *way* faster.\ngit-hash-object being more than probably what you do the most when you\nimport a remote repository, it is not much of a surprise that forking\nthousands of times is a huge performance waste.\n\nSo, just for the record, I did a lame hack of git-svn to see what kind\nof speedup could happen in git-svn. You can find this lame hack as a\npatch below. I did some tests (with a 1.5.2.1 release) and here are the\nresults, importing only the trunk (192 revisions), with no checkout, and\nredirecting stdout to /dev/null:\n\noriginal git-svn:\nreal 25m1.871s\nuser 8m51.593s\nsys 12m31.659s\n\npatched git-svn:\nreal 14m45.870s\nuser 7m31.928s\nsys 4m1.047s\n\nSome notes about the patch:\n- I've not looked at the rest of the code to see if there was a way to\n get the size of the file so that SHA-1 sum and compression could be\n done in one pass and without copying the whole file in memory.\n- The object creation in the .git/objects directory is not as safe as\n what git-hash-object does.\n\nSome notes about git-svn:\n- A few lines above the patched zone, the file is already read once to\n do the MD5 sum. It should be possible to do SHA-1, MD5 sums and\n deflate in just one pass.\n- It might be worth testing if git-cat-file is called a lot. If so,\n implementing a simple git-cat-file equivalent that would work for\n unpacked objects could improve speed.\n\nThe same things obviously apply to git-cvsimport and other scripts\ncalling git-hash-object a lot.\n\nMike\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex d3c8cd0..202c228 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2417,6 +2417,8 @@ use warnings;\n use Carp qw/croak/;\n use IO::File qw//;\n use Digest::MD5;\n+use Digest::SHA1;\n+use Compress::Zlib;\n \n # file baton members: path, mode_a, mode_b, pool, fh, blob, base\n sub new {\n@@ -2603,15 +2605,26 @@ sub close_file {\n \t\t\t$buf eq 'link ' or die \"$path has mode 120000\",\n \t\t\t \"but is not a link\\n\";\n \t\t}\n-\t\tdefined(my $pid = open my $out,'-|') or die \"Can't fork: $!\\n\";\n-\t\tif (!$pid) {\n-\t\t\topen STDIN, '<&', $fh or croak $!;\n-\t\t\texec qw/git-hash-object -w --stdin/ or croak $!;\n+\t\tmy $size = 0;\n+\t\tmy $buf = \"\";\n+\t\twhile (my $read = sysread $fh, my $tmp, 4096) {\n+\t\t\t$size += $read;\n+\t\t\t$buf .= $tmp;\n \t\t}\n-\t\tchomp($hash = do { local $/; <$out> });\n-\t\tclose $out or croak $!;\n+\t\tmy $sha1 = Digest::SHA1->new;\n+\t\t$sha1->add(\"blob $size\\0\");\n+\t\t$sha1->add($buf);\n+\t\t$hash = $sha1->hexdigest;\n \t\tclose $fh or croak $!;\n \t\t$hash =~ /^[a-f\\d]{40}$/ or die \"not a sha1: $hash\\n\";\n+\t\tmy $blob_dir = \"$ENV{GIT_DIR}/objects/\" . substr($hash, 0, 2);\n+\t\tmy $blob_file = $blob_dir . \"/\" . substr($hash, 2);\n+\t\tif (! -f $blob_file) {\n+\t\t\tmkdir $blob_dir unless -d $blob_dir;\n+\t\t\topen BLOB, \">$blob_file\";\n+\t\t\tprint BLOB compress(\"blob $size\\0\" . $buf);\n+\t\t\tclose BLOB;\n+\t\t}\n \t\tclose $fb->{base} or croak $!;\n \t} else {\n \t\t$hash = $fb->{blob} or die \"no blob information\\n\";\n"},{"id":"52632","messageId":"7vd4wwj16d.fsf@gitster.siamese.dyndns.org","threadId":"9784","inReplyTo":"20070905184710.GA3632@glandium.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-05T20:40:42Z","receivedAt":"2007-09-05T20:40:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n> The same things obviously apply to git-cvsimport and other scripts\n> calling git-hash-object a lot.\n\nI *obviously* hate this patch, as it makes this Porcelain\ncommand to be aware of the internal representation too much.\n\nI wonder if letting fast-import handle the object creation is an\noption, though.\n"},{"id":"52637","messageId":"85bqcghktl.fsf@lola.goethe.zz","threadId":"9784","inReplyTo":"7vd4wwj16d.fsf@gitster.siamese.dyndns.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-05T21:19:18Z","receivedAt":"2007-09-05T21:19:18Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> Mike Hommey writes:\n>\n>> The same things obviously apply to git-cvsimport and other scripts\n>> calling git-hash-object a lot.\n>\n> I *obviously* hate this patch, as it makes this Porcelain\n> command to be aware of the internal representation too much.\n>\n> I wonder if letting fast-import handle the object creation is an\n> option, though.\n\nI think it would be saner to give git-hash-object an operation mode\nthat makes it usable as a pipe-controlled daemon, so that one needs\nnot fork and exec for interning another object. That way, porcelain\ncommands could keep one bidirectional pipe (feed object type and\nsource and whether to use -w into git-hash-project, receive object id)\nto git-hash-object around until they finish.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"52664","messageId":"e2a1d0aa0709051807h53c95419r3fcb29affb92fdd2@mail.gmail.com","threadId":"9784","inReplyTo":"85bqcghktl.fsf@lola.goethe.zz","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Patrick Doyle","fromEmail":"wpdster@gmail.com","sentAt":"2007-09-06T01:07:10Z","receivedAt":"2007-09-06T01:07:10Z","isPatch":false,"sender":{"key":"wpdster@gmail.com","avatar":null},"body":"I'll add my hearty vote of support, and perhaps even a patch or two,\nto anything that would make git-svn faster. Just last week, I gave up\nwaiting for it to complete the import of a (rather large) uClinux\ndistribution, falling back on SVN. I'm looking forward to trying it\nagain with Mike's patch, or perhaps even implementing and trying out\nDavid's idea.\n\nHmmm.... I'm supposed to be on a couple of planes for a rather longish\nperiod of time next week. Perhaps I'll give it a shot. (But don't\ncount on much -- the laptop battery is pretty shot nowadays and, given\nthe nature of the rest of the participitants on this list, it'll\nprobably be implemented and tested by the time I pack up for the\ntrip.)\n\n--wpd\n"},{"id":"52670","messageId":"20070906021659.GE18160@spearce.org","threadId":"9784","inReplyTo":"7vd4wwj16d.fsf@gitster.siamese.dyndns.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-06T02:16:59Z","receivedAt":"2007-09-06T02:16:59Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Mike Hommey writes:\n> \n> > The same things obviously apply to git-cvsimport and other scripts\n> > calling git-hash-object a lot.\n> \n> I *obviously* hate this patch, as it makes this Porcelain\n> command to be aware of the internal representation too much.\n\nMe too. I think its bad enough that fast-import knows the\ninternal representation of a packfile. And its plumbing!\n \n> I wonder if letting fast-import handle the object creation is an\n> option, though.\n\nI agree. This is *exactly* what fast-import was built for. It\nreally would be the better tool to use here. Much better than\nreinventing the wheel. Its already been invented twice for you\nin Git (normal object creation path and fast-import path). No\nneed to do it a third time.\n\n-- \nShawn.\n"},{"id":"52671","messageId":"20070906021941.GF18160@spearce.org","threadId":"9784","inReplyTo":"85bqcghktl.fsf@lola.goethe.zz","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-06T02:19:41Z","receivedAt":"2007-09-06T02:19:41Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"David Kastrup wrote:\n> Junio C Hamano writes:\n> \n> > Mike Hommey writes:\n> >\n> >> The same things obviously apply to git-cvsimport and other scripts\n> >> calling git-hash-object a lot.\n> >\n> > I wonder if letting fast-import handle the object creation is an\n> > option, though.\n> \n> I think it would be saner to give git-hash-object an operation mode\n> that makes it usable as a pipe-controlled daemon, so that one needs\n> not fork and exec for interning another object. That way, porcelain\n> commands could keep one bidirectional pipe (feed object type and\n> source and whether to use -w into git-hash-project, receive object id)\n> to git-hash-object around until they finish.\n\nAside from getting the hashes back from fast-import, that's what\nfast-import is for. I could also make it disable writing. Hmm.\nJunio and I were just talking about making fast-import send the\nmarks table back out on stdout. This would make it easier for a\nfrontend process to stream a whole bunch of objects into the process,\nthen get back all of their SHA-1s. Less context switches and more\nparallel operation.\n\n-- \nShawn.\n"},{"id":"52693","messageId":"20070906055211.GC5741@glandium.org","threadId":"9784","inReplyTo":"7vd4wwj16d.fsf@gitster.siamese.dyndns.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-09-06T05:52:11Z","receivedAt":"2007-09-06T05:52:11Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Wed, Sep 05, 2007 at 01:40:42PM -0700, Junio C Hamano wrote:\n> Mike Hommey writes:\n> \n> > The same things obviously apply to git-cvsimport and other scripts\n> > calling git-hash-object a lot.\n> \n> I *obviously* hate this patch, as it makes this Porcelain\n> command to be aware of the internal representation too much.\n\nThe patch was not supposed to be applied. I said it was lame ;)\nIt was more of a proof of concept.\n\nAnyways, thinking a bit more about it, I was wondering if it wouldn't be\na good idea to have Git.pm have a \"native\" implementation (by native I\nmean a .so module) for low-level plumbing tools such as hash-object,\ncat-file and such.\n\nObviously, reinventing the wheel is not good, so this native\nimplementation would be using a \"git library\" API, such as what has\nbeen done under SoC (though I don't know if this API exposes low-level\nplumbing functions)\n\n> I wonder if letting fast-import handle the object creation is an\n> option, though.\n\nIt could, probably. The reason I didn't use it is that it was way\nquicker to hack a 10 lines patch to create the blobs by hand than it\nwould have been to fork a fast-import object at the correct place during\ngit-svn initialization and piping to it at the appropriate times.\n\nMy goal was only to check how faster this would make it not to fork a\ngit-hash-object per blob.\n\nMike\n"},{"id":"52706","messageId":"20070906070407.GA19624@soma","threadId":"9784","inReplyTo":"20070905184710.GA3632@glandium.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-09-06T07:04:08Z","receivedAt":"2007-09-06T07:04:08Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Mike Hommey wrote:\n> Hi,\n\nHi Mike,\n\n> Being a pervert abusing the way subversion doesn't deal with branches\n> and tags, I'm actually not a user of git-svn or git-svnimport, because\n> they just can't deal easily with my perversion. So I'm writing a script\n> to do the conversion for me, and since I also like to learn new things\n> when I'm coding, I'm writing it in ruby.\n> \n> Anyways, one of the things I'm trying to convert is my svk repository\n> for debian packaging of xulrunner (so, a significant subset of the\n> mozilla tree), which doesn't involve a lot of revisions (around 280,\n> because I only imported releases or CVS snapshots), but involves a lot\n> of files (roughly 20k).\n> \n> The first thing I noticed when twisting around the svk repo so that\n> git-svn could somehow import it a while ago, is that running git-svn\n> was in my case significantly slower than svnadmin dump | svnadmin load\n> (more than 2 times slower).\n> \n> And now, with my own script, I got the same kind of \"slowdown\". So I\n> investigated it, and it didn't take long to realize that replacing\n> git-hash-object by a simple reimplementation in ruby was *way* faster.\n> git-hash-object being more than probably what you do the most when you\n> import a remote repository, it is not much of a surprise that forking\n> thousands of times is a huge performance waste.\n\nI haven't looked at the times in a while, but I suspect that exec()\nis the (much bigger) culprit.\n\nSince I usually import off remote repositories, so I notice network\nlatency way before I notice local performance problems with git-svn.\n\n> So, just for the record, I did a lame hack of git-svn to see what kind\n> of speedup could happen in git-svn. You can find this lame hack as a\n> patch below. I did some tests (with a 1.5.2.1 release) and here are the\n> results, importing only the trunk (192 revisions), with no checkout, and\n> redirecting stdout to /dev/null:\n> \n> original git-svn:\n> real 25m1.871s\n> user 8m51.593s\n> sys 12m31.659s\n> \n> patched git-svn:\n> real 14m45.870s\n> user 7m31.928s\n> sys 4m1.047s\n\nThat's awesome.\n\n> - It might be worth testing if git-cat-file is called a lot. If so,\n> implementing a simple git-cat-file equivalent that would work for\n> unpacked objects could improve speed.\n\nIIRC git-cat-file is called a lot. Every modified file needs the\noriginal cat-ed to make use of the delta.\n\n> The same things obviously apply to git-cvsimport and other scripts\n> calling git-hash-object a lot.\n\nMaking git-svn use fast-import would be very nice. I've got a bunch\nof other git-svn things that I need to work on, but having git-svn\nconverted to use fast-import would be nice. Or allowing Git.pm\nto access more of the git internals...\n\nHowever, how well/poorly would fast-import work for incremental\nfetches throughout the day?\n\n-- \nEric Wong\n"},{"id":"52847","messageId":"20070907045557.GY18160@spearce.org","threadId":"9784","inReplyTo":"20070906070407.GA19624@soma","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-07T04:55:57Z","receivedAt":"2007-09-07T04:55:57Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eric Wong wrote:\n> Making git-svn use fast-import would be very nice. I've got a bunch\n> of other git-svn things that I need to work on, but having git-svn\n> converted to use fast-import would be nice. Or allowing Git.pm\n> to access more of the git internals...\n> \n> However, how well/poorly would fast-import work for incremental\n> fetches throughout the day?\n\nIt would work just fine. git-p4 uses fast-import and runs it in\nincremental mode all of the time.\n\nWhat will happen is you will get a packfile per git-svn fetch, so\nif you fetch 5 times that day you will get 5 packfiles that day.\nBut you could also get 5 packfiles from git-fetch if each of those\nfetches brought in 100 or more new objects. So it really is not\nthat big of a deal.\n\nAt some point the number of packfiles gets out of control, but so\ndoes the number of loose objects. Repacking is the obvious fix in\nboth cases.\n\n-- \nShawn.\n"},{"id":"52851","messageId":"20070907054121.GA18421@glandium.org","threadId":"9784","inReplyTo":"20070905184710.GA3632@glandium.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-09-07T05:41:21Z","receivedAt":"2007-09-07T05:41:21Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Wed, Sep 05, 2007 at 08:47:10PM +0200, Mike Hommey wrote:\n> Some notes about git-svn:\n> - A few lines above the patched zone, the file is already read once to\n> do the MD5 sum. It should be possible to do SHA-1, MD5 sums and\n> deflate in just one pass.\n\nFWIW, I tried, and it doesn't make it much quicker to do everything in one pass.\n99% of the speedup comes from not using git-hash-object.\n\nMike\n"},{"id":"52856","messageId":"46E0EF81.6060403@midwinter.com","threadId":"9784","inReplyTo":"20070907045557.GY18160@spearce.org","subject":"Re: Significant performance waste in git-svn and friends","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-09-07T06:28:17Z","receivedAt":"2007-09-07T06:28:17Z","isPatch":false,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Shawn O. Pearce wrote:\n> What will happen is you will get a packfile per git-svn fetch, so\n> if you fetch 5 times that day you will get 5 packfiles that day.\n> But you could also get 5 packfiles from git-fetch if each of those\n> fetches brought in 100 or more new objects. So it really is not\n> that big of a deal.\n>\n> At some point the number of packfiles gets out of control, but so\n> does the number of loose objects. Repacking is the obvious fix in\n> both cases.\n\nThis dovetails nicely with the \"git gc --auto\" work in progress, \nparticularly the recent patch to pack small packfiles together when \nthere get to be too many of them; run that at the end of every git-svn \nfetch and the problem goes away.\n\n-Steve\n"}]}