Re: [PATCH v3 0/4] Speed up unpack_trees()

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> not used behind is *not* OK.  And lack of restoring the bottom in
>> the new codepath makes me suspect exactly such a bug _after_ the
>> traversal exits the subtree we are using this new optimization in
>> and moves on.
>
> Hmph, thinking about this further, I cannot convince myself that
> lack of bottom adjustment can lead to a triggerable bug.  The only
> case that a subtree traversal need to skip some unpacked entries in
> the index and then revisit them by rewinding, e.g. entries "t-i" and
> "t-j" that are left unprocessed while entries "t/1", "t/2", etc. are
> processed, in the illustration of da165f47 ("unpack-trees.c: prepare
> for looking ahead in the index", 2010-01-07), is when one of the
> trees have a non-tree with the same name as the subtree we are
> trying to descend into, and as long as we know all trees have the
> thing as a tree, I do not think of a case where such ordering
> inversion would get in the way.

One more, and hopefully the final, note.

A paranoid may be soothed by a simple "cache_bottom must match pos
at this point" at the beginning of the optimized traversal.  Just
like you already have an assert to ensure that pos points at the
first entry in the directory in index_pos_by_traverse_info(), there
should not be any unused entry in the index before that entry and
the bottom pointer must be pointing at it.  It is a cheap check, and
if violated, would indicate that the above "I do not think of a
case ..." was incomplete.

> That was the only thing I found questionable in 2/4, which is the
> most important piece in the series, so we probably are OK.
>
> Thanks for working on this one.