{"thread":{"id":"17474","subject":"\"git svn fetch\" slow","startedAt":"2009-01-31T13:14:57Z","lastAt":"2009-02-05T08:02:40Z","messageCount":11,"participants":["Markus Heidelberg","Sverre Rabbelier","Eric Wong","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"102694","messageId":"200901311414.58205.markus.heidelberg@web.de","threadId":"17474","inReplyTo":null,"subject":"\"git svn fetch\" slow","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-01-31T13:14:57Z","receivedAt":"2009-01-31T13:14:57Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Hi,\n\nsince several days \"git svn fetch\" didn't seem to work any more. I\nbisected it down to\n\n commit dbc6c74d0858d77e61e092a48d467e725211f8e9\n git-svn: handle empty files marked as symlinks in SVN\n 2009-01-11\n\nIn the new function _mark_empty_symlinks() there is a loop that takes\nabout 36 seconds for me. That means each svn revision takes 36+x seconds\nfor downloading. So it still works, but I aborted it before waiting so\nmuch time, so I thought, it didn't work any more.\n\nThe loop loops over each blob (\"git ls-tree -r git-svn | wc -l\" times).\nThe project I'm using git-svn with is Buildroot and it has currently\n3074 blobs in the tree. Printing a loop counter every time the loop is\nexecuted, I can see that it mostly goes really fast, but there are\nfiles, where it needs much time then.\n\nCould there be a way to avoid this time consuming step?\n\nMarkus\n"},{"id":"102703","messageId":"bd6139dc0901310823lcced62frd61445cb80d56fca@mail.gmail.com","threadId":"17474","inReplyTo":"200901311414.58205.markus.heidelberg@web.de","subject":"Re: \"git svn fetch\" slow","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-31T16:23:34Z","receivedAt":"2009-01-31T16:23:34Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Sat, Jan 31, 2009 at 14:14, Markus Heidelberg\n wrote:\n> since several days \"git svn fetch\" didn't seem to work any more. I\n> bisected it down to\n\nI noticed it too, it's horribly slow; I can't really revert the patch\nsince it conflicts, and I'm not familiar with the code, so I don't\nknow how to resolve the conflict :(.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"102705","messageId":"200901311801.56130.markus.heidelberg@web.de","threadId":"17474","inReplyTo":"bd6139dc0901310823lcced62frd61445cb80d56fca@mail.gmail.com","subject":"Re: \"git svn fetch\" slow","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-01-31T17:01:55Z","receivedAt":"2009-01-31T17:01:55Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Sverre Rabbelier, 31.01.2009:\n> On Sat, Jan 31, 2009 at 14:14, Markus Heidelberg\n> wrote:\n> > since several days \"git svn fetch\" didn't seem to work any more. I\n> > bisected it down to\n> \n> I noticed it too, it's horribly slow; I can't really revert the patch\n> since it conflicts, and I'm not familiar with the code, so I don't\n> know how to resolve the conflict :(.\n\nThe following should work around it:\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 79888a0..bc7bd21 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3255,7 +3255,6 @@ sub new {\n \tbless $self, $class;\n \tif (exists $git_svn->{last_commit}) {\n \t\t$self->{c} = $git_svn->{last_commit};\n-\t\t$self->{empty_symlinks} = _mark_empty_symlinks($git_svn);\n \t}\n \t$self->{empty} = {};\n \t$self->{dir_prop} = {};\n"},{"id":"102706","messageId":"bd6139dc0901310931l7ff5a3del35d51560fd04b3d5@mail.gmail.com","threadId":"17474","inReplyTo":"200901311801.56130.markus.heidelberg@web.de","subject":"Re: \"git svn fetch\" slow","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-31T17:31:45Z","receivedAt":"2009-01-31T17:31:45Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Sat, Jan 31, 2009 at 18:01, Markus Heidelberg\n wrote:\n> The following should work around it:\n\nAwesome! I tested it and it does indeed work around the issue for me, thanks!\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"102744","messageId":"20090201021844.GB18855@dcvr.yhbt.net","threadId":"17474","inReplyTo":"200901311414.58205.markus.heidelberg@web.de","subject":"[PATCH] git-svn: allow disabling expensive broken symlink checks","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2009-02-01T02:18:44Z","receivedAt":"2009-02-01T02:18:44Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Since dbc6c74d0858d77e61e092a48d467e725211f8e9, git-svn has had\nan expensive check for broken symlinks that exist in some\nrepositories. This leads to a heavy performance hit on\nrepositories with many empty blobs that are not supposed to be\nsymlinks.\n\nThe workaround is enabled by default; and may be disabled via:\n\n git config svn.brokenSymlinkWorkaround false\n\nReported by Markus Heidelberg.\n\nSigned-off-by: Eric Wong \n---\n\n Markus Heidelberg wrote:\n > Hi,\n > \n > since several days \"git svn fetch\" didn't seem to work any more. I\n > bisected it down to\n > \n > commit dbc6c74d0858d77e61e092a48d467e725211f8e9\n > git-svn: handle empty files marked as symlinks in SVN\n > 2009-01-11\n > \n > In the new function _mark_empty_symlinks() there is a loop that takes\n > about 36 seconds for me. That means each svn revision takes 36+x seconds\n > for downloading. So it still works, but I aborted it before waiting so\n > much time, so I thought, it didn't work any more.\n > \n > The loop loops over each blob (\"git ls-tree -r git-svn | wc -l\" times).\n > The project I'm using git-svn with is Buildroot and it has currently\n > 3074 blobs in the tree. Printing a loop counter every time the loop is\n > executed, I can see that it mostly goes really fast, but there are\n > files, where it needs much time then.\n > \n > Could there be a way to avoid this time consuming step?\n > \n > Markus\n\n Documentation/git-svn.txt | 8 ++++++++\n git-svn.perl | 20 ++++++++++++++++++++\n t/t9131-git-svn-empty-symlink.sh | 10 ++++++++++\n 3 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\nindex 7b654f7..3d45654 100644\n--- a/Documentation/git-svn.txt\n+++ b/Documentation/git-svn.txt\n@@ -499,6 +499,14 @@ svn-remote..rewriteRoot::\n \tthe repository with a public http:// or svn:// URL in the\n \tmetadata so users of it will see the public URL.\n \n+svn.brokenSymlinkWorkaround::\n+This disables potentially expensive checks to workaround broken symlinks\n+checked into SVN by broken clients. Set this option to \"false\" if you\n+track a SVN repository with many empty blobs that are not symlinks.\n+This option may be changed while \"git-svn\" is running and take effect on\n+the next revision fetched. If unset, git-svn assumes this option to be\n+\"true\".\n+\n --\n \n Since the noMetadata, rewriteRoot, useSvnsyncProps and useSvmProps\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 79888a0..bebcbde 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3271,10 +3271,18 @@ sub new {\n # do_{switch,update}\n sub _mark_empty_symlinks {\n \tmy ($git_svn) = @_;\n+\tmy $bool = Git::config_bool('svn.brokenSymlinkWorkaround');\n+\treturn {} if (defined($bool) && ! $bool);\n+\n \tmy %ret;\n \tmy ($rev, $cmt) = $git_svn->last_rev_commit;\n \treturn {} unless ($rev && $cmt);\n \n+\t# allow the warning to be printed for each revision we fetch to\n+\t# ensure the user sees it. The user can also disable the workaround\n+\t# on the repository even while git svn is running and the next\n+\t# revision fetched will skip this expensive function.\n+\tmy $printed_warning;\n \tchomp(my $empty_blob = `git hash-object -t blob --stdin < /dev/null`);\n \tmy ($ls, $ctx) = command_output_pipe(qw/ls-tree -r -z/, $cmt);\n \tlocal $/ = \"\\0\";\n@@ -3283,6 +3291,18 @@ sub _mark_empty_symlinks {\n \twhile (<$ls>) {\n \t\tchomp;\n \t\ts/\\A100644 blob $empty_blob\\t//o or next;\n+\t\tunless ($printed_warning) {\n+\t\t\tprint STDERR \"Scanning for empty symlinks, \",\n+\t\t\t \"this may take a while if you have \",\n+\t\t\t\t \"many empty files\\n\",\n+\t\t\t\t \"You may disable this with `\",\n+\t\t\t\t \"git config svn.brokenSymlinkWorkaround \",\n+\t\t\t\t \"false'.\\n\",\n+\t\t\t\t \"This may be done in a different \",\n+\t\t\t\t \"terminal without restarting \",\n+\t\t\t\t \"git svn\\n\";\n+\t\t\t$printed_warning = 1;\n+\t\t}\n \t\tmy $path = $_;\n \t\tmy (undef, $props) =\n \t\t $git_svn->ra->get_file($pfx.$path, $rev, undef);\ndiff --git a/t/t9131-git-svn-empty-symlink.sh b/t/t9131-git-svn-empty-symlink.sh\nindex 704a4f8..c3c3e42 100755\n--- a/t/t9131-git-svn-empty-symlink.sh\n+++ b/t/t9131-git-svn-empty-symlink.sh\n@@ -87,4 +87,14 @@ test_expect_success '\"bar\" is an empty file' 'test -f x/bar && ! test -s x/bar'\n test_expect_success 'get \"bar\" => symlink fix from svn' \\\n \t\t'(cd x && git svn rebase)'\n test_expect_success '\"bar\" becomes a symlink' 'test -L x/bar'\n+\n+\n+test_expect_success 'clone using git svn' 'git svn clone -r1 \"$svnrepo\" y'\n+test_expect_success 'disable broken symlink workaround' \\\n+ '(cd y && git config svn.brokenSymlinkWorkaround false)'\n+test_expect_success '\"bar\" is an empty file' 'test -f y/bar && ! test -s y/bar'\n+test_expect_success 'get \"bar\" => symlink fix from svn' \\\n+\t\t'(cd y && git svn rebase)'\n+test_expect_success '\"bar\" becomes a symlink' '! test -L y/bar'\n+\n test_done\n-- \nEric Wong\n"},{"id":"102811","messageId":"7vbptlo7zg.fsf@gitster.siamese.dyndns.org","threadId":"17474","inReplyTo":"20090201021844.GB18855@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: allow disabling expensive broken symlink checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-02T03:03:47Z","receivedAt":"2009-02-02T03:03:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Since dbc6c74d0858d77e61e092a48d467e725211f8e9, git-svn has had\n> an expensive check for broken symlinks that exist in some\n> repositories. This leads to a heavy performance hit on\n> repositories with many empty blobs that are not supposed to be\n> symlinks.\n>\n> The workaround is enabled by default; and may be disabled via:\n>\n> git config svn.brokenSymlinkWorkaround false\n>\n> Reported by Markus Heidelberg.\n>\n> Signed-off-by: Eric Wong \n\nHow common is this breakage in people's subversion repositories that\ndbc6c74d (git-svn: handle empty files marked as symlinks in SVN,\n2009-01-11) works around?\n\nWhat's the way to recover from a broken import, when the subversion\nrepository does have such a breakage, and the user used git-svn that\npredates dbc6c74? Is it very involved, and it is much better to have the\nsafety by default than to force everybody else who interacts with\nnon-broken subversion repository suffer from this performance penalty?\n\nBecause the fix (that is broken from the performance angle) is relatively\nrecent, I am wondering if it makes more sense to turn it off by default,\nand allow people with such a broken history to optionally turn it on.\n"},{"id":"102917","messageId":"20090203044549.GA2483@dcvr.yhbt.net","threadId":"17474","inReplyTo":"7vbptlo7zg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-svn: allow disabling expensive broken symlink checks","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2009-02-03T04:45:49Z","receivedAt":"2009-02-03T04:45:49Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> > Since dbc6c74d0858d77e61e092a48d467e725211f8e9, git-svn has had\n> > an expensive check for broken symlinks that exist in some\n> > repositories. This leads to a heavy performance hit on\n> > repositories with many empty blobs that are not supposed to be\n> > symlinks.\n> >\n> > The workaround is enabled by default; and may be disabled via:\n> >\n> > git config svn.brokenSymlinkWorkaround false\n> >\n> > Reported by Markus Heidelberg.\n> >\n> > Signed-off-by: Eric Wong \n> \n> How common is this breakage in people's subversion repositories that\n> dbc6c74d (git-svn: handle empty files marked as symlinks in SVN,\n> 2009-01-11) works around?\n\nIt's not common at all. Some broken Windows clients were able to\ncreate it.\n\n> What's the way to recover from a broken import, when the subversion\n> repository does have such a breakage, and the user used git-svn that\n> predates dbc6c74? Is it very involved, and it is much better to have the\n> safety by default than to force everybody else who interacts with\n> non-broken subversion repository suffer from this performance penalty?\n\nPreviously, git-svn would just stop importing and refuse to continue.\nSo allowing the user to enable it would be a problem; too. I don't\nrecall the error being easy to distinguish from other errors.\n\n> Because the fix (that is broken from the performance angle) is relatively\n> recent, I am wondering if it makes more sense to turn it off by default,\n> and allow people with such a broken history to optionally turn it on.\n\nI'm considering disabling it by default, too. It only gets triggered if\nthere are any empty blobs at all in the repository (which are fairly\nrare, but not as rare as broken symlinks in SVN). So for a \"normal\"\nrepository it would just be the (low) overhead of calling ls-files for\nevery revision we fetch (and the hash-object writes:\n\n>> How common is this breakage in people's subversion repositories that\n>> dbc6c74d (git-svn: handle empty files marked as symlinks in SVN,\n>> 2009-01-11) works around?\n>\n> It's not common at all. Some broken Windows clients were able to\n> create it.\n>\n>> What's the way to recover from a broken import, when the subversion\n>> repository does have such a breakage, and the user used git-svn that\n>> predates dbc6c74? Is it very involved, and it is much better to have the\n>> safety by default than to force everybody else who interacts with\n>> non-broken subversion repository suffer from this performance penalty?\n>\n> Previously, git-svn would just stop importing and refuse to continue.\n> So allowing the user to enable it would be a problem; too. I don't\n> recall the error being easy to distinguish from other errors.\n>\n>> Because the fix (that is broken from the performance angle) is relatively\n>> recent, I am wondering if it makes more sense to turn it off by default,\n>> and allow people with such a broken history to optionally turn it on.\n>\n> I'm considering disabling it by default, too.\n\nI leave it entirely up to you to choose whichever default you find\nsensible (I do not think I have to say this). I wasn't complaining your\noriginal choice to stay on the safer side, with an option to trigger a\nfaster but potentially riskier behaviour.\n\nI was curious how black-and-white the deciding factor for a sensible\ndefault would be for this particular case.\n"},{"id":"103014","messageId":"20090203191021.GB2077@dcvr.yhbt.net","threadId":"17474","inReplyTo":"7v4ozcdnbt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-svn: allow disabling expensive broken symlink checks","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2009-02-03T19:10:21Z","receivedAt":"2009-02-03T19:10:21Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> >> How common is this breakage in people's subversion repositories that\n> >> dbc6c74d (git-svn: handle empty files marked as symlinks in SVN,\n> >> 2009-01-11) works around?\n> >\n> > It's not common at all. Some broken Windows clients were able to\n> > create it.\n> >\n> >> What's the way to recover from a broken import, when the subversion\n> >> repository does have such a breakage, and the user used git-svn that\n> >> predates dbc6c74? Is it very involved, and it is much better to have the\n> >> safety by default than to force everybody else who interacts with\n> >> non-broken subversion repository suffer from this performance penalty?\n> >\n> > Previously, git-svn would just stop importing and refuse to continue.\n> > So allowing the user to enable it would be a problem; too. I don't\n> > recall the error being easy to distinguish from other errors.\n> >\n> >> Because the fix (that is broken from the performance angle) is relatively\n> >> recent, I am wondering if it makes more sense to turn it off by default,\n> >> and allow people with such a broken history to optionally turn it on.\n> >\n> > I'm considering disabling it by default, too.\n> \n> I leave it entirely up to you to choose whichever default you find\n> sensible (I do not think I have to say this). I wasn't complaining your\n> original choice to stay on the safer side, with an option to trigger a\n> faster but potentially riskier behaviour.\n> \n> I was curious how black-and-white the deciding factor for a sensible\n> default would be for this particular case.\n\nIf there are no objections by the time I get home tonight I'll\ndisable the workaround by default.\n\n-- \nEric Wong\n"},{"id":"103285","messageId":"20090205074215.GA24887@dcvr.yhbt.net","threadId":"17474","inReplyTo":"20090203191021.GB2077@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: allow disabling expensive broken symlink checks","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2009-02-05T07:42:15Z","receivedAt":"2009-02-05T07:42:15Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Eric Wong wrote:\n> Junio C Hamano wrote:\n> > Eric Wong writes:\n> > \n> > >> How common is this breakage in people's subversion repositories that\n> > >> dbc6c74d (git-svn: handle empty files marked as symlinks in SVN,\n> > >> 2009-01-11) works around?\n> > >\n> > > It's not common at all. Some broken Windows clients were able to\n> > > create it.\n> > >\n> > >> What's the way to recover from a broken import, when the subversion\n> > >> repository does have such a breakage, and the user used git-svn that\n> > >> predates dbc6c74? Is it very involved, and it is much better to have the\n> > >> safety by default than to force everybody else who interacts with\n> > >> non-broken subversion repository suffer from this performance penalty?\n> > >\n> > > Previously, git-svn would just stop importing and refuse to continue.\n> > > So allowing the user to enable it would be a problem; too. I don't\n> > > recall the error being easy to distinguish from other errors.\n\nActually I was wrong on git-svn refusing to continue. git-svn will\ncreate a regular 100644 file with \"link path/to/dest\" as its contents.\ngit-svn only croaks on checksum differences with the blob itself; it\ndoes not have an easy way to verify the mode change => 120000 if it\nhappened previously.\n\n> > >> Because the fix (that is broken from the performance angle) is relatively\n> > >> recent, I am wondering if it makes more sense to turn it off by default,\n> > >> and allow people with such a broken history to optionally turn it on.\n> > >\n> > > I'm considering disabling it by default, too.\n\nI'm considering keeping it on by default given the above\n(re)discovery...\n\n-- \nEric Wong\n"},{"id":"103292","messageId":"7vljslgvkv.fsf@gitster.siamese.dyndns.org","threadId":"17474","inReplyTo":"20090205074215.GA24887@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: allow disabling expensive broken symlink checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T08:02:40Z","receivedAt":"2009-02-05T08:02:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Eric Wong wrote:\n>\n>> > > Previously, git-svn would just stop importing and refuse to continue.\n>> > > So allowing the user to enable it would be a problem; too. I don't\n>> > > recall the error being easy to distinguish from other errors.\n>\n> Actually I was wrong on git-svn refusing to continue. git-svn will\n> create a regular 100644 file with \"link path/to/dest\" as its contents.\n> git-svn only croaks on checksum differences with the blob itself; it\n> does not have an easy way to verify the mode change => 120000 if it\n> happened previously.\n\nThanks for being thorough.\n"}]}