{"thread":{"id":"36258","subject":"[PATCH] Make XDF_NEED_MINIMAL default in blame.","startedAt":"2014-03-20T20:18:58Z","lastAt":"2014-03-20T21:21:04Z","messageCount":3,"participants":["Michael Andreen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"237204","messageId":"6555655.XSJ9EnW4BY@mako","threadId":"36258","inReplyTo":null,"subject":"[PATCH] Make XDF_NEED_MINIMAL default in blame.","fromName":"Michael Andreen","fromEmail":"harv@ruin.nu","sentAt":"2014-03-20T20:18:58Z","receivedAt":"2014-03-20T20:18:58Z","isPatch":true,"sender":{"key":"harv@ruin.nu","avatar":null},"body":"Currently git blame has a big problem finding copies and moves when you\nsplit up a big file into smaller ones. One example in the git repository\nis 2cf565c, which split the documentation into smaller files.\n\nIn 582aa00 XDF_NEED_MINIMAL was removed as the default for performance\nreasons, mainly for diff and rebase, but blame was also changed.\n\nIn 059a500 the problem with blame was noticed and the flag --minimal was\nintroduced. However this flag is not documented and it is not possible\nto set when using \"git gui blame\".\n\nSetting XDF_NEED_MINIMAL as default has a small performance impact when\nyou run on a file with few modifications. However, if you run it on a\nfile with a bigger number of modifications, the performance impact is\nsmall enough to not be noticable.\n\nThe previous behavior can still be activated with --no-minimal.\n\n((2cf565c...))$ time PAGER=cat git blame -C -M\n    Documentation/git-ls-files.txt > /dev/null\n\nreal    0m0.003s\nuser    0m0.002s\nsys 0m0.000s\n\n((2cf565c...))$ time PAGER=cat git blame --minimal -C -M\n    Documentation/git-ls-files.txt > /dev/null\n\nreal    0m0.010s\nuser    0m0.009s\nsys 0m0.000s\n\n((2cf565c...))$ time PAGER=cat git blame -C -C -C -M\n    Documentation/git-ls-files.txt > /dev/null\n\nreal    0m0.010s\nuser    0m0.010s\nsys 0m0.000s\n\n((2cf565c...))$ time PAGER=cat git blame --minimal -C -C -C -M\n    Documentation/git-ls-files.txt > /dev/null\n\nreal    0m0.028s\nuser    0m0.027s\nsys 0m0.000s\n\n(master)$ time PAGER=cat git blame -C -C -C -M\n    Documentation/git-ls-files.txt > /dev/null\n\nreal    0m2.338s\nuser    0m2.283s\nsys 0m0.056s\n\n(master)$ time PAGER=cat git blame --minimal -C -C -C -M\n    Documentation/git-ls-files.txt > /dev/null\n\nreal    0m2.355s\nuser    0m2.285s\nsys 0m0.069s\n\n(master)$ time PAGER=cat git blame -C -M cache.h > /dev/null\n\nreal    0m1.755s\nuser    0m1.730s\nsys 0m0.024s\n\n(master)$ time PAGER=cat git blame --minimal -C -M cache.h > /dev/null\n\nreal    0m1.785s\nuser    0m1.770s\nsys 0m0.014s\n\n(master)$ time PAGER=cat git blame -C -C -C -M cache.h > /dev/null\n\nreal    0m31.515s\nuser    0m30.810s\nsys 0m0.684s\n\n(master)$ time PAGER=cat git blame --minimal -C -C -C -M cache.h >\n/dev/null\n\nreal    0m31.504s\nuser    0m30.885s\nsys 0m0.598s\n\nSigned-off-by: Michael Andreen <harv@ruin.nu>\n---\nThere hasn't been any arguments against this patch. Just updated the message \nwith a note about --no-minimal.\n\nApplies cleanly on both master and maint.\n\n builtin/blame.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex e5b5d71..0e7ebd0 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -42,7 +42,7 @@ static int show_root;\n static int reverse;\n static int blank_boundary;\n static int incremental;\n-static int xdl_opts;\n+static int xdl_opts = XDF_NEED_MINIMAL;\n static int abbrev = -1;\n static int no_whole_file_rename;\n \n-- \n1.8.3.2\n"},{"id":"237205","messageId":"xmqq8us4y4ym.fsf@gitster.dls.corp.google.com","threadId":"36258","inReplyTo":"6555655.XSJ9EnW4BY@mako","subject":"Re: [PATCH] Make XDF_NEED_MINIMAL default in blame.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-20T20:45:21Z","receivedAt":"2014-03-20T20:45:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Andreen <harv@ruin.nu> writes:\n\n> There hasn't been any arguments against this patch. Just updated the message \n> with a note about --no-minimal.\n\nThere hasn't been any argument for this patch, either.\n\nIt is not like we are still in year 2007; timing result in a small\nproject like Git itself is not a good enough argument to change a\nwell established default at this late in the game, especially when\nthere are ways like command line options for users to specify their\npreferred settings.\n"},{"id":"237209","messageId":"3088608.Qolv0cJMME@river","threadId":"36258","inReplyTo":"xmqq8us4y4ym.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Make XDF_NEED_MINIMAL default in blame.","fromName":"Michael Andreen","fromEmail":"harv@ruin.nu","sentAt":"2014-03-20T21:21:04Z","receivedAt":"2014-03-20T21:21:04Z","isPatch":true,"sender":{"key":"harv@ruin.nu","avatar":null},"body":"On Thursday, March 20, 2014 01:45:21 PM Junio C Hamano wrote:\n> There hasn't been any argument for this patch, either.\n> \n> It is not like we are still in year 2007; timing result in a small\n> project like Git itself is not a good enough argument to change a\n> well established default at this late in the game, especially when\n> there are ways like command line options for users to specify their\n> preferred settings.\n\nThe main reason why I submitted it is because the current behavior is very \nunintuitive when you split big files into smaller ones. Like the one that was \ndone in 2cf565c.\n\nIf you do:\n\n $ git checkout 2cf565c\n $ git blame -C -C -C -M Documentation/git-ls-files.txt\n\nthen blame will not be able to detect any moves, even though big chunks have \nbeen moved without whitespace changes. Worse is that if you do\n\n $ git gui blame Documentation/git-ls-files.txt\n\nthen that won't see any movies either, and there is no way to make git gui \nblame use --minimal.\n\nI spent a lot of time at $work trying to code-review a big split like this, \nwondering why \"git gui blame\" and \"git blame -C -C -C -M\" couldn't detect \nmoves when complete functions had been moved with no changes (not even \nwhitespace). So by making it default it will hopefully help someone else from \nlosing time investigating this.\n\nIt wasn't until I did a bisect on git, finding 582aa00 and later googling for \nXDF_NEED_MINIMAL to find 059a500, that I found out about --minimal. Which is \nnot documented anywhere.\n\nI guess it can be argued for just documenting --minimal and either patching \n\"git gui blame\" to use it, or make it configurable. However I think the \ncurrent default is really confusing, and the reason for changing it back in \n2007 was for performance, so that was what I tried to focus on, hopefully \nshowing that there aren't any noticeable differences.\n\n/Michael\n"}]}