On Thu, Nov 13, 2014 at 02:26:55PM -0500, Jeff King wrote:
> > Makes sense, including the use of strbuf (otherwise you would
> > allocate ce and then discard when it turns out that it is not
> > needed, which is probably with the same allocation pressure, but
> > looks uglier).
>
> Exactly. Constructing it in ce->name does save you an allocation/memcpy
> in the case that we actually use the new entry, but I thought it would
> look weirder. It probably doesn't matter much either way, so I tried to
> write the most obvious thing.
Actually, it is not that bad:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5410dac..5a78758 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -67,6 +67,7 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
{
int len;
struct cache_entry *ce;
+ int pos;
if (S_ISDIR(mode))
return READ_TREE_RECURSIVE;
@@ -79,6 +80,23 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
ce->ce_flags = create_ce_flags(0) | CE_UPDATE;
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
+
+ /*
+ * If the entry is the same as the current index, we can leave the old
+ * entry in place. Whether it is UPTODATE or not, checkout_entry will
+ * do the right thing.
+ */
+ pos = cache_name_pos(ce->name, ce->ce_namelen);
+ if (pos >= 0) {
+ struct cache_entry *old = active_cache[pos];
+ if (ce->ce_mode == old->ce_mode &&
+ !hashcmp(ce->sha1, old->sha1)) {
+ old->ce_flags |= CE_UPDATE;
+ free(ce);
+ return 0;
+ }
+ }
+
add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
return 0;
}
and in some ways more readable, as you form the whole thing, and then as
the final step either add it, or realize that what is there is fine (I'd
almost wonder if it could be a flag to add_cache_entry).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html