[PATCH 1/1] unpack-trees: exit check_updates() early if updates are not wanted

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

From: Elijah Newren <newren@gmail.com>

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 2c9078d05bf2
("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 33ecf7eb61 (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 7847892716 (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 <newren@gmail.com>
---
 unpack-trees.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 2399b6818b..4c68dbdb43 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -372,15 +372,20 @@ 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++) {
@@ -388,18 +393,18 @@ static int check_updates(struct unpack_trees_options *o)
 
 		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.
@@ -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);
-- 
gitgitgadget