# gitweb - option to disable rename detection 6 messages from 2005-08-15 to 2005-08-16. Participants: Yasushi SHOJI, Linus Torvalds, Junio C Hamano. Thread: https://gitlist.dev/t/1525 ## Yasushi SHOJI, 2005-08-15 18:31 Subject: gitweb - option to disable rename detection Message-ID: <87d5ofoxvt.wl@mail2.atmark-techno.com> URL: https://gitlist.dev/e/87d5ofoxvt.wl%40mail2.atmark-techno.com ``` Hi all, It seems to me that git-diff-tree needs huge memory if you try to diff on big change with rename detection enabled. This isn't problem for sane project but if you create a repo with only major releases imports, git-diff-tree run by git_commit() eats system memory and die ;P so here is a proposal patch to add an option to disable rename detection on selected project. # please bear with me! I _know_ my perl code sucks. this is just to # show my idea. regards, -- yashi diff --git a/gitweb.cgi b/gitweb.cgi --- a/gitweb.cgi +++ b/gitweb.cgi @@ -40,6 +40,11 @@ my $home_text = "indextext.html"; #my $projects_list = $projectroot; my $projects_list = "index/index.aux"; +# rename detection on big change require a lot of memory. If your tree +# has big change commit, list those project names to disable rename +# detection. most sane project doesn't need to disable this feature. +my $projects_disable_rename_detection = ""; + # input validation and dispatch my $action = $cgi->param('a'); if (defined $action) { @@ -1572,6 +1577,17 @@ sub git_log { git_footer_html(); } +sub git_diff_tree_options { + my $found = 0; + my @list = split(/ /, $projects_disable_rename_detection); + foreach my $p (@list) { + if ($project eq $p) { + $found = 1; + } + } + $found ? "" : "-M"; +} + sub git_commit { my %co = git_read_commit($hash); if (!%co) { @@ -1587,7 +1603,8 @@ sub git_commit { $root = " --root"; $parent = ""; } - open my $fd, "-|", "$gitbin/git-diff-tree -r -M $root $parent $hash" or die_error(undef, "Open failed."); + my $opts = git_diff_tree_options(); + open my $fd, "-|", "$gitbin/git-diff-tree -r $opts $root $parent $hash" or die_error(undef, "Open failed."); @difftree = map { chomp; $_ } <$fd>; close $fd or die_error(undef, "Reading diff-tree failed."); git_header_html(); ``` ## Linus Torvalds, 2005-08-15 18:53 Subject: Re: gitweb - option to disable rename detection Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0508151140050.3553%40g5.osdl.org In-Reply-To: <87d5ofoxvt.wl@mail2.atmark-techno.com> ``` On Tue, 16 Aug 2005, Yasushi SHOJI wrote: > > It seems to me that git-diff-tree needs huge memory if you try to diff > on big change with rename detection enabled. > > This isn't problem for sane project but if you create a repo with only > major releases imports, git-diff-tree run by git_commit() eats system > memory and die ;P Instead of disabling it entirely, how about just having some limit on it? The basic rename detection works very well for "normal" changes as you point out, but it very fundamentally is O(n^2) in number of files created and deleted, so we could instead just limit it and say "if we have tons of new/deleted files, disable it in the interest of CPU/memory usage". Linus ``` ## Yasushi SHOJI, 2005-08-15 20:48 Subject: Re: gitweb - option to disable rename detection Message-ID: <87br3zorjk.wl@mail2.atmark-techno.com> URL: https://gitlist.dev/e/87br3zorjk.wl%40mail2.atmark-techno.com In-Reply-To: ``` At Mon, 15 Aug 2005 11:53:20 -0700 (PDT), Linus Torvalds wrote: > > On Tue, 16 Aug 2005, Yasushi SHOJI wrote: > > > > It seems to me that git-diff-tree needs huge memory if you try to diff > > on big change with rename detection enabled. > > > > This isn't problem for sane project but if you create a repo with only > > major releases imports, git-diff-tree run by git_commit() eats system > > memory and die ;P > > Instead of disabling it entirely, how about just having some limit on it? ah, that's a good idea. here is a quick and dirty patch. -- yashi diff --git a/gitweb.cgi b/gitweb.cgi --- a/gitweb.cgi +++ b/gitweb.cgi @@ -40,6 +40,9 @@ my $home_text = "indextext.html"; #my $projects_list = $projectroot; my $projects_list = "index/index.aux"; +# max number of changes to use rename detection on git-diff-tree +my $rename_detection_threshold = 1000; + # input validation and dispatch my $action = $cgi->param('a'); if (defined $action) { @@ -1587,7 +1590,18 @@ sub git_commit { $root = " --root"; $parent = ""; } - open my $fd, "-|", "$gitbin/git-diff-tree -r -M $root $parent $hash" or die_error(undef, "Open failed."); + my $nr_files; + my $opts = ""; + my $disabled_notice; + open my $fd, "-|", "$gitbin/git-diff-tree -r $root $parent $hash" or die_error(undef, "Open failed."); + $nr_files++ while <$fd>; + close $fd or die_error(undef, "Counting diff-tree failed."); + if ($nr_files < $rename_detection_threshold) { + $opts .= " -M"; + } else { + $disabled_notice = "(Rename detection disabled)"; + } + open my $fd, "-|", "$gitbin/git-diff-tree -r $opts $root $parent $hash" or die_error(undef, "Open failed."); @difftree = map { chomp; $_ } <$fd>; close $fd or die_error(undef, "Reading diff-tree failed."); git_header_html(); @@ -1671,7 +1685,7 @@ sub git_commit { print "\n"; print "
\n"; if ($#difftree > 10) { - print(($#difftree + 1) . " files changed:\n"); + print(($#difftree + 1) . " files changed" . $disabled_notice . ":\n"); } print "
\n"; print "\n"; ``` ## Linus Torvalds, 2005-08-15 22:43 Subject: Re: gitweb - option to disable rename detection Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0508151542290.3553%40g5.osdl.org In-Reply-To: <87br3zorjk.wl@mail2.atmark-techno.com> ``` On Tue, 16 Aug 2005, Yasushi SHOJI wrote: > > > Instead of disabling it entirely, how about just having some limit on it? > > ah, that's a good idea. here is a quick and dirty patch. This makes it somewhat more expensive - I was thinking about disabling it in git-diff-tree, since the rename logic already knows how many new/deleted files there are. Linus ``` ## Junio C Hamano, 2005-08-16 01:21 Subject: Re: gitweb - option to disable rename detection Message-ID: <7voe7yr812.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7voe7yr812.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Linus Torvalds writes: > This makes it somewhat more expensive - I was thinking about disabling it > in git-diff-tree, since the rename logic already knows how many > new/deleted files there are. That's doable. I'll rig something up on my next GIT day, along with the clean-up for diff option handling which we have postponed for some time. Clearly this would introduce another option to diffcore. OTOH, you could take totally the opposite avenue and take advantage of the fact that git commit ancestry is immultable. Once commitdiff is made, you do not have to regenerate it but cache it and reuse for the next request. I may be mistaken, but I vaguely recall kernel.org webservers already does something like that. If you have infinite amount of disk space, you could choose to cache everything, and also prime the cache before any real users ask for a page. Once you go that route, you could even give -C flag for copy detection, with --find-copies-harder which is an ultra expensive option, and nobody would notice. ``` ## Yasushi SHOJI, 2005-08-16 23:14 Subject: Re: gitweb - option to disable rename detection Message-ID: <871x4txynk.wl@mail2.atmark-techno.com> URL: https://gitlist.dev/e/871x4txynk.wl%40mail2.atmark-techno.com In-Reply-To: ``` At Mon, 15 Aug 2005 15:43:26 -0700 (PDT), Linus Torvalds wrote: > > On Tue, 16 Aug 2005, Yasushi SHOJI wrote: > > > > > Instead of disabling it entirely, how about just having some limit on it? > > > > ah, that's a good idea. here is a quick and dirty patch. > > This makes it somewhat more expensive. yes. but much cheaper than OOM ;P > - I was thinking about disabling it > in git-diff-tree, since the rename logic already knows how many > new/deleted files there are. I'll leave this feature for homework. I'll work on prepare_temp_file() first since it seems to fix my problem. regards, -- yashi ```