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

On Thu, Aug 9, 2018 at 10:16 AM Ben Peart [off-list ref] wrote:

In fact, in the other [1] patch series, we're detecting the number of
cache entries that are the same as the cache tree and using that to
traverse_by_cache_tree().  At that point, couldn't we copy the
corresponding cache tree entries over to the destination so that those
don't have to get recreated in the later call to cache_tree_update()?

We could. But as I stated in another mail, I saw this cache-tree
optimization as a separate problem and didn't want to mix them up.
That way cache_tree_copy() could be used elsewhere if the opportunity
shows up.

Mixing them up could also complicate the problem. You you merge stuff,
you add new cache-entries to o->result with add_index_entry() which
tries to invalidate those paths in o->result's cache-tree. Right now
the cache-tree is empty so it's really no-op. But if you copy
cache-tree over while merging, that invalidation might either
invalidate your newly copied cache-tree, or get slowed down because
non-empty o->result's cache-tree means you start to need to walk it to
find if there's any path to invalidate.

PS. This code keeps messing me up. invalidate_ce_path() may also
invalidate cache-tree in the _source_ index. For this optimization to
really shine, you better keep the the original cache-tree intact (so
that you can reuse as much as possible).

I don't see the purpose of this source cache tree invalidation at all.
My guess at this point is Linus actually made a mistake in 34110cd4e3
(Make 'unpack_trees()' have a separate source and destination index -
2008-03-06) and he should have invalidated _destination_ index instead
of the source one. I'm going to dig in some more and probably will
send a patch to remove this invalidation.
-- 
Duy