git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [BUG-ish] diff compaction heuristic false positive

From
Stefan Beller <sbeller@google.com>
Date
Jun 10, 2016, 16:25 UTC
Message-ID
<CAGZ79kZLT8AfmWTrrW+a-v7aXw5sm68P2H=vT7QZr2hj4Z2gDA@mail.gmail.com>
In-Reply-To
<xmqqvb1hf35y.fsf@gitster.mtv.corp.google.com>
On Fri, Jun 10, 2016 at 8:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> On Fri, Jun 10, 2016 at 03:50:43AM -0400, Jeff King wrote:
>>
>>> I found a false positive with the new compaction heuristic in v2.9:
>>> [...]
>>
>> And by the way, this is less "hey neat, I found a case" and more "wow,
>> this is a lot worse than I thought".
>>
>> I diffed the old and new output for the top 10,000 commits in this
>> particular ruby code base. There were 45 commits with changed diffs.
>> Spot-checking them manually, a little over 1/3 of them featured this bad
>> pattern. The others looked like strict improvements.
>>
>> That's a lot worse than the outcomes we saw on other code bases earlier.
>> 1/3 bad is still a net improvement, so I dunno. Is this worth worrying
>> about? Should we bring back the documentation for the knob to disable
>> it? Should we consider making it tunable via gitattributes?
>>
>> I don't think that last one really helps; the good cases _and_ the bad
>> ones are both in ruby code (though certainly the C code we looked at
>> earlier was all good).
>>
>> It may also be possible to make it Just Work by using extra information
>> like indentation. I haven't thought hard enough about that to say.
>>
>> -Peff
>
> I recall saying "we'd end up being better in some and worse in
> others" at the very beginning.  How about toggling the default back
> for the upcoming release, keeping the experimentation knob in the
> code, and try different heuristics like the "indentation" during the
> next cycle?

Sure. I thought about for a while now and by now I agree with Junio. No matter what kind of heuristic we can come up with it is easy to construct a counter example.

That said, let's try the indentation thing, though I suspect one of the early motivating examples (an excerpt from a kernel config file) would not do well with it, as it had not an indentation scheme as programming languages do.

Thanks, Stefan

Previous: Junio C HamanoNext: Jacob Keller
Message 4 of 16 in “[BUG-ish] diff compaction heuristic false positive”
  1. Jeff KingJun 10, 2016
  2. Jeff KingJun 10, 2016
  3. Junio C HamanoJun 10, 2016
  4. Stefan BellerJun 10, 2016
  5. Jacob KellerJun 10, 2016
  6. Re* [BUG-ish] diff compaction heuristic false positiveJunio C Hamano, Jun 10, 2016
  7. Stefan BellerJun 10, 2016
  8. Jeff KingJun 10, 2016
  9. diff: disable compaction heuristic for nowJunio C Hamano, Jun 10, 2016
  10. Jeff KingJun 10, 2016
  11. Junio C HamanoJun 10, 2016
  12. Jeff KingJun 10, 2016
  13. Junio C HamanoJun 10, 2016
  14. Michael HaggertyJun 10, 2016
  15. Jeff KingJun 10, 2016
  16. Michael HaggertyJun 10, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.