@matheusbsbs / matheus-bagliano / commits / 25e61909e9

diff --color-moved-ws=allow-indentation-change: improve hash lookups

As libxdiff does not have a whitespace flag to ignore the indentation the code for --color-moved-ws=allow-indentation-change uses XDF_IGNORE_WHITESPACE and then filters out any hash lookups where there are non-indentation changes. This filtering is inefficient as we have to perform another string comparison. By using the offset data that we have already computed to skip the indentation we can avoid using XDF_IGNORE_WHITESPACE and safely remove the extra checks which improves the performance by 11% and paves the way for the elimination of string comparisons in the next commit. This change slightly increases the run time of other --color-moved modes. This could be avoided by using different comparison functions for the different modes but after the next two commits there is no measurable benefit in doing so. There is a change in behavior for lines that begin with a form-feed or vertical-tab character. Since b46054b374 ("xdiff: use git-compat-util", 2019-04-11) xdiff does not treat '\f' or '\v' as whitespace characters. This means that lines starting with those characters are never considered to be blank and never match a line that does not start with the same character. After this patch a line matching "^[\f\v\r]*[ \t]*$" is considered to be blank by --color-moved-ws=allow-indentation-change and lines beginning "^[\f\v\r]*[ \t]*" can match another line if the suffixes match. This changes the output of git show for d18f76dccf ("compat/regex: use the regex engine from gawk for compat", 2010-08-17) as some lines in the pre-image before a moved block that contain '\f' are now considered moved as well as they match a blank line before the moved lines in the post-image. This commit updates one of the tests to reflect this change. Test HEAD^ HEAD -------------------------------------------------------------------------------------------------------------- 4002.1: diff --no-color-moved --no-color-moved-ws large change 0.38(0.33+0.05) 0.38(0.33+0.05) +0.0% 4002.2: diff --color-moved --no-color-moved-ws large change 0.86(0.82+0.04) 0.88(0.84+0.04) +2.3% 4002.3: diff --color-moved-ws=allow-indentation-change large change 0.97(0.94+0.03) 0.86(0.81+0.05) -11.3% 4002.4: log --no-color-moved --no-color-moved-ws 1.16(1.07+0.09) 1.16(1.06+0.09) +0.0% 4002.5: log --color-moved --no-color-moved-ws 1.32(1.26+0.06) 1.33(1.27+0.05) +0.8% 4002.6: log --color-moved-ws=allow-indentation-change 1.35(1.29+0.06) 1.33(1.24+0.08) -1.5% 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 25e61909e95ffd376787ce0ecc5dd6cf7d3f0e78

2 files changed +30 -57

diff.c

+19 -46

@@ -850,28 +850,15 @@ static void fill_es_indent_data(struct emitted_diff_symbol *es)

850 }

851

852 static int compute_ws_delta(const struct emitted_diff_symbol *a,

853 - const struct emitted_diff_symbol *b,

854 - int *out)

855 -{

856 - int a_len = a->len,

857 - b_len = b->len,

858 - a_off = a->indent_off,

859 - a_width = a->indent_width,

860 - b_off = b->indent_off,

853 + const struct emitted_diff_symbol *b)

854 +{

855 + int a_width = a->indent_width,

856 b_width = b->indent_width;

857

863 - if (a_width == INDENT_BLANKLINE && b_width == INDENT_BLANKLINE) {

864 - *out = INDENT_BLANKLINE;

865 - return 1;

866 - }

867 -

868 - if (a_len - a_off != b_len - b_off ||

869 - memcmp(a->line + a_off, b->line + b_off, a_len - a_off))

870 - return 0;

871 -

872 - *out = a_width - b_width;

858 + if (a_width == INDENT_BLANKLINE && b_width == INDENT_BLANKLINE)

859 + return INDENT_BLANKLINE;

860

874 - return 1;

861 + return a_width - b_width;

862 }

863

864 static int cmp_in_block_with_wsd(const struct moved_entry *cur,

@@ -916,26 +903,17 @@ static int moved_entry_cmp(const void *hashmap_cmp_fn_data,

903 const void *keydata)

904 {

905 const struct diff_options *diffopt = hashmap_cmp_fn_data;

919 - const struct moved_entry *a, *b;

906 + const struct emitted_diff_symbol *a, *b;

907 unsigned flags = diffopt->color_moved_ws_handling

908 & XDF_WHITESPACE_FLAGS;

909

923 - a = container_of(eptr, const struct moved_entry, ent);

924 - b = container_of(entry_or_key, const struct moved_entry, ent);

910 + a = container_of(eptr, const struct moved_entry, ent)->es;

911 + b = container_of(entry_or_key, const struct moved_entry, ent)->es;

912

926 - if (diffopt->color_moved_ws_handling &

927 - COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)

928 - /*

929 - * As there is not specific white space config given,

930 - * we'd need to check for a new block, so ignore all

931 - * white space. The setup of the white space

932 - * configuration for the next block is done else where

933 - */

934 - flags |= XDF_IGNORE_WHITESPACE;

935 -

936 - return !xdiff_compare_lines(a->es->line, a->es->len,

937 - b->es->line, b->es->len,

938 - flags);

913 + return !xdiff_compare_lines(a->line + a->indent_off,

914 + a->len - a->indent_off,

915 + b->line + b->indent_off,

916 + b->len - b->indent_off, flags);

917 }

918

919 static struct moved_entry *prepare_entry(struct diff_options *o,

@@ -944,7 +922,8 @@ static struct moved_entry *prepare_entry(struct diff_options *o,

922 struct moved_entry *ret = xmalloc(sizeof(*ret));

923 struct emitted_diff_symbol *l = &o->emitted_symbols->buf[line_no];

924 unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS;

947 - unsigned int hash = xdiff_hash_string(l->line, l->len, flags);

925 + unsigned int hash = xdiff_hash_string(l->line + l->indent_off,

926 + l->len - l->indent_off, flags);

927

928 hashmap_entry_init(&ret->ent, hash);

929 ret->es = l;

@@ -1036,13 +1015,11 @@ static void fill_potential_moved_blocks(struct diff_options *o,

1015 hashmap_for_each_entry_from(hm, match, ent) {

1016 ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc);

1017 if (o->color_moved_ws_handling &

1039 - COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) {

1040 - if (compute_ws_delta(l, match->es, &(pmb[pmb_nr]).wsd))

1041 - pmb[pmb_nr++].match = match;

1042 - } else {

1018 + COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)

1019 + pmb[pmb_nr].wsd = compute_ws_delta(l, match->es);

1020 + else

1021 pmb[pmb_nr].wsd = 0;

1044 - pmb[pmb_nr++].match = match;

1045 - }

1022 + pmb[pmb_nr++].match = match;

1023 }

1024

1025 *pmb_p = pmb;

@@ -6276,10 +6253,6 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)

6253 if (o->color_moved) {

6254 struct hashmap add_lines, del_lines;

6255

6279 - if (o->color_moved_ws_handling &

6280 - COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)

6281 - o->color_moved_ws_handling |= XDF_IGNORE_WHITESPACE;

6282 -

6256 hashmap_init(&del_lines, moved_entry_cmp, o, 0);

6257 hashmap_init(&add_lines, moved_entry_cmp, o, 0);

6258

t/t4015-diff-whitespace.sh

+11 -11

@@ -2206,10 +2206,10 @@ EMPTY=''

2206 test_expect_success 'compare mixed whitespace delta across moved blocks' '

2207

2208 git reset --hard &&

2209 - tr Q_ "\t " <<-EOF >text.txt &&

2210 - ${EMPTY}

2211 - ____too short without

2212 - ${EMPTY}

2209 + tr "^|Q_" "\f\v\t " <<-EOF >text.txt &&

2210 + ^__

2211 + |____too short without

2212 + ^

2213 ___being grouped across blank line

2214 ${EMPTY}

2215 context

@@ -2228,7 +2228,7 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '

2228 git add text.txt &&

2229 git commit -m "add text.txt" &&

2230

2231 - tr Q_ "\t " <<-EOF >text.txt &&

2231 + tr "^|Q_" "\f\v\t " <<-EOF >text.txt &&

2232 context

2233 lines

2234 to

@@ -2239,7 +2239,7 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '

2239 ${EMPTY}

2240 QQtoo short without

2241 ${EMPTY}

2242 - Q_______being grouped across blank line

2242 + ^Q_______being grouped across blank line

2243 ${EMPTY}

2244 Q_QThese two lines have had their

2245 indentation reduced by four spaces

@@ -2251,16 +2251,16 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '

2251 -c core.whitespace=space-before-tab \

2252 diff --color --color-moved --ws-error-highlight=all \

2253 --color-moved-ws=allow-indentation-change >actual.raw &&

2254 - grep -v "index" actual.raw | test_decode_color >actual &&

2254 + grep -v "index" actual.raw | tr "\f\v" "^|" | test_decode_color >actual &&

2255

2256 cat <<-\EOF >expected &&

2257 <BOLD>diff --git a/text.txt b/text.txt<RESET>

2258 <BOLD>--- a/text.txt<RESET>

2259 <BOLD>+++ b/text.txt<RESET>

2260 <CYAN>@@ -1,16 +1,16 @@<RESET>

2261 - <BOLD;MAGENTA>-<RESET>

2262 - <BOLD;MAGENTA>-<RESET><BOLD;MAGENTA> too short without<RESET>

2263 - <BOLD;MAGENTA>-<RESET>

2261 + <BOLD;MAGENTA>-<RESET><BOLD;MAGENTA>^<RESET><BRED> <RESET>

2262 + <BOLD;MAGENTA>-<RESET><BOLD;MAGENTA>| too short without<RESET>

2263 + <BOLD;MAGENTA>-<RESET><BOLD;MAGENTA>^<RESET>

2264 <BOLD;MAGENTA>-<RESET><BOLD;MAGENTA> being grouped across blank line<RESET>

2265 <BOLD;MAGENTA>-<RESET>

2266 <RESET>context<RESET>

@@ -2280,7 +2280,7 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '

2280 <BOLD;YELLOW>+<RESET>

2281 <BOLD;YELLOW>+<RESET> <BOLD;YELLOW>too short without<RESET>

2282 <BOLD;YELLOW>+<RESET>

2283 - <BOLD;YELLOW>+<RESET> <BOLD;YELLOW> being grouped across blank line<RESET>

2283 + <BOLD;YELLOW>+<RESET><BOLD;YELLOW>^ being grouped across blank line<RESET>

2284 <BOLD;YELLOW>+<RESET>

2285 <BOLD;CYAN>+<RESET> <BRED> <RESET> <BOLD;CYAN>These two lines have had their<RESET>

2286 <BOLD;CYAN>+<RESET><BOLD;CYAN>indentation reduced by four spaces<RESET>