@matheusbsbs / matheus-bagliano / commits / 52d14e166d

diff --color-moved-ws=allow-indentation-change: simplify and optimize

If we already have a block of potentially moved lines then as we move down the diff we need to check if the next line of each potentially moved line matches the current line of the diff. The implementation of --color-moved-ws=allow-indentation-change was needlessly performing this check on all the lines in the diff that matched the current line rather than just the current line. To exacerbate the problem finding all the other lines in the diff that match the current line involves a fuzzy lookup so we were wasting even more time performing a second comparison to filter out the non-matching lines. Fixing this reduces time to run git diff --color-moved-ws=allow-indentation-change v2.28.0 v2.29.0 by 93% compared to master and simplifies the code. Test HEAD^ HEAD --------------------------------------------------------------------------------------------------------------- 4002.1: diff --no-color-moved --no-color-moved-ws large change 0.38 (0.35+0.03) 0.38(0.35+0.03) +0.0% 4002.2: diff --color-moved --no-color-moved-ws large change 0.86 (0.80+0.06) 0.87(0.83+0.04) +1.2% 4002.3: diff --color-moved-ws=allow-indentation-change large change 19.01(18.93+0.06) 0.97(0.92+0.04) -94.9% 4002.4: log --no-color-moved --no-color-moved-ws 1.16 (1.06+0.09) 1.17(1.06+0.10) +0.9% 4002.5: log --color-moved --no-color-moved-ws 1.32 (1.25+0.07) 1.32(1.24+0.08) +0.0% 4002.6: log --color-moved-ws=allow-indentation-change 1.71 (1.64+0.06) 1.36(1.25+0.10) -20.5% Test master HEAD --------------------------------------------------------------------------------------------------------------- 4002.1: diff --no-color-moved --no-color-moved-ws large change 0.38 (0.33+0.05) 0.38(0.35+0.03) +0.0% 4002.2: diff --color-moved --no-color-moved-ws large change 0.80 (0.75+0.04) 0.87(0.83+0.04) +8.7% 4002.3: diff --color-moved-ws=allow-indentation-change large change 14.20(14.15+0.05) 0.97(0.92+0.04) -93.2% 4002.4: log --no-color-moved --no-color-moved-ws 1.15 (1.05+0.09) 1.17(1.06+0.10) +1.7% 4002.5: log --color-moved --no-color-moved-ws 1.30 (1.19+0.11) 1.32(1.24+0.08) +1.5% 4002.6: log --color-moved-ws=allow-indentation-change 1.70 (1.63+0.06) 1.36(1.25+0.10) -20.0% Helped-by: Jeff King <peff@peff.net> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Dec 9, 2021 at 10:30 UTC 52d14e166d7360b17f74ae169a83e5e642892bdb
1 file changed +20 -50
diff.c
+20 -50
@@ -879,37 +879,21 @@ static int compute_ws_delta(const struct emitted_diff_symbol *a,
879 return 1;
880 }
881
882 -static int cmp_in_block_with_wsd(const struct diff_options *o,
883 - const struct moved_entry *cur,
884 - const struct moved_entry *match,
885 - struct moved_block *pmb,
886 - int n)
887 -{
888 - struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
889 - int al = cur->es->len, bl = match->es->len, cl = l->len;
882 +static int cmp_in_block_with_wsd(const struct moved_entry *cur,
883 + const struct emitted_diff_symbol *l,
884 + struct moved_block *pmb)
885 +{
886 + int al = cur->es->len, bl = l->len;
887 const char *a = cur->es->line,
891 - *b = match->es->line,
892 - *c = l->line;
888 + *b = l->line;
889 int a_off = cur->es->indent_off,
890 a_width = cur->es->indent_width,
895 - c_off = l->indent_off,
896 - c_width = l->indent_width;
891 + b_off = l->indent_off,
892 + b_width = l->indent_width;
893 int delta;
894
899 - /*
900 - * We need to check if 'cur' is equal to 'match'. As those
901 - * are from the same (+/-) side, we do not need to adjust for
902 - * indent changes. However these were found using fuzzy
903 - * matching so we do have to check if they are equal. Here we
904 - * just check the lengths. We delay calling memcmp() to check
905 - * the contents until later as if the length comparison for a
906 - * and c fails we can avoid the call all together.
907 - */
908 - if (al != bl)
909 - return 1;
910 -
895 /* If 'l' and 'cur' are both blank then they match. */
912 - if (a_width == INDENT_BLANKLINE && c_width == INDENT_BLANKLINE)
896 + if (a_width == INDENT_BLANKLINE && b_width == INDENT_BLANKLINE)
897 return 0;
898
899 /*
@@ -918,7 +902,7 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
902 * match those of the current block and that the text of 'l' and 'cur'
903 * after the indentation match.
904 */
921 - delta = c_width - a_width;
905 + delta = b_width - a_width;
906
907 /*
908 * If the previous lines of this block were all blank then set its
@@ -927,9 +911,8 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
911 if (pmb->wsd == INDENT_BLANKLINE)
912 pmb->wsd = delta;
913
930 - return !(delta == pmb->wsd && al - a_off == cl - c_off &&
931 - !memcmp(a, b, al) && !
932 - memcmp(a + a_off, c + c_off, al - a_off));
914 + return !(delta == pmb->wsd && al - a_off == bl - b_off &&
915 + !memcmp(a + a_off, b + b_off, al - a_off));
916 }
917
918 static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
@@ -1030,36 +1013,23 @@ static void pmb_advance_or_null(struct diff_options *o,
1013 }
1014
1015 static void pmb_advance_or_null_multi_match(struct diff_options *o,
1033 - struct moved_entry *match,
1034 - struct hashmap *hm,
1016 + struct emitted_diff_symbol *l,
1017 struct moved_block *pmb,
1036 - int pmb_nr, int n)
1018 + int pmb_nr)
1019 {
1020 int i;
1039 - char *got_match = xcalloc(1, pmb_nr);
1040 -
1041 - hashmap_for_each_entry_from(hm, match, ent) {
1042 - for (i = 0; i < pmb_nr; i++) {
1043 - struct moved_entry *prev = pmb[i].match;
1044 - struct moved_entry *cur = (prev && prev->next_line) ?
1045 - prev->next_line : NULL;
1046 - if (!cur)
1047 - continue;
1048 - if (!cmp_in_block_with_wsd(o, cur, match, &pmb[i], n))
1049 - got_match[i] |= 1;
1050 - }
1051 - }
1021
1022 for (i = 0; i < pmb_nr; i++) {
1054 - if (got_match[i]) {
1023 + struct moved_entry *prev = pmb[i].match;
1024 + struct moved_entry *cur = (prev && prev->next_line) ?
1025 + prev->next_line : NULL;
1026 + if (cur && !cmp_in_block_with_wsd(cur, l, &pmb[i])) {
1027 /* Advance to the next line */
1056 - pmb[i].match = pmb[i].match->next_line;
1028 + pmb[i].match = cur;
1029 } else {
1030 moved_block_clear(&pmb[i]);
1031 }
1032 }
1061 -
1062 - free(got_match);
1033 }
1034
1035 static int shrink_potential_moved_blocks(struct moved_block *pmb,
@@ -1223,7 +1193,7 @@ static void mark_color_as_moved(struct diff_options *o,
1193
1194 if (o->color_moved_ws_handling &
1195 COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)
1226 - pmb_advance_or_null_multi_match(o, match, hm, pmb, pmb_nr, n);
1196 + pmb_advance_or_null_multi_match(o, l, pmb, pmb_nr);
1197 else
1198 pmb_advance_or_null(o, match, hm, pmb, pmb_nr);
1199