Skip to content

Commit

Permalink
unpack-trees: exit check_updates() early if updates are not wanted
Browse files Browse the repository at this point in the history
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.
  * The code would previously call setup_collided_checkout_detection()
    and report_collided_checkout() even when o->dry_run.  However, this
    was just an expensive no-op because
    setup_collided_checkout_detection() merely cleared the CE_MATCHED
    flag for each cache entry, and report_collided_checkout() reported
    which ones had it set.  Since a dry-run would skip all the
    checkout_entry() calls, CE_MATCHED would never get set and thus
    no collisions would be reported.  Since we can't detect the
    collisions anyway without doing updates, skipping the collisions
    detection setup and reporting is an optimization.
  * The code previously would call get_progress() and
    display_progress() even when (!o->update || o->dry_run).  This
    served to show how long it took to skip all the updates, which is
    somewhat useless.  Since we are skipping the updates, we can skip
    showing how long it takes to skip them.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
newren authored and gitster committed Jan 7, 2020
1 parent 53a06cf commit 26f924d
Showing 1 changed file with 14 additions and 12 deletions.
26 changes: 14 additions & 12 deletions unpack-trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -372,34 +372,39 @@ static int check_updates(struct unpack_trees_options *o)
state.refresh_cache = 1;
state.istate = index;

if (!o->update || o->dry_run) {
remove_marked_cache_entries(index, 0);
trace_performance_leave("check_updates");
return 0;
}

if (o->clone)
setup_collided_checkout_detection(&state, index);

progress = get_progress(o);

if (o->update)
git_attr_set_direction(GIT_ATTR_CHECKOUT);
git_attr_set_direction(GIT_ATTR_CHECKOUT);

if (should_update_submodules() && o->update && !o->dry_run)
if (should_update_submodules())
load_gitmodules_file(index, NULL);

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

if (ce->ce_flags & CE_WT_REMOVE) {
display_progress(progress, ++cnt);
if (o->update && !o->dry_run)
unlink_entry(ce);
unlink_entry(ce);
}
}

remove_marked_cache_entries(index, 0);
remove_scheduled_dirs();

if (should_update_submodules() && o->update && !o->dry_run)
if (should_update_submodules())
load_gitmodules_file(index, &state);

enable_delayed_checkout(&state);
if (has_promisor_remote() && o->update && !o->dry_run) {
if (has_promisor_remote()) {
/*
* Prefetch the objects that are to be checked out in the loop
* below.
Expand Down Expand Up @@ -431,15 +436,12 @@ static int check_updates(struct unpack_trees_options *o)
ce->name);
display_progress(progress, ++cnt);
ce->ce_flags &= ~CE_UPDATE;
if (o->update && !o->dry_run) {
errs |= checkout_entry(ce, &state, NULL, NULL);
}
errs |= checkout_entry(ce, &state, NULL, NULL);
}
}
stop_progress(&progress);
errs |= finish_delayed_checkout(&state, NULL);
if (o->update)
git_attr_set_direction(GIT_ATTR_CHECKIN);
git_attr_set_direction(GIT_ATTR_CHECKIN);

if (o->clone)
report_collided_checkout(index);
Expand Down

0 comments on commit 26f924d

Please sign in to comment.