@matheusbsbs / matheus-bagliano / commits / e721c1544f

checkout: avoid unnecessary match_pathspec calls

In checkout_paths() we do this - for all updated items, call match_pathspec - for all items, call match_pathspec (inside unmerge_cache) - for all items, call match_pathspec (for showing "path .. is unmerged) - for updated items, call match_pathspec and update paths That's a lot of duplicate match_pathspec(s) and the function is not exactly cheap to be called so many times, especially on large indexes. This patch makes it call match_pathspec once per updated index entry, save the result in ce_flags and reuse the results in the following loops. The changes in 0a1283b (checkout $tree $path: do not clobber local changes in $path not in $tree - 2011-09-30) limit the affected paths to ones we read from $tree. We do not do anything to other modified entries in this case, so the "for all items" above could be modified to "for all updated items". But.. The command's behavior now is modified slightly: unmerged entries that match $path, but not updated by $tree, are now NOT touched. Although this should be considered a bug fix, not a regression. A new test is added for this change. And while at there, free ps_matched after use. The following command is tested on webkit, 215k entries. The pattern is chosen mainly to make match_pathspec sweat: git checkout -- "*[a-zA-Z]*[a-zA-Z]*[a-zA-Z]*" before after real 0m3.493s 0m2.737s user 0m2.239s 0m1.586s sys 0m1.252s 0m1.151s Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Mar 27, 2013 at 12:58 UTC e721c1544f9166e8f08a7f8c0e6178ea3a45e255

5 files changed +80 -8

builtin/checkout.c

+36 -7

@@ -271,24 +271,55 @@ static int checkout_paths(const struct checkout_opts *opts,

271 ;

272 ps_matched = xcalloc(1, pos);

273

274 + /*

275 + * Make sure all pathspecs participated in locating the paths

276 + * to be checked out.

277 + */

278 for (pos = 0; pos < active_nr; pos++) {

279 struct cache_entry *ce = active_cache[pos];

280 + ce->ce_flags &= ~CE_MATCHED;

281 if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))

282 + /*

283 + * "git checkout tree-ish -- path", but this entry

284 + * is in the original index; it will not be checked

285 + * out to the working tree and it does not matter

286 + * if pathspec matched this entry. We will not do

287 + * anything to this entry at all.

288 + */

289 continue;

278 - match_pathspec(opts->pathspec, ce->name, ce_namelen(ce), 0, ps_matched);

290 + /*

291 + * Either this entry came from the tree-ish we are

292 + * checking the paths out of, or we are checking out

293 + * of the index.

294 + *

295 + * If it comes from the tree-ish, we already know it

296 + * matches the pathspec and could just stamp

297 + * CE_MATCHED to it from update_some(). But we still

298 + * need ps_matched and read_tree_recursive (and

299 + * eventually tree_entry_interesting) cannot fill

300 + * ps_matched yet. Once it can, we can avoid calling

301 + * match_pathspec() for _all_ entries when

302 + * opts->source_tree != NULL.

303 + */

304 + if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce),

305 + 0, ps_matched))

306 + ce->ce_flags |= CE_MATCHED;

307 }

308

281 - if (report_path_error(ps_matched, opts->pathspec, opts->prefix))

309 + if (report_path_error(ps_matched, opts->pathspec, opts->prefix)) {

310 + free(ps_matched);

311 return 1;

312 + }

313 + free(ps_matched);

314

315 /* "checkout -m path" to recreate conflicted state */

316 if (opts->merge)

286 - unmerge_cache(opts->pathspec);

317 + unmerge_marked_index(&the_index);

318

319 /* Any unmerged paths? */

320 for (pos = 0; pos < active_nr; pos++) {

321 struct cache_entry *ce = active_cache[pos];

291 - if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce), 0, NULL)) {

322 + if (ce->ce_flags & CE_MATCHED) {

323 if (!ce_stage(ce))

324 continue;

325 if (opts->force) {

@@ -313,9 +344,7 @@ static int checkout_paths(const struct checkout_opts *opts,

344 state.refresh_cache = 1;

345 for (pos = 0; pos < active_nr; pos++) {

346 struct cache_entry *ce = active_cache[pos];

316 - if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))

317 - continue;

318 - if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce), 0, NULL)) {

347 + if (ce->ce_flags & CE_MATCHED) {

348 if (!ce_stage(ce)) {

349 errs |= checkout_entry(ce, &state, NULL);

350 continue;

cache.h

@@ -162,6 +162,9 @@ struct cache_entry {

162 #define CE_UNPACKED (1 << 24)

163 #define CE_NEW_SKIP_WORKTREE (1 << 25)

164

165 +/* used to temporarily mark paths matched by pathspecs */

166 +#define CE_MATCHED (1 << 26)

167 +

168 /*

169 * Extended on-disk flags

170 */

resolve-undo.c

+18 -1

@@ -118,7 +118,7 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)

118 struct cache_entry *ce;

119 struct string_list_item *item;

120 struct resolve_undo_info *ru;

121 - int i, err = 0;

121 + int i, err = 0, matched;

122

123 if (!istate->resolve_undo)

124 return pos;

@@ -137,6 +137,7 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)

137 ru = item->util;

138 if (!ru)

139 return pos;

140 + matched = ce->ce_flags & CE_MATCHED;

141 remove_index_entry_at(istate, pos);

142 for (i = 0; i < 3; i++) {

143 struct cache_entry *nce;

@@ -144,6 +145,8 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)

145 continue;

146 nce = make_cache_entry(ru->mode[i], ru->sha1[i],

147 ce->name, i + 1, 0);

148 + if (matched)

149 + nce->ce_flags |= CE_MATCHED;

150 if (add_index_entry(istate, nce, ADD_CACHE_OK_TO_ADD)) {

151 err = 1;

152 error("cannot unmerge '%s'", ce->name);

@@ -156,6 +159,20 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)

159 return unmerge_index_entry_at(istate, pos);

160 }

161

162 +void unmerge_marked_index(struct index_state *istate)

163 +{

164 + int i;

165 +

166 + if (!istate->resolve_undo)

167 + return;

168 +

169 + for (i = 0; i < istate->cache_nr; i++) {

170 + struct cache_entry *ce = istate->cache[i];

171 + if (ce->ce_flags & CE_MATCHED)

172 + i = unmerge_index_entry_at(istate, i);

173 + }

174 +}

175 +

176 void unmerge_index(struct index_state *istate, const char **pathspec)

177 {

178 int i;

resolve-undo.h

@@ -12,5 +12,6 @@ extern struct string_list *resolve_undo_read(const char *, unsigned long);

12 extern void resolve_undo_clear_index(struct index_state *);

13 extern int unmerge_index_entry_at(struct index_state *, int);

14 extern void unmerge_index(struct index_state *, const char **);

15 +extern void unmerge_marked_index(struct index_state *);

17 #endif

t/t2022-checkout-paths.sh

+22

@@ -39,4 +39,26 @@ test_expect_success 'checking out paths out of a tree does not clobber unrelated

39 test_cmp expect.next2 dir/next2

40 '

42 +test_expect_success 'do not touch unmerged entries matching $path but not in $tree' '

43 + git checkout next &&

44 + git reset --hard &&

45 +

46 + cat dir/common >expect.common &&

47 + EMPTY_SHA1=$(git hash-object -w --stdin </dev/null) &&

48 + git rm dir/next0 &&

49 + cat >expect.next0 <<-EOF &&

50 + 100644 $EMPTY_SHA1 1 dir/next0

51 + 100644 $EMPTY_SHA1 2 dir/next0

52 + EOF

53 + git update-index --index-info <expect.next0 &&

54 +

55 + git checkout master dir &&

56 +

57 + test_cmp expect.common dir/common &&

58 + test_path_is_file dir/master &&

59 + git diff --exit-code master dir/master &&

60 + git ls-files -s dir/next0 >actual.next0 &&

61 + test_cmp expect.next0 actual.next0

62 +'

63 +

64 test_done