Re: [PATCH v2 13/13] merge-ort, diffcore-rename: employ cached renames when possible

> On Mon, May 17, 2021 at 7:23 AM Derrick Stolee <stolee@gmail.com> wrote:>> As I was reading, I was also thinking that it would be good to
>> have some kind of tracing, if only a summary of how often we
>> relied upon the cached renames.
> 
> That's an interesting thought.  If there are renames that remain
> cached, the code always uses them (whether we need the cached rename
> or not doesn't matter since we already have it and it is cheap to
> use), so a summary would probably be the only thing that would make
> sense.
> 
> The easiest summary of how often we rely upon cached renames, is
> checking if we have enough to avoid calling diffcore_rename_extended()
> entirely.  That's precisely what the code above does that you are
> responding to.
> 
> What more information could we get out?  I guess we could get ever so
> slightly more information by tracking how many times we decide that
> the cache from a previous merge can be re-used (by tracking the number
> of times that cached_valid_side is set to 1 or 2 instead of 0).  Since
> we sometimes have some cached renames but not enough to skip rename
> detection (because source paths that were irrelevant in previous
> commits are marked relevant for the current commit), this would be an
> independent number from the region_enter-diffcore_rename count used
> above.
> 
>> That would present a mechanism
>> for the test cases to verify that the rename cache is behaving
>> as expected
> 
> How so?  I did check something like that above with verifying I was
> able to reduce the number of calls to diffcore_rename_extended() while
> still getting the expected result (which can only be done if renames
> are known).  The only additional thing I can think of we could check
> would be a testcase where renames are cached and can be re-used but
> it's not enough to avoid calling diffcore_rename_extended().  Did you
> have something else in mind?
> 
>> but also provides a way to diagnose any issues that
>> might arise in the future by asking a user to reproduce a
>> problematic rebase/merge with a GIT_TRACE2* target. Such a
>> change would fit as a follow-up, and does not need to insert
>> into an already heavy patch.
> 
> I'm not sure what information could be recorded that would help
> diagnose any such issues.  "9 out of 10 commits in your rebase reused
> cached renames" just doesn't seem granular enough to help.  Is there
> something else you were thinking of recording?