Re: [PATCH v2 4/4] unpack-trees: cheaper index update when walking by cache-tree

From: Duy Nguyen <pclouds@gmail.com>
Date: Aug 10, 2018, 19:48 UTC
Message-ID: <CACsJy8D+UcmokEVn-=PRp7cZMK9fY0H+epKJpR8ytLSJdjWHcg@mail.gmail.com>
In-Reply-To: <CABPp-BHMC8k3t2_9KzdJvg80e-nqwsbLUceTLNjQ=ST=9XthEA@mail.gmail.com>

On Fri, Aug 10, 2018 at 9:40 PM Elijah Newren <newren@gmail.com> wrote:

Show 18 quoted lines

>
> On Fri, Aug 10, 2018 at 12:30 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > On Fri, Aug 10, 2018 at 8:39 PM Elijah Newren <newren@gmail.com> wrote:
> ...
> > > Why do we still need to go through add_index_entry()?  I thought that
> > > the whole point was that you already checked that at the current path,
> > > the trees being unpacked were all equal and matched both the index and
> > > the cache_tree.  If so, why is there any need for an update at all?
> > > (Did I read your all_trees_same_as_cache_tree() function wrong, and
> > > you don't actually know these all match in some important way?)
> >
> > Unless fn is oneway_diff, we have to create a new index (in o->result)
> > based on o->src_index and some other trees. So we have to add entries
>
> Oh, right, o->src_index may not equal o->dst_index (because of people
> like me who call it that way from merge-recursive.c) and even if it
> does, we still have the temporary o->result in the mean time.  I
> should have remembered that; just didn't.

Your forgetting about this actually helps. I think the idea of avoiding add_index_entry() may be worth considering.

We know that 90% of cases of unpack_trees() is from the_index to the_index. So if instead of creating a full temporary index, where 90% of it might be the same as source index, if we just mark in the source index (e.g. in ce_flags) the entries that should be copied to o->result and _not_ create them in o->result. When it's time to create o->dst_index (which is the_index) from o->result, we could just do little manipulation to delete stuff that the_index has but o->result does not and add a bit more things. It is something that at least sounds nice in my head, but I'm not sure if it works out...

-- 
Duy