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

Re: [PATCH 3/4] diff-highlight: match up lines before highlighting

From
JLJonathan Lebon <jonathan.lebon@gmail.com>
Date
Nov 17, 2015, 05:18 UTC
Message-ID
<CAM0PgJ4erahEZsL75c4mytF158xxeE6itG8KOMMi4n-zBTHcww@mail.gmail.com>
In-Reply-To
<20151103220314.GE22484@sigill.intra.peff.net>
On Tue, Nov 3, 2015 at 5:03 PM, Jeff King <peff@peff.net> wrote:
Show 11 quoted lines
> Your is _much_ slower. I get:
>
>   real    0m25.538s
>   user    0m25.420s
>   sys     0m0.120s
>
> for the old versus:
>
>   real    2m3.580s
>   user    2m3.548s
>   sys     0m0.156s

Thanks for investigating and trying it out. I got the same results here as well.

> for your series. In an interactive setting, the latency may not be that
> noticeable, but if you are digging far into history (e.g., "git log -p",
> then using "/" in less to search for a commit or some test), I suspect
> it would be very noticeable.
Agreed.
> I was thinking there was some low-hanging fruit in memoizing the
> calculations, but even the prefix/suffix computation is pairwise. I'm
> not really sure how to make this much faster.

I gave memoization a try to see if it could improve the situation. I also lowered maxhunksize to 10. Doing `git log -p` on git.git went from 2m31 to 2m11. So I think it would require a whole other approach overall.

> As for the output itself, the diff between the two looks promising. The
> first several cases I looked at ar strict improvements. Some of them are
> kind of weird, especially in English text.

Yes, I'm very happy with the improvements and run with these patches all the time for now.

> In the other thread I mentioned earlier, the solution I cooked up was
> dropping highlighting entirely for hunks over a certain percentage of
> highlighting. I wonder if we could do something similar here (e.g.,
> don't match lines where more than 50% of the line would be highlighted).

I looked over but haven't tested the patches in the other thread yet. But overall, the LCS definitely looks promising. I'm hoping to find some time to have a more serious go at it and maybe pick it up where you left off.

>
> -Peff

Thanks again for reviewing these patches and apologies for the delayed reply.

Jonathan
Previous: Jeff KingNext: Jeff King
Message 13 of 17 in “diff-highlight: make a few improvements”
  1. 0/4 diff-highlight: make a few improvementsJonathan Lebon, Nov 3, 2015
  2. 1/4 diff-highlight: add `less -r` to cmd in READMEJonathan Lebon, Nov 3, 2015
  3. Junio C HamanoNov 3, 2015
  4. Jonathan LebonNov 3, 2015
  5. Jeff KingNov 3, 2015
  6. Junio C HamanoNov 3, 2015
  7. Jeff KingNov 3, 2015
  8. 2/4 diff-highlight: factor out prefix/suffix functionsJonathan Lebon, Nov 3, 2015
  9. Jeff KingNov 3, 2015
  10. 3/4 diff-highlight: match up lines before highlightingJonathan Lebon, Nov 3, 2015
  11. Jeff KingNov 3, 2015
  12. Jeff KingNov 3, 2015
  13. Jonathan LebonNov 17, 2015
  14. Jeff KingNov 17, 2015
  15. 4/4 diff-highlight: add maxhunksize config optionJonathan Lebon, Nov 3, 2015
  16. Jeff KingNov 3, 2015
  17. Jeff KingNov 3, 2015

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.