Skip to content

Commit 8be9973

Browse files
newrengitster
authored andcommitted
unpack-trees: exit check_updates() early if updates are not wanted
check_updates() has a lot of code that repeatedly checks whether o->update or o->dry_run are set. (Note that o->dry_run is a near-synonym for !o->update, but not quite as per commit 2c9078d ("unpack-trees: add the dry_run flag to unpack_trees_options", 2011-05-25).) In fact, this function almost turns into a no-op whenever the condition !o->update || o->dry_run is met. Simplify the code by checking this condition at the beginning of the function, and when it is true, do the few things that are relevant and return early. There are a few things that make the conversion not quite obvious: * The fact that check_updates() does not actually turn into a no-op when updates are not wanted may be slightly surprising. However, commit 33ecf7e (Discard "deleted" cache entries after using them to update the working tree, 2008-02-07) put the discarding of unused cache entries in check_updates() so we still need to keep the call to remove_marked_cache_entries(). It's possible this call belongs in another function, but it is certainly needed as tests will fail if it is removed. * The original called remove_scheduled_dirs() unconditionally. Technically, commit 7847892 (unlink_entry(): introduce schedule_dir_for_removal(), 2009-02-09) should have made that call conditional, but it didn't matter in practice because remove_scheduled_dirs() becomes a no-op when all the calls to unlink_entry() are skipped. As such, we do not need to call it. * When (o->dry_run && o->update), the original would have two calls to git_attr_set_direction() surrounding a bunch of skipped updates. These two calls to git_attr_set_direction() cancel each other out and thus can be omitted when o->dry_run is true just as they already are when !o->update. Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 53a06cf commit 8be9973

File tree

1 file changed

+14
-12
lines changed

1 file changed

+14
-12
lines changed

unpack-trees.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -372,34 +372,39 @@ static int check_updates(struct unpack_trees_options *o)
372372
state.refresh_cache = 1;
373373
state.istate = index;
374374

375+
if (!o->update || o->dry_run) {
376+
remove_marked_cache_entries(index, 0);
377+
trace_performance_leave("check_updates");
378+
return 0;
379+
}
380+
375381
if (o->clone)
376382
setup_collided_checkout_detection(&state, index);
377383

378384
progress = get_progress(o);
379385

380-
if (o->update)
381-
git_attr_set_direction(GIT_ATTR_CHECKOUT);
386+
git_attr_set_direction(GIT_ATTR_CHECKOUT);
382387

383-
if (should_update_submodules() && o->update && !o->dry_run)
388+
if (should_update_submodules())
384389
load_gitmodules_file(index, NULL);
385390

386391
for (i = 0; i < index->cache_nr; i++) {
387392
const struct cache_entry *ce = index->cache[i];
388393

389394
if (ce->ce_flags & CE_WT_REMOVE) {
390395
display_progress(progress, ++cnt);
391-
if (o->update && !o->dry_run)
392-
unlink_entry(ce);
396+
unlink_entry(ce);
393397
}
394398
}
399+
395400
remove_marked_cache_entries(index, 0);
396401
remove_scheduled_dirs();
397402

398-
if (should_update_submodules() && o->update && !o->dry_run)
403+
if (should_update_submodules())
399404
load_gitmodules_file(index, &state);
400405

401406
enable_delayed_checkout(&state);
402-
if (has_promisor_remote() && o->update && !o->dry_run) {
407+
if (has_promisor_remote()) {
403408
/*
404409
* Prefetch the objects that are to be checked out in the loop
405410
* below.
@@ -431,15 +436,12 @@ static int check_updates(struct unpack_trees_options *o)
431436
ce->name);
432437
display_progress(progress, ++cnt);
433438
ce->ce_flags &= ~CE_UPDATE;
434-
if (o->update && !o->dry_run) {
435-
errs |= checkout_entry(ce, &state, NULL, NULL);
436-
}
439+
errs |= checkout_entry(ce, &state, NULL, NULL);
437440
}
438441
}
439442
stop_progress(&progress);
440443
errs |= finish_delayed_checkout(&state, NULL);
441-
if (o->update)
442-
git_attr_set_direction(GIT_ATTR_CHECKIN);
444+
git_attr_set_direction(GIT_ATTR_CHECKIN);
443445

444446
if (o->clone)
445447
report_collided_checkout(index);

0 commit comments

Comments
 (0)