> Jeff King <peff@xxxxxxxx> writes:
>
> > Hmm. That commit removes the call to display_progress() from the main
> > loop of threaded_second_pass(), and doesn't appear to add another one
> > anywhere. Is the solution really as simple as adding it back in? I.e.
> >
> > diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> > index 8acd078aa0..6dbb4317e0 100644
> > --- a/builtin/index-pack.c
> > +++ b/builtin/index-pack.c
> > @@ -1028,6 +1028,10 @@ static void *threaded_second_pass(void *data)
> > struct object_entry *child_obj;
> > struct base_data *child;
> >
> > + counter_lock();
> > + display_progress(progress, nr_resolved_deltas);
> > + counter_unlock();
> > +
> > work_lock();
> > if (list_empty(&work_head)) {
> > /*
> >
> > That _seems_ to work fine, but I'm not sure why it was removed in the
> > first place (for a good reason, or simply as an accident when rewriting
> > the variable declarations at the top of the loop?).
>
> The above looks like an obvious and trivial fix to go back closer to
> the original.
Thanks for noticing it, Peff. Yes, that looks like the correct fix. I
thought that it might have crept in when I was rebasing, but looks like
it was present in the original version [1].
[1] https://lore.kernel.org/git/505d8e79cd983d5b3dfd56c4f0432ad647132957.1570663470.git.jonathantanmy@xxxxxxxxxx/
> We seem to have removed find_unresolved_deltas() helper function in
> that series, but there remains a mention to it in a comment, which
> we would probably should rethink (it just may be the matter of
> removing the mention, or if "just like in ..." may have been the
> last example of doing what the comment suggests all code to do, it
> may reveal a need for larger clean-up---I dunno).
Maybe the whole comment block should be deleted and replaced with
something like:
Ensure that this node has been uncompressed and return its contents.
In the typical and best case, this node would already be uncompressed
(through the invocation to resolve_delta() in threaded_second_pass())
and it would not be pruned. However, if pruning of this node was
necessary due to reaching delta_base_cache_limit, this function will
find the closest ancestor with uncompressed data that has not been
pruned (or if there is none, the ultimate base object), and uncompress
each node in the delta chain in order to generate the uncompressed data
for this node.
(I'm using "uncompress" here because I find the original "deflate" term
confusing - I thought that "deflate" meant compress, and thus the
"data" here would be the uncompressed form, and hence undeflated or
inflated.)
The original comment [2] was describing two paths in which uncompressed
forms could be generated - one in find_unresolved_deltas() going from
parent to children, and one here from "current node to top parent"; and
it said that all generated uncompressed forms (here, and "just like in
find_unresolved_deltas()" - as Junio quoted) can be pruned. I think my
suggested comment block above retains that information.
[2] https://kernel.googlesource.com/pub/scm/git/git/+/refs/tags/v2.28.0/builtin/index-pack.c#868
