Skip to content

Commit 52d14e1

Browse files
phillipwoodgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Phillip Wood <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 76e32d6 commit 52d14e1

File tree

1 file changed

+20
-50
lines changed

1 file changed

+20
-50
lines changed

diff.c

Lines changed: 20 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -879,37 +879,21 @@ static int compute_ws_delta(const struct emitted_diff_symbol *a,
879879
return 1;
880880
}
881881

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;
890887
const char *a = cur->es->line,
891-
*b = match->es->line,
892-
*c = l->line;
888+
*b = l->line;
893889
int a_off = cur->es->indent_off,
894890
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;
897893
int delta;
898894

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-
911895
/* 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)
913897
return 0;
914898

915899
/*
@@ -918,7 +902,7 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
918902
* match those of the current block and that the text of 'l' and 'cur'
919903
* after the indentation match.
920904
*/
921-
delta = c_width - a_width;
905+
delta = b_width - a_width;
922906

923907
/*
924908
* 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,
927911
if (pmb->wsd == INDENT_BLANKLINE)
928912
pmb->wsd = delta;
929913

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));
933916
}
934917

935918
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,
10301013
}
10311014

10321015
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,
10351017
struct moved_block *pmb,
1036-
int pmb_nr, int n)
1018+
int pmb_nr)
10371019
{
10381020
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-
}
10521021

10531022
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])) {
10551027
/* Advance to the next line */
1056-
pmb[i].match = pmb[i].match->next_line;
1028+
pmb[i].match = cur;
10571029
} else {
10581030
moved_block_clear(&pmb[i]);
10591031
}
10601032
}
1061-
1062-
free(got_match);
10631033
}
10641034

10651035
static int shrink_potential_moved_blocks(struct moved_block *pmb,
@@ -1223,7 +1193,7 @@ static void mark_color_as_moved(struct diff_options *o,
12231193

12241194
if (o->color_moved_ws_handling &
12251195
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);
12271197
else
12281198
pmb_advance_or_null(o, match, hm, pmb, pmb_nr);
12291199

0 commit comments

Comments
 (0)