Skip to content

Commit de67293

Browse files
gitstergitster
committed
Merge branch 'sg/line-log-tree-diff-optim'
Optimize unnecessary full-tree diff away from "git log -L" machinery. * sg/line-log-tree-diff-optim: line-log: avoid unnecessary full tree diffs line-log: extract pathspec parsing from line ranges into a helper function
2 parents 9548622 + a2bb801 commit de67293

File tree

2 files changed

+134
-19
lines changed

2 files changed

+134
-19
lines changed

‎line-log.c‎

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,38 @@ static struct line_log_data *lookup_line_range(struct rev_info *revs,
737737
return ret;
738738
}
739739

740+
static int same_paths_in_pathspec_and_range(struct pathspec *pathspec,
741+
struct line_log_data *range)
742+
{
743+
int i;
744+
struct line_log_data *r;
745+
746+
for (i = 0, r = range; i < pathspec->nr && r; i++, r = r->next)
747+
if (strcmp(pathspec->items[i].match, r->path))
748+
return 0;
749+
if (i < pathspec->nr || r)
750+
/* different number of pathspec items and ranges */
751+
return 0;
752+
753+
return 1;
754+
}
755+
756+
static void parse_pathspec_from_ranges(struct pathspec *pathspec,
757+
struct line_log_data *range)
758+
{
759+
struct line_log_data *r;
760+
struct argv_array array = ARGV_ARRAY_INIT;
761+
const char **paths;
762+
763+
for (r = range; r; r = r->next)
764+
argv_array_push(&array, r->path);
765+
paths = argv_array_detach(&array);
766+
767+
parse_pathspec(pathspec, 0, PATHSPEC_PREFER_FULL, "", paths);
768+
/* strings are now owned by pathspec */
769+
free(paths);
770+
}
771+
740772
void line_log_init(struct rev_info *rev, const char *prefix, struct string_list *args)
741773
{
742774
struct commit *commit = NULL;
@@ -746,20 +778,7 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list
746778
range = parse_lines(rev->diffopt.repo, commit, prefix, args);
747779
add_line_range(rev, commit, range);
748780

749-
if (!rev->diffopt.detect_rename) {
750-
struct line_log_data *r;
751-
struct argv_array array = ARGV_ARRAY_INIT;
752-
const char **paths;
753-
754-
for (r = range; r; r = r->next)
755-
argv_array_push(&array, r->path);
756-
paths = argv_array_detach(&array);
757-
758-
parse_pathspec(&rev->diffopt.pathspec, 0,
759-
PATHSPEC_PREFER_FULL, "", paths);
760-
/* strings are now owned by pathspec */
761-
free(paths);
762-
}
781+
parse_pathspec_from_ranges(&rev->diffopt.pathspec, range);
763782
}
764783

765784
static void move_diff_queue(struct diff_queue_struct *dst,
@@ -817,15 +836,29 @@ static void queue_diffs(struct line_log_data *range,
817836
struct diff_queue_struct *queue,
818837
struct commit *commit, struct commit *parent)
819838
{
839+
struct object_id *tree_oid, *parent_tree_oid;
840+
820841
assert(commit);
821842

843+
tree_oid = get_commit_tree_oid(commit);
844+
parent_tree_oid = parent ? get_commit_tree_oid(parent) : NULL;
845+
846+
if (opt->detect_rename &&
847+
!same_paths_in_pathspec_and_range(&opt->pathspec, range)) {
848+
clear_pathspec(&opt->pathspec);
849+
parse_pathspec_from_ranges(&opt->pathspec, range);
850+
}
822851
DIFF_QUEUE_CLEAR(&diff_queued_diff);
823-
diff_tree_oid(parent ? get_commit_tree_oid(parent) : NULL,
824-
get_commit_tree_oid(commit), "", opt);
825-
if (opt->detect_rename) {
852+
diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
853+
if (opt->detect_rename && diff_might_be_rename()) {
854+
/* must look at the full tree diff to detect renames */
855+
clear_pathspec(&opt->pathspec);
856+
DIFF_QUEUE_CLEAR(&diff_queued_diff);
857+
858+
diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
859+
826860
filter_diffs_for_paths(range, 1);
827-
if (diff_might_be_rename())
828-
diffcore_std(opt);
861+
diffcore_std(opt);
829862
filter_diffs_for_paths(range, 0);
830863
}
831864
move_diff_queue(queue, &diff_queued_diff);

‎t/t4211-line-log.sh‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,86 @@ test_expect_success '--raw is forbidden' '
132132
test_must_fail git log -L1,24:b.c --raw
133133
'
134134

135+
test_expect_success 'setup for checking fancy rename following' '
136+
git checkout --orphan moves-start &&
137+
git reset --hard &&
138+
139+
printf "%s\n" 12 13 14 15 b c d e >file-1 &&
140+
printf "%s\n" 22 23 24 25 B C D E >file-2 &&
141+
git add file-1 file-2 &&
142+
test_tick &&
143+
git commit -m "Add file-1 and file-2" &&
144+
oid_add_f1_f2=$(git rev-parse --short HEAD) &&
145+
146+
git checkout -b moves-main &&
147+
printf "%s\n" 11 12 13 14 15 b c d e >file-1 &&
148+
git commit -a -m "Modify file-1 on main" &&
149+
oid_mod_f1_main=$(git rev-parse --short HEAD) &&
150+
151+
printf "%s\n" 21 22 23 24 25 B C D E >file-2 &&
152+
git commit -a -m "Modify file-2 on main #1" &&
153+
oid_mod_f2_main_1=$(git rev-parse --short HEAD) &&
154+
155+
git mv file-1 renamed-1 &&
156+
git commit -m "Rename file-1 to renamed-1 on main" &&
157+
158+
printf "%s\n" 11 12 13 14 15 b c d e f >renamed-1 &&
159+
git commit -a -m "Modify renamed-1 on main" &&
160+
oid_mod_r1_main=$(git rev-parse --short HEAD) &&
161+
162+
printf "%s\n" 21 22 23 24 25 B C D E F >file-2 &&
163+
git commit -a -m "Modify file-2 on main #2" &&
164+
oid_mod_f2_main_2=$(git rev-parse --short HEAD) &&
165+
166+
git checkout -b moves-side moves-start &&
167+
printf "%s\n" 12 13 14 15 16 b c d e >file-1 &&
168+
git commit -a -m "Modify file-1 on side #1" &&
169+
oid_mod_f1_side_1=$(git rev-parse --short HEAD) &&
170+
171+
printf "%s\n" 22 23 24 25 26 B C D E >file-2 &&
172+
git commit -a -m "Modify file-2 on side" &&
173+
oid_mod_f2_side=$(git rev-parse --short HEAD) &&
174+
175+
git mv file-2 renamed-2 &&
176+
git commit -m "Rename file-2 to renamed-2 on side" &&
177+
178+
printf "%s\n" 12 13 14 15 16 a b c d e >file-1 &&
179+
git commit -a -m "Modify file-1 on side #2" &&
180+
oid_mod_f1_side_2=$(git rev-parse --short HEAD) &&
181+
182+
printf "%s\n" 22 23 24 25 26 A B C D E >renamed-2 &&
183+
git commit -a -m "Modify renamed-2 on side" &&
184+
oid_mod_r2_side=$(git rev-parse --short HEAD) &&
185+
186+
git checkout moves-main &&
187+
git merge moves-side &&
188+
oid_merge=$(git rev-parse --short HEAD)
189+
'
190+
191+
test_expect_success 'fancy rename following #1' '
192+
cat >expect <<-EOF &&
193+
$oid_merge Merge branch '\''moves-side'\'' into moves-main
194+
$oid_mod_f1_side_2 Modify file-1 on side #2
195+
$oid_mod_f1_side_1 Modify file-1 on side #1
196+
$oid_mod_r1_main Modify renamed-1 on main
197+
$oid_mod_f1_main Modify file-1 on main
198+
$oid_add_f1_f2 Add file-1 and file-2
199+
EOF
200+
git log -L1:renamed-1 --oneline --no-patch >actual &&
201+
test_cmp expect actual
202+
'
203+
204+
test_expect_success 'fancy rename following #2' '
205+
cat >expect <<-EOF &&
206+
$oid_merge Merge branch '\''moves-side'\'' into moves-main
207+
$oid_mod_r2_side Modify renamed-2 on side
208+
$oid_mod_f2_side Modify file-2 on side
209+
$oid_mod_f2_main_2 Modify file-2 on main #2
210+
$oid_mod_f2_main_1 Modify file-2 on main #1
211+
$oid_add_f1_f2 Add file-1 and file-2
212+
EOF
213+
git log -L1:renamed-2 --oneline --no-patch >actual &&
214+
test_cmp expect actual
215+
'
216+
135217
test_done

0 commit comments

Comments
 (0)