| Message ID | c3e5dce1910b3d640757e0845d646c3b040a8e28.1637056179.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | diff --color-moved[-ws] speedups | expand |
Hi Phillip, On Tue, 16 Nov 2021, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > This change will allow us to easily combine pmb_advance_or_null() and > pmb_advance_or_null_multi_match() in the next commit. Calling > xdiff_compare_lines() directly rather than using a function pointer > from the hash map has little effect on the run time. Good. I verified that the function `moved_entry_cmp()` (https://github.com/gitgitgadget/git/blob/c3e5dce191/diff.c#L918-L944) calls `xdiff_compare_lines()`, and it is this function that is used for both `del_lines` and `add_lines` which would have been passed as `hm`: https://github.com/gitgitgadget/git/blob/c3e5dce191/diff.c#L6339-L6340 > > Test HEAD^ HEAD > ------------------------------------------------------------------------------------------------------------- > 4002.1: diff --no-color-moved --no-color-moved-ws large change 0.38(0.35+0.03) 0.38(0.32+0.06) +0.0% > 4002.2: diff --color-moved --no-color-moved-ws large change 0.87(0.83+0.04) 0.87(0.80+0.06) +0.0% > 4002.3: diff --color-moved-ws=allow-indentation-change large change 0.97(0.92+0.04) 0.97(0.93+0.04) +0.0% > 4002.4: log --no-color-moved --no-color-moved-ws 1.17(1.06+0.10) 1.16(1.10+0.05) -0.9% > 4002.5: log --color-moved --no-color-moved-ws 1.32(1.24+0.08) 1.31(1.22+0.09) -0.8% > 4002.6: log --color-moved-ws=allow-indentation-change 1.36(1.25+0.10) 1.35(1.25+0.10) -0.7% Honestly, I would have expected an improvement, given that `moved_entry_cmp()` has to do a few things before it can call `xdiff_compare_lines()`. I love your attention to detail, providing performance numbers in the commit message to prove that it at least has no negative impact on the speed. Thanks, Dscho > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > diff.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/diff.c b/diff.c > index 78a486021ab..22e0edac173 100644 > --- a/diff.c > +++ b/diff.c > @@ -994,17 +994,20 @@ static void add_lines_to_move_detection(struct diff_options *o, > } > > static void pmb_advance_or_null(struct diff_options *o, > - struct moved_entry *match, > - struct hashmap *hm, > + struct emitted_diff_symbol *l, > struct moved_block *pmb, > int pmb_nr) > { > int i; > + unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS; > + > for (i = 0; i < pmb_nr; i++) { > struct moved_entry *prev = pmb[i].match; > struct moved_entry *cur = (prev && prev->next_line) ? > prev->next_line : NULL; > - if (cur && !hm->cmpfn(o, &cur->ent, &match->ent, NULL)) { > + if (cur && xdiff_compare_lines(cur->es->line, cur->es->len, > + l->line, l->len, > + flags)) { > pmb[i].match = cur; > } else { > pmb[i].match = NULL; > @@ -1195,7 +1198,7 @@ static void mark_color_as_moved(struct diff_options *o, > COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) > pmb_advance_or_null_multi_match(o, l, pmb, pmb_nr); > else > - pmb_advance_or_null(o, match, hm, pmb, pmb_nr); > + pmb_advance_or_null(o, l, pmb, pmb_nr); > > pmb_nr = shrink_potential_moved_blocks(pmb, pmb_nr); > > -- > gitgitgadget > >
diff --git a/diff.c b/diff.c index 78a486021ab..22e0edac173 100644 --- a/diff.c +++ b/diff.c @@ -994,17 +994,20 @@ static void add_lines_to_move_detection(struct diff_options *o, } static void pmb_advance_or_null(struct diff_options *o, - struct moved_entry *match, - struct hashmap *hm, + struct emitted_diff_symbol *l, struct moved_block *pmb, int pmb_nr) { int i; + unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS; + for (i = 0; i < pmb_nr; i++) { struct moved_entry *prev = pmb[i].match; struct moved_entry *cur = (prev && prev->next_line) ? prev->next_line : NULL; - if (cur && !hm->cmpfn(o, &cur->ent, &match->ent, NULL)) { + if (cur && xdiff_compare_lines(cur->es->line, cur->es->len, + l->line, l->len, + flags)) { pmb[i].match = cur; } else { pmb[i].match = NULL; @@ -1195,7 +1198,7 @@ static void mark_color_as_moved(struct diff_options *o, COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) pmb_advance_or_null_multi_match(o, l, pmb, pmb_nr); else - pmb_advance_or_null(o, match, hm, pmb, pmb_nr); + pmb_advance_or_null(o, l, pmb, pmb_nr); pmb_nr = shrink_potential_moved_blocks(pmb, pmb_nr);