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

[PATCH v2 7/9] diff --color-moved-ws: optimize allow-indentation-change

From
PWPhillip Wood <phillip.wood@talktalk.net>
Date
Nov 23, 2018, 11:16 UTC
Message-ID
<20181123111658.30342-8-phillip.wood@talktalk.net>
In-Reply-To
<20181123111658.30342-1-phillip.wood@talktalk.net>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
When running
  git diff --color-moved-ws=allow-indentation-change v2.18.0 v2.19.0

cmp_in_block_with_wsd() is called 694908327 times. Of those 42.7% return after comparing a and b. By comparing the lengths first we can return early in all but 0.03% of those cases without dereferencing the string pointers. The comparison between a and c fails in 6.8% of calls, by comparing the lengths first we reject all the failing calls without dereferencing the string pointers.

This reduces the time to run the command above by by 42% from 14.6s to 8.5s. This is still much slower than the normal --color-moved which takes ~0.6-0.7s to run but is a significant improvement.

The next commits will replace the current implementation with one that works with mixed tabs and spaces in the indentation. I think it is worth optimizing the current implementation first to enable a fair comparison between the two implementations.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 diff.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/diff.c b/diff.c
index 8c08dd68df..c378ce3daf 100644
--- a/diff.c
+++ b/diff.c
@@ -829,20 +829,23 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
 				 int n)
 {
 	struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
-	int al = cur->es->len, cl = l->len;
+	int al = cur->es->len, bl = match->es->len, cl = l->len;
 	const char *a = cur->es->line,
 		   *b = match->es->line,
 		   *c = l->line;
-
+	const char *orig_a = a;
 	int wslen;
 
 	/*
-	 * We need to check if 'cur' is equal to 'match'.
-	 * As those are from the same (+/-) side, we do not need to adjust for
-	 * indent changes. However these were found using fuzzy matching
-	 * so we do have to check if they are equal.
+	 * We need to check if 'cur' is equal to 'match'.  As those
+	 * are from the same (+/-) side, we do not need to adjust for
+	 * indent changes. However these were found using fuzzy
+	 * matching so we do have to check if they are equal. Here we
+	 * just check the lengths. We delay calling memcmp() to check
+	 * the contents until later as if the length comparison for a
+	 * and c fails we can avoid the call all together.
 	 */
-	if (strcmp(a, b))
+	if (al != bl)
 		return 1;
 
 	if (!pmb->wsd.string)
@@ -870,7 +873,7 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
 		al -= wslen;
 	}
 
-	if (al != cl || memcmp(a, c, al))
+	if (al != cl || memcmp(orig_a, b, bl) || memcmp(a, c, al))
 		return 1;
 
 	return 0;
-- 
2.19.1.1690.g258b440b18
Previous: Phillip WoodNext: Phillip Wood
Message 34 of 44 in “diff --color-moved-ws: allow mixed spaces and tabs in indentation change”
  1. 0/3 diff --color-moved-ws: allow mixed spaces and tabs in indentation changePhillip Wood, Sep 24, 2018
  2. 1/3 xdiff-interface: make xdl_blankline() availablePhillip Wood, Sep 24, 2018
  3. Stefan BellerSep 24, 2018
  4. 2/3 diff.c: remove unused variablesPhillip Wood, Sep 24, 2018
  5. 3/3 diff: add --color-moved-ws=allow-mixed-indentation-changePhillip Wood, Sep 24, 2018
  6. Stefan BellerSep 25, 2018
  7. 3/3 diff: add --color-moved-ws=allow-mixed-indentation-changePhillip Wood, Oct 9, 2018
  8. Stefan BellerOct 9, 2018
  9. Phillip WoodOct 10, 2018
  10. Stefan BellerOct 10, 2018
  11. Phillip WoodSep 24, 2018
  12. 0/9 diff --color-moved-ws fixes and enhancmentPhillip Wood, Nov 16, 2018
  13. 1/9 diff: document --no-color-movedPhillip Wood, Nov 16, 2018
  14. 7/9 diff --color-moved-ws: optimize allow-indentation-changePhillip Wood, Nov 16, 2018
  15. Stefan BellerNov 16, 2018
  16. Phillip WoodNov 17, 2018
  17. 4/9 diff --color-moved-ws: demonstrate false positivesPhillip Wood, Nov 16, 2018
  18. 8/9 diff --color-moved-ws: modify allow-indentation-changePhillip Wood, Nov 16, 2018
  19. Stefan BellerNov 16, 2018
  20. Phillip WoodNov 17, 2018
  21. 6/9 diff --color-moved=zebra: be stricter with color alternationPhillip Wood, Nov 16, 2018
  22. 9/9 diff --color-moved-ws: handle blank linesPhillip Wood, Nov 16, 2018
  23. Stefan BellerNov 20, 2018
  24. Phillip WoodNov 21, 2018
  25. 5/9 diff --color-moved-ws: fix false positivesPhillip Wood, Nov 16, 2018
  26. 3/9 diff: allow --no-color-moved-wsPhillip Wood, Nov 16, 2018
  27. 2/9 diff: use whitespace consistentlyPhillip Wood, Nov 16, 2018
  28. Stefan BellerNov 16, 2018
  29. 0/9 diff --color-moved-ws fixes and enhancmentPhillip Wood, Nov 23, 2018
  30. 1/9 diff: document --no-color-movedPhillip Wood, Nov 23, 2018
  31. 5/9 diff --color-moved-ws: fix false positivesPhillip Wood, Nov 23, 2018
  32. 4/9 diff --color-moved-ws: demonstrate false positivesPhillip Wood, Nov 23, 2018
  33. 6/9 diff --color-moved=zebra: be stricter with color alternationPhillip Wood, Nov 23, 2018
  34. 7/9 diff --color-moved-ws: optimize allow-indentation-changePhillip Wood, Nov 23, 2018
  35. 8/9 diff --color-moved-ws: modify allow-indentation-changePhillip Wood, Nov 23, 2018
  36. 9/9 diff --color-moved-ws: handle blank linesPhillip Wood, Nov 23, 2018
  37. 3/9 diff: allow --no-color-moved-wsPhillip Wood, Nov 23, 2018
  38. 2/9 Use "whitespace" consistentlyPhillip Wood, Nov 23, 2018
  39. Stefan BellerNov 26, 2018
  40. Phillip WoodNov 27, 2018
  41. Phillip WoodJan 8, 2019
  42. Junio C HamanoJan 8, 2019
  43. Stefan BellerJan 10, 2019
  44. Junio C HamanoJan 10, 2019

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.