{"thread":{"id":"1525","subject":"gitweb - option to disable rename detection","startedAt":"2005-08-15T18:31:18Z","lastAt":"2005-08-16T23:14:23Z","messageCount":6,"participants":["Yasushi SHOJI","Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"7306","messageId":"87d5ofoxvt.wl@mail2.atmark-techno.com","threadId":"1525","inReplyTo":null,"subject":"gitweb - option to disable rename detection","fromName":"Yasushi SHOJI","fromEmail":"yashi@atmark-techno.com","sentAt":"2005-08-15T18:31:18Z","receivedAt":"2005-08-15T18:31:18Z","isPatch":false,"sender":{"key":"yashi@atmark-techno.com","avatar":"https://gravatar.com/avatar/4817e8703ac4379935834d87453faa9d0c94b9dc19d83fcc54c67875eb133e59?d=mp&s=160"},"body":"Hi all,\n\nIt seems to me that git-diff-tree needs huge memory if you try to diff\non big change with rename detection enabled.\n\nThis isn't problem for sane project but if you create a repo with only\nmajor releases imports, git-diff-tree run by git_commit() eats system\nmemory and die ;P\n\nso here is a proposal patch to add an option to disable rename\ndetection on selected project.\n\n# please bear with me! I _know_ my perl code sucks.  this is just to\n# show my idea.\n\nregards,\n--\n         yashi\n\ndiff --git a/gitweb.cgi b/gitweb.cgi\n--- a/gitweb.cgi\n+++ b/gitweb.cgi\n@@ -40,6 +40,11 @@ my $home_text =              \"indextext.html\";\n #my $projects_list = $projectroot;\n my $projects_list = \"index/index.aux\";\n \n+# rename detection on big change require a lot of memory. If your tree\n+# has big change commit, list those project names to disable rename\n+# detection. most sane project doesn't need to disable this feature.\n+my $projects_disable_rename_detection = \"\";\n+\n # input validation and dispatch\n my $action = $cgi->param('a');\n if (defined $action) {\n@@ -1572,6 +1577,17 @@ sub git_log {\n        git_footer_html();\n }\n \n+sub git_diff_tree_options {\n+       my $found = 0;\n+       my @list = split(/ /, $projects_disable_rename_detection);\n+       foreach my $p (@list) {\n+               if ($project eq $p) {\n+                       $found = 1;\n+               }\n+       }\n+       $found ? \"\" : \"-M\";\n+}\n+\n sub git_commit {\n        my %co = git_read_commit($hash);\n        if (!%co) {\n@@ -1587,7 +1603,8 @@ sub git_commit {\n                $root = \" --root\";\n                $parent = \"\";\n        }\n-       open my $fd, \"-|\", \"$gitbin/git-diff-tree -r -M $root $parent $hash\" or die_error(undef, \"Open failed.\");\n+       my $opts = git_diff_tree_options();\n+       open my $fd, \"-|\", \"$gitbin/git-diff-tree -r $opts $root $parent $hash\" or die_error(undef, \"Open failed.\");\n        @difftree = map { chomp; $_ } <$fd>;\n        close $fd or die_error(undef, \"Reading diff-tree failed.\");\n        git_header_html();\n"},{"id":"7307","messageId":"Pine.LNX.4.58.0508151140050.3553@g5.osdl.org","threadId":"1525","inReplyTo":"87d5ofoxvt.wl@mail2.atmark-techno.com","subject":"Re: gitweb - option to disable rename detection","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-15T18:53:20Z","receivedAt":"2005-08-15T18:53:20Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 16 Aug 2005, Yasushi SHOJI wrote:\n> \n> It seems to me that git-diff-tree needs huge memory if you try to diff\n> on big change with rename detection enabled.\n> \n> This isn't problem for sane project but if you create a repo with only\n> major releases imports, git-diff-tree run by git_commit() eats system\n> memory and die ;P\n\nInstead of disabling it entirely, how about just having some limit on it?\n\nThe basic rename detection works very well for \"normal\" changes as you \npoint out, but it very fundamentally is O(n^2) in number of files created \nand deleted, so we could instead just limit it and say \"if we have tons of \nnew/deleted files, disable it in the interest of CPU/memory usage\".\n\n\t\t\tLinus\n"},{"id":"7314","messageId":"87br3zorjk.wl@mail2.atmark-techno.com","threadId":"1525","inReplyTo":"Pine.LNX.4.58.0508151140050.3553@g5.osdl.org","subject":"Re: gitweb - option to disable rename detection","fromName":"Yasushi SHOJI","fromEmail":"yashi@atmark-techno.com","sentAt":"2005-08-15T20:48:15Z","receivedAt":"2005-08-15T20:48:15Z","isPatch":false,"sender":{"key":"yashi@atmark-techno.com","avatar":"https://gravatar.com/avatar/4817e8703ac4379935834d87453faa9d0c94b9dc19d83fcc54c67875eb133e59?d=mp&s=160"},"body":"At Mon, 15 Aug 2005 11:53:20 -0700 (PDT),\nLinus Torvalds wrote:\n> \n> On Tue, 16 Aug 2005, Yasushi SHOJI wrote:\n> > \n> > It seems to me that git-diff-tree needs huge memory if you try to diff\n> > on big change with rename detection enabled.\n> > \n> > This isn't problem for sane project but if you create a repo with only\n> > major releases imports, git-diff-tree run by git_commit() eats system\n> > memory and die ;P\n> \n> Instead of disabling it entirely, how about just having some limit on it?\n\nah, that's a good idea.  here is a quick and dirty patch.\n--\n          yashi\n\ndiff --git a/gitweb.cgi b/gitweb.cgi\n--- a/gitweb.cgi\n+++ b/gitweb.cgi\n@@ -40,6 +40,9 @@ my $home_text =               \"indextext.html\";\n #my $projects_list = $projectroot;\n my $projects_list = \"index/index.aux\";\n \n+# max number of changes to use rename detection on git-diff-tree\n+my $rename_detection_threshold = 1000;\n+\n # input validation and dispatch\n my $action = $cgi->param('a');\n if (defined $action) {\n@@ -1587,7 +1590,18 @@ sub git_commit {\n                $root = \" --root\";\n                $parent = \"\";\n        }\n-       open my $fd, \"-|\", \"$gitbin/git-diff-tree -r -M $root $parent $hash\" or die_error(undef, \"Open failed.\");\n+       my $nr_files;\n+       my $opts = \"\";\n+       my $disabled_notice;\n+       open my $fd, \"-|\", \"$gitbin/git-diff-tree -r $root $parent $hash\" or die_error(undef, \"Open failed.\");\n+       $nr_files++ while <$fd>;\n+       close $fd or die_error(undef, \"Counting diff-tree failed.\");\n+       if ($nr_files <  $rename_detection_threshold) {\n+               $opts .= \" -M\";\n+       } else {\n+               $disabled_notice = \"(Rename detection disabled)\";\n+       }\n+       open my $fd, \"-|\", \"$gitbin/git-diff-tree -r $opts $root $parent $hash\" or die_error(undef, \"Open failed.\");\n        @difftree = map { chomp; $_ } <$fd>;\n        close $fd or die_error(undef, \"Reading diff-tree failed.\");\n        git_header_html();\n@@ -1671,7 +1685,7 @@ sub git_commit {\n        print \"</div>\\n\";\n        print \"<div class=\\\"list_head\\\">\\n\";\n        if ($#difftree > 10) {\n-               print(($#difftree + 1) . \" files changed:\\n\");\n+               print(($#difftree + 1) . \" files changed\" . $disabled_notice  . \":\\n\");\n        }\n        print \"</div>\\n\";\n        print \"<table cellspacing=\\\"0\\\">\\n\";\n"},{"id":"7319","messageId":"Pine.LNX.4.58.0508151542290.3553@g5.osdl.org","threadId":"1525","inReplyTo":"87br3zorjk.wl@mail2.atmark-techno.com","subject":"Re: gitweb - option to disable rename detection","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-15T22:43:26Z","receivedAt":"2005-08-15T22:43:26Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 16 Aug 2005, Yasushi SHOJI wrote:\n>\n> > Instead of disabling it entirely, how about just having some limit on it?\n> \n> ah, that's a good idea.  here is a quick and dirty patch.\n\nThis makes it somewhat more expensive - I was thinking about disabling it \nin git-diff-tree, since the rename logic already knows how many \nnew/deleted files there are.\n\n\t\tLinus\n"},{"id":"7336","messageId":"7voe7yr812.fsf@assigned-by-dhcp.cox.net","threadId":"1525","inReplyTo":"Pine.LNX.4.58.0508151542290.3553@g5.osdl.org","subject":"Re: gitweb - option to disable rename detection","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-16T01:21:29Z","receivedAt":"2005-08-16T01:21:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> This makes it somewhat more expensive - I was thinking about disabling it \n> in git-diff-tree, since the rename logic already knows how many \n> new/deleted files there are.\n\nThat's doable.  I'll rig something up on my next GIT day, along\nwith the clean-up for diff option handling which we have\npostponed for some time.  Clearly this would introduce another\noption to diffcore.\n\nOTOH, you could take totally the opposite avenue and take\nadvantage of the fact that git commit ancestry is immultable.\nOnce commitdiff is made, you do not have to regenerate it but\ncache it and reuse for the next request.  I may be mistaken, but\nI vaguely recall kernel.org webservers already does something\nlike that.\n\nIf you have infinite amount of disk space, you could choose to\ncache everything, and also prime the cache before any real users\nask for a page.  Once you go that route, you could even give -C\nflag for copy detection, with --find-copies-harder which is an\nultra expensive option, and nobody would notice.\n"},{"id":"7398","messageId":"871x4txynk.wl@mail2.atmark-techno.com","threadId":"1525","inReplyTo":"Pine.LNX.4.58.0508151542290.3553@g5.osdl.org","subject":"Re: gitweb - option to disable rename detection","fromName":"Yasushi SHOJI","fromEmail":"yashi@atmark-techno.com","sentAt":"2005-08-16T23:14:23Z","receivedAt":"2005-08-16T23:14:23Z","isPatch":false,"sender":{"key":"yashi@atmark-techno.com","avatar":"https://gravatar.com/avatar/4817e8703ac4379935834d87453faa9d0c94b9dc19d83fcc54c67875eb133e59?d=mp&s=160"},"body":"At Mon, 15 Aug 2005 15:43:26 -0700 (PDT),\nLinus Torvalds wrote:\n> \n> On Tue, 16 Aug 2005, Yasushi SHOJI wrote:\n> >\n> > > Instead of disabling it entirely, how about just having some limit on it?\n> > \n> > ah, that's a good idea.  here is a quick and dirty patch.\n> \n> This makes it somewhat more expensive.\n\nyes.  but much cheaper than OOM ;P\n\n>                                        - I was thinking about disabling it \n> in git-diff-tree, since the rename logic already knows how many \n> new/deleted files there are.\n\nI'll leave this feature for homework.  I'll work on\nprepare_temp_file() first since it seems to fix my problem.\n\nregards,\n--\n         yashi\n"}]}