Re: [PATCH v2] checkout: optimize "git checkout -b <new_branch>"

-- 8< --

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 28627650cd..912e565acc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c

@@ -478,6 +478,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
  {
  	int ret;
  	struct lock_file lock_file = LOCK_INIT;
+	int skip_cache_tree_update = 0;
  
  	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
  	if (read_cache_preload(NULL) < 0)

@@ -493,6 +494,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
  		struct tree_desc trees[2];
  		struct tree *tree;
  		struct unpack_trees_options topts;
+		int nr_trees;
  
  		memset(&topts, 0, sizeof(topts));
  		topts.head_idx = -1;

@@ -514,20 +516,45 @@ static int merge_working_tree(const struct checkout_opts *opts,
  		topts.merge = 1;
  		topts.gently = opts->merge && old_branch_info->commit;
  		topts.verbose_update = opts->show_progress;
-		topts.fn = twoway_merge;
+		/*
+		 * We repair cache-tree explicitly in this function,
+		 * no point in doing it again inside unpack_trees()
+		 */
+		topts.skip_cache_tree_update = 1;
  		if (opts->overwrite_ignore) {
  			topts.dir = xcalloc(1, sizeof(*topts.dir));
  			topts.dir->flags |= DIR_SHOW_IGNORED;
  			setup_standard_excludes(topts.dir);
  		}
-		tree = parse_tree_indirect(old_branch_info->commit ?
-					   &old_branch_info->commit->object.oid :
-					   the_hash_algo->empty_tree);
-		init_tree_desc(&trees[0], tree->buffer, tree->size);
-		tree = parse_tree_indirect(&new_branch_info->commit->object.oid);
-		init_tree_desc(&trees[1], tree->buffer, tree->size);
-
-		ret = unpack_trees(2, trees, &topts);
+		if (!oidcmp(&old_branch_info->commit->object.oid,
+			    &new_branch_info->commit->object.oid)) {
+			/*
+			 * Skip 2-way merge because we know the end
+			 * result would be the same anyway (to double
+			 * check).
+			 */
+			topts.fn = noop_merge;
+			nr_trees = 0;
+			/*
+			 * Repairing cache-tree is an optional optimization
+			 * and could be safely skipped. Maybe have some
+			 * heuristics based on the_index->cache_nr to
+			 * determine if updating cache-tree could be too
+			 * costly (including index writing cost) and skip it
+			 * instead.
+			 */
+			skip_cache_tree_update = 1;
+		} else {
+			topts.fn = twoway_merge;
+			tree = parse_tree_indirect(old_branch_info->commit ?
+						   &old_branch_info->commit->object.oid :
+						   the_hash_algo->empty_tree);
+			init_tree_desc(&trees[0], tree->buffer, tree->size);
+			tree = parse_tree_indirect(&new_branch_info->commit->object.oid);
+			init_tree_desc(&trees[1], tree->buffer, tree->size);
+			nr_trees = 2;
+		}
+		ret = unpack_trees(nr_trees, trees, &topts);
  		clear_unpack_trees_porcelain(&topts);
  		if (ret == -1) {
  			/*

@@ -597,13 +624,15 @@ static int merge_working_tree(const struct checkout_opts *opts,
  		}
  	}
  
-	if (!active_cache_tree)
-		active_cache_tree = cache_tree();
+	if (!skip_cache_tree_update) {
+		if (!active_cache_tree)
+			active_cache_tree = cache_tree();
  
-	if (!cache_tree_fully_valid(active_cache_tree))
-		cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
+		if (!cache_tree_fully_valid(active_cache_tree))
+			cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
+	}
  
-	if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
+	if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK | SKIP_IF_UNCHANGED))
  		die(_("unable to write new index file"));
  
  	if (!opts->force && !opts->quiet)

diff --git a/unpack-trees.c b/unpack-trees.c
index cd0680f11e..007d6662f2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c

@@ -1275,6 +1275,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
  	int i, ret;
  	static struct cache_entry *dfc;
  	struct exclude_list el;
+	int result_is_actually_src = 0;
  
  	if (len > MAX_UNPACK_TREES)
  		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);

@@ -1353,6 +1354,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
  
  		if (traverse_trees(len, t, &info) < 0)
  			goto return_failed;
+	} else if (o->fn == noop_merge) {
+		o->result = *o->src_index;
+		o->cache_bottom = o->src_index->cache_nr;
+		result_is_actually_src = 1;
  	}
  
  	/* Any left-over entries in the index? */

@@ -1426,7 +1431,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
  
  	ret = check_updates(o) ? (-2) : 0;
  	if (o->dst_index) {
-		if (!ret) {
+		if (!ret &&!o->skip_cache_tree_update) {
  			if (!o->result.cache_tree)
  				o->result.cache_tree = cache_tree();
  			if (!cache_tree_fully_valid(o->result.cache_tree))

@@ -1435,7 +1440,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
  						  WRITE_TREE_REPAIR);
  		}
  		move_index_extensions(&o->result, o->src_index);
-		discard_index(o->dst_index);
+		if (result_is_actually_src && o->dst_index == o->src_index)
+			; /* no discarding */
+		else
+			discard_index(o->dst_index);
  		*o->dst_index = o->result;
  	} else {
  		discard_index(&o->result);

@@ -2193,3 +2201,9 @@ int oneway_merge(const struct cache_entry * const *src,
  	}
  	return merged_entry(a, old, o);
  }
+
+int noop_merge(const struct cache_entry * const *src,
+	       struct unpack_trees_options *o)
+{
+	BUG("This is basically a flag and not supposed to be called");
+}

diff --git a/unpack-trees.h b/unpack-trees.h
index c2b434c606..3e5bce19a1 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h

@@ -52,6 +52,7 @@ struct unpack_trees_options {
  		     diff_index_cached,
  		     debug_unpack,
  		     skip_sparse_checkout,
+		     skip_cache_tree_update,
  		     gently,
  		     exiting_early,
  		     show_all_errors,

@@ -96,5 +97,7 @@ int bind_merge(const struct cache_entry * const *src,
  	       struct unpack_trees_options *o);
  int oneway_merge(const struct cache_entry * const *src,
  		 struct unpack_trees_options *o);
+int noop_merge(const struct cache_entry * const *src,
+	       struct unpack_trees_options *o);
  
  #endif
-- 8< --

--

Duy