Performance optimization for detection of modified submodules
In the worst case is_submodule_modified() got called three times for each submodule. The information we got from scanning the whole submodule tree the first time can be reused instead. New parameters have been added to diff_change() and diff_addremove(), the information is stored in a new member of struct diff_filespec. Its value is then reused instead of calling is_submodule_modified() again. When no explicit "-dirty" is needed in the output the call to is_submodule_modified() is not necessary when the submodules HEAD already disagrees with the ref of the superproject, as this alone marks it as modified. To achieve that, get_stat_data() got an extra argument. Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jens Lehmann committed
Jan 18, 2010 at 21:26 UTC
e3d42c4773bccebb50f01b108d20b06c6a11e615
6 files changed
+56
-29
diff-lib.c
+31
-15
@@ -73,6 +73,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
73
unsigned int oldmode, newmode;
74
struct cache_entry *ce = active_cache[i];
75
int changed;
76
+ unsigned dirty_submodule = 0;
77
78
if (DIFF_OPT_TST(&revs->diffopt, QUICK) &&
79
DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
@@ -173,12 +174,16 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
174
if (silent_on_removed)
175
continue;
176
diff_addremove(&revs->diffopt, '-', ce->ce_mode,
176
- ce->sha1, ce->name);
177
+ ce->sha1, ce->name, 0);
178
continue;
179
}
180
changed = ce_match_stat(ce, &st, ce_option);
180
- if (S_ISGITLINK(ce->ce_mode) && !changed)
181
- changed = is_submodule_modified(ce->name);
181
+ if (S_ISGITLINK(ce->ce_mode)
182
+ && (!changed || (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
183
+ && is_submodule_modified(ce->name)) {
184
+ changed = 1;
185
+ dirty_submodule = 1;
186
+ }
187
if (!changed) {
188
ce_mark_uptodate(ce);
189
if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
@@ -188,7 +193,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
193
newmode = ce_mode_from_stat(ce, st.st_mode);
194
diff_change(&revs->diffopt, oldmode, newmode,
195
ce->sha1, (changed ? null_sha1 : ce->sha1),
191
- ce->name);
196
+ ce->name, 0, dirty_submodule);
197
198
}
199
diffcore_std(&revs->diffopt);
@@ -204,16 +209,18 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
209
static void diff_index_show_file(struct rev_info *revs,
210
const char *prefix,
211
struct cache_entry *ce,
207
- const unsigned char *sha1, unsigned int mode)
212
+ const unsigned char *sha1, unsigned int mode,
213
+ unsigned dirty_submodule)
214
{
215
diff_addremove(&revs->diffopt, prefix[0], mode,
210
- sha1, ce->name);
216
+ sha1, ce->name, dirty_submodule);
217
}
218
219
static int get_stat_data(struct cache_entry *ce,
220
const unsigned char **sha1p,
221
unsigned int *modep,
216
- int cached, int match_missing)
222
+ int cached, int match_missing,
223
+ unsigned *dirty_submodule, int output_format)
224
{
225
const unsigned char *sha1 = ce->sha1;
226
unsigned int mode = ce->ce_mode;
@@ -233,8 +240,13 @@ static int get_stat_data(struct cache_entry *ce,
240
return -1;
241
}
242
changed = ce_match_stat(ce, &st, 0);
236
- if (changed
237
- || (S_ISGITLINK(ce->ce_mode) && is_submodule_modified(ce->name))) {
243
+ if (S_ISGITLINK(ce->ce_mode)
244
+ && (!changed || (output_format & DIFF_FORMAT_PATCH))
245
+ && is_submodule_modified(ce->name)) {
246
+ changed = 1;
247
+ *dirty_submodule = 1;
248
+ }
249
+ if (changed) {
250
mode = ce_mode_from_stat(ce, st.st_mode);
251
sha1 = null_sha1;
252
}
@@ -251,15 +263,17 @@ static void show_new_file(struct rev_info *revs,
263
{
264
const unsigned char *sha1;
265
unsigned int mode;
266
+ unsigned dirty_submodule = 0;
267
268
/*
269
* New file in the index: it might actually be different in
270
* the working copy.
271
*/
259
- if (get_stat_data(new, &sha1, &mode, cached, match_missing) < 0)
272
+ if (get_stat_data(new, &sha1, &mode, cached, match_missing,
273
+ &dirty_submodule, revs->diffopt.output_format) < 0)
274
return;
275
262
- diff_index_show_file(revs, "+", new, sha1, mode);
276
+ diff_index_show_file(revs, "+", new, sha1, mode, dirty_submodule);
277
}
278
279
static int show_modified(struct rev_info *revs,
@@ -270,11 +284,13 @@ static int show_modified(struct rev_info *revs,
284
{
285
unsigned int mode, oldmode;
286
const unsigned char *sha1;
287
+ unsigned dirty_submodule = 0;
288
274
- if (get_stat_data(new, &sha1, &mode, cached, match_missing) < 0) {
289
+ if (get_stat_data(new, &sha1, &mode, cached, match_missing,
290
+ &dirty_submodule, revs->diffopt.output_format) < 0) {
291
if (report_missing)
292
diff_index_show_file(revs, "-", old,
277
- old->sha1, old->ce_mode);
293
+ old->sha1, old->ce_mode, 0);
294
return -1;
295
}
296
@@ -309,7 +325,7 @@ static int show_modified(struct rev_info *revs,
325
return 0;
326
327
diff_change(&revs->diffopt, oldmode, mode,
312
- old->sha1, sha1, old->name);
328
+ old->sha1, sha1, old->name, 0, dirty_submodule);
329
return 0;
330
}
331
@@ -356,7 +372,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,
372
* Something removed from the tree?
373
*/
374
if (!idx) {
359
- diff_index_show_file(revs, "-", tree, tree->sha1, tree->ce_mode);
375
+ diff_index_show_file(revs, "-", tree, tree->sha1, tree->ce_mode, 0);
376
return;
377
}
378
diff.c
+11
-4
@@ -2032,7 +2032,7 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
2032
char *data = xmalloc(100), *dirty = "";
2033
2034
/* Are we looking at the work tree? */
2035
- if (!s->sha1_valid && is_submodule_modified(s->path))
2035
+ if (!s->sha1_valid && s->dirty_submodule)
2036
dirty = "-dirty";
2037
2038
len = snprintf(data, 100,
@@ -3719,7 +3719,7 @@ int diff_result_code(struct diff_options *opt, int status)
3719
void diff_addremove(struct diff_options *options,
3720
int addremove, unsigned mode,
3721
const unsigned char *sha1,
3722
- const char *concatpath)
3722
+ const char *concatpath, unsigned dirty_submodule)
3723
{
3724
struct diff_filespec *one, *two;
3725
@@ -3751,8 +3751,10 @@ void diff_addremove(struct diff_options *options,
3751
3752
if (addremove != '+')
3753
fill_filespec(one, sha1, mode);
3754
- if (addremove != '-')
3754
+ if (addremove != '-') {
3755
fill_filespec(two, sha1, mode);
3756
+ two->dirty_submodule = dirty_submodule;
3757
+ }
3758
3759
diff_queue(&diff_queued_diff, one, two);
3760
if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
@@ -3763,7 +3765,8 @@ void diff_change(struct diff_options *options,
3765
unsigned old_mode, unsigned new_mode,
3766
const unsigned char *old_sha1,
3767
const unsigned char *new_sha1,
3766
- const char *concatpath)
3768
+ const char *concatpath,
3769
+ unsigned old_dirty_submodule, unsigned new_dirty_submodule)
3770
{
3771
struct diff_filespec *one, *two;
3772
@@ -3776,6 +3779,8 @@ void diff_change(struct diff_options *options,
3779
const unsigned char *tmp_c;
3780
tmp = old_mode; old_mode = new_mode; new_mode = tmp;
3781
tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
3782
+ tmp = old_dirty_submodule; old_dirty_submodule = new_dirty_submodule;
3783
+ new_dirty_submodule = tmp;
3784
}
3785
3786
if (options->prefix &&
@@ -3786,6 +3791,8 @@ void diff_change(struct diff_options *options,
3791
two = alloc_filespec(concatpath);
3792
fill_filespec(one, old_sha1, old_mode);
3793
fill_filespec(two, new_sha1, new_mode);
3794
+ one->dirty_submodule = old_dirty_submodule;
3795
+ two->dirty_submodule = new_dirty_submodule;
3796
3797
diff_queue(&diff_queued_diff, one, two);
3798
if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
diff.h
+6
-4
@@ -14,12 +14,13 @@ typedef void (*change_fn_t)(struct diff_options *options,
14
unsigned old_mode, unsigned new_mode,
15
const unsigned char *old_sha1,
16
const unsigned char *new_sha1,
17
- const char *fullpath);
17
+ const char *fullpath,
18
+ unsigned old_dirty_submodule, unsigned new_dirty_submodule);
19
20
typedef void (*add_remove_fn_t)(struct diff_options *options,
21
int addremove, unsigned mode,
22
const unsigned char *sha1,
22
- const char *fullpath);
23
+ const char *fullpath, unsigned dirty_submodule);
24
25
typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
26
struct diff_options *options, void *data);
@@ -177,13 +178,14 @@ extern void diff_addremove(struct diff_options *,
178
int addremove,
179
unsigned mode,
180
const unsigned char *sha1,
180
- const char *fullpath);
181
+ const char *fullpath, unsigned dirty_submodule);
182
183
extern void diff_change(struct diff_options *,
184
unsigned mode1, unsigned mode2,
185
const unsigned char *sha1,
186
const unsigned char *sha2,
186
- const char *fullpath);
187
+ const char *fullpath,
188
+ unsigned dirty_submodule1, unsigned dirty_submodule2);
189
190
extern void diff_unmerge(struct diff_options *,
191
const char *path,
diffcore.h
+1
@@ -42,6 +42,7 @@ struct diff_filespec {
42
#define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)
43
unsigned should_free : 1; /* data should be free()'ed */
44
unsigned should_munmap : 1; /* data should be munmap()'ed */
45
+ unsigned dirty_submodule : 1; /* For submodules: its work tree is dirty */
46
47
struct userdiff_driver *driver;
48
/* data should be considered "binary"; -1 means "don't know yet" */
revision.c
+3
-2
@@ -268,7 +268,7 @@ static int tree_difference = REV_TREE_SAME;
268
static void file_add_remove(struct diff_options *options,
269
int addremove, unsigned mode,
270
const unsigned char *sha1,
271
- const char *fullpath)
271
+ const char *fullpath, unsigned dirty_submodule)
272
{
273
int diff = addremove == '+' ? REV_TREE_NEW : REV_TREE_OLD;
274
@@ -281,7 +281,8 @@ static void file_change(struct diff_options *options,
281
unsigned old_mode, unsigned new_mode,
282
const unsigned char *old_sha1,
283
const unsigned char *new_sha1,
284
- const char *fullpath)
284
+ const char *fullpath,
285
+ unsigned old_dirty_submodule, unsigned new_dirty_submodule)
286
{
287
tree_difference = REV_TREE_DIFFERENT;
288
DIFF_OPT_SET(options, HAS_CHANGES);
tree-diff.c
+4
-4
@@ -68,7 +68,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
68
if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
69
newbase[baselen + pathlen1] = 0;
70
opt->change(opt, mode1, mode2,
71
- sha1, sha2, newbase);
71
+ sha1, sha2, newbase, 0, 0);
72
newbase[baselen + pathlen1] = '/';
73
}
74
retval = diff_tree_sha1(sha1, sha2, newbase, opt);
@@ -77,7 +77,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
77
}
78
79
fullname = malloc_fullname(base, baselen, path1, pathlen1);
80
- opt->change(opt, mode1, mode2, sha1, sha2, fullname);
80
+ opt->change(opt, mode1, mode2, sha1, sha2, fullname, 0, 0);
81
free(fullname);
82
return 0;
83
}
@@ -241,7 +241,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
241
242
if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
243
newbase[baselen + pathlen] = 0;
244
- opt->add_remove(opt, *prefix, mode, sha1, newbase);
244
+ opt->add_remove(opt, *prefix, mode, sha1, newbase, 0);
245
newbase[baselen + pathlen] = '/';
246
}
247
@@ -252,7 +252,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
252
free(newbase);
253
} else {
254
char *fullname = malloc_fullname(base, baselen, path, pathlen);
255
- opt->add_remove(opt, prefix[0], mode, sha1, fullname);
255
+ opt->add_remove(opt, prefix[0], mode, sha1, fullname, 0);
256
free(fullname);
257
}
258
}