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

From: Duy Nguyen <pclouds@gmail.com>
Date: Aug 10, 2018, 16:08 UTC
Message-ID: <CACsJy8AOV+73RLx2GyWTvcoKxop2t_9x0mFjN9raOsGjDdQ2bg@mail.gmail.com>
In-Reply-To: <eb39eecf-81b0-e937-d686-47b7565d6511@gmail.com>

On Thu, Aug 9, 2018 at 10:16 AM Ben Peart <peartben@gmail.com> wrote:

Show 5 quoted lines

> 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