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

> > If we're going to go this route, I think we should first check that
> > o->fn is one of those known safe functions.  And if we're going that
> > route, the comments I bring up on patch 2 about possibly avoiding
> > call_unpack_fn() altogether might even obviate this patch while
> > speeding things up more.
>
> Yes I do need to check o->fn. I might have to think more about
> avoiding call_unpack_fn(). Even if we avoid it though, we still go
> through add_index_entry() and suffer the same checks every time unless
> we do somethine like this (but then of course it's safer because
> you're doing it in a specific x-way merge, not generic code like
> this).

> > > @@ -1561,6 +1581,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> > >                 if (!ret) {
> > >                         if (!o->result.cache_tree)
> > >                                 o->result.cache_tree = cache_tree();
> > > +                       /*
> > > +                        * TODO: Walk o.src_index->cache_tree, quickly check
> > > +                        * if o->result.cache has the exact same content for
> > > +                        * any valid cache-tree in o.src_index, then we can
> > > +                        * just copy the cache-tree over instead of hashing a
> > > +                        * new tree object.
> > > +                        */
> >
> > Interesting.  I really don't know how cache_tree works...but if we
> > avoided calling call_unpack_fn, and thus left the original index entry
> > in place instead of replacing it with an equal one, would that as a
> > side effect speed up the cache_tree_valid/cache_tree_update calls for
> > us?  Or is there still work here?
>
> Naah. Notice that we don't care at all about the source's cache-tree
> when we update o->result one (and we never ever do anything about
> o->result's cache-tree during the merge). Whether you invalidate or
> not, o->result's cache-tree is always empty and you still have to
> recreate all cache-tree in o->result. You essentially play full cost
> of "git write-tree" here if I'm not mistaken.