Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes: > Thomas Rast wrote: >> + struct object *obj; >> + int type = sha1_object_info(sha1, NULL); >> + /* If it's neither tag nor commit, parsing the object is wasted effort */ >> + if (type != OBJ_TAG && type != OBJ_COMMIT) >> + return NULL; >> + obj = deref_tag(parse_object(sha1), NULL, 0); [...] > In contrast, parse_object() first calls lookup_object() to look it up > in some hashtable to get the type -- the packfile idx, presumably? > Why don't you also do that instead of sha1_object_info()? Or, why > don't you wrap parse_object() in an API that doesn't go beyond the > first blob check (and not execute parse_object_buffer())? > > Also, does this patch fix the bug Alex reported? > > Apologies if I've misunderstood something horribly (which does seem to > be the case). Yes, it does fix the bug. (It's not really buggy, just slow.) However, you implicitly point out an important point: If we have the object, and it was already parsed (obj->parsed is set), parse_object() is essentially free. But sha1_object_info is not, it will in particular unconditionally dig through long delta chains to discover the base type of an object that has already been unpacked. As for your original questions: lookup_object() is "do we have it in our big object hashtable?" -- the one that holds many[1] objects, that Peff recently sped up. sha1_object_info() and read_object() are in many ways parallel functions that do approximately the following: check all pack indexes for this object if we found a hit: attempt to unpack by recursively going through deltas (for _info, no need to unpack, but we still go through the delta chain because the type of object is determined by the innermost delta base) try to load it as a loose object it could have been repacked and pruned while we were looking, so: reload pack index information try the packs again (search indexes, then unpack) complain [1] blobs in particular are frequently not stored in that hash table, because it is an insert-only table -- Thomas Rast trast@{inf,student}.ethz.ch -- 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