Re: [PATCH 1/2] diffcore-rename: support rename cache

7 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:37

Yann Dirson [off-list ref] writes:
On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
quoted
This patch teaches diffcore_rename() to look into
$GIT_DIR/rename-cache and make use of it to recreate diff_filepair.
With proper cache, there should be no available entry for estimation
after exact matching.
This is something I have thought about in the past, good to see that
implemented :)
quoted
Rename caching is per commit. I don't think abitrary tree-tree caching
is worth it.
That could be a nice complement to my directory-rename patch.
Has anybody thought about interaction between that caching and pathspec
limited operation?

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:45:37

On 11/8/08, Junio C Hamano [off-list ref] wrote:
Yann Dirson [off-list ref] writes:

 > On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
 >> This patch teaches diffcore_rename() to look into
 >> $GIT_DIR/rename-cache and make use of it to recreate diff_filepair.
 >> With proper cache, there should be no available entry for estimation
 >> after exact matching.
 >
 > This is something I have thought about in the past, good to see that
 > implemented :)
 >
 >> Rename caching is per commit. I don't think abitrary tree-tree caching
 >> is worth it.
 >
 > That could be a nice complement to my directory-rename patch.


Has anybody thought about interaction between that caching and pathspec
 limited operation?
I didn't. But I think all out-of-pathspec diff pairs are removed
before it reaches diffcore_rename() so the cache has nothing to do
with it (except it still loads full cache for a commit).
-- 
Duy

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Yann Dirson <hidden>
Date: 2016-06-15 22:45:37

On Sat, Nov 08, 2008 at 11:01:20AM +0700, Nguyen Thai Ngoc Duy wrote:
On 11/8/08, Junio C Hamano [off-list ref] wrote:
quoted
Yann Dirson [off-list ref] writes:

 > On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
 >> This patch teaches diffcore_rename() to look into
 >> $GIT_DIR/rename-cache and make use of it to recreate diff_filepair.
 >> With proper cache, there should be no available entry for estimation
 >> after exact matching.
 >
 > This is something I have thought about in the past, good to see that
 > implemented :)
 >
 >> Rename caching is per commit. I don't think abitrary tree-tree caching
 >> is worth it.
 >
 > That could be a nice complement to my directory-rename patch.


Has anybody thought about interaction between that caching and pathspec
 limited operation?
I didn't. But I think all out-of-pathspec diff pairs are removed
before it reaches diffcore_rename() so the cache has nothing to do
with it (except it still loads full cache for a commit).
Well, it could be that an out-of-pathspec pair would have a better
score than an in-pathspec one.  Maybe cache recording should be turned
off when doing pathspec limitation ?

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:45:37

On 11/8/08, Yann Dirson [off-list ref] wrote:
On Sat, Nov 08, 2008 at 11:01:20AM +0700, Nguyen Thai Ngoc Duy wrote:
 > On 11/8/08, Junio C Hamano [off-list ref] wrote:
 > > Yann Dirson [off-list ref] writes:
 > >
 > >  > On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
 > >  >> This patch teaches diffcore_rename() to look into
 > >  >> $GIT_DIR/rename-cache and make use of it to recreate diff_filepair.
 > >  >> With proper cache, there should be no available entry for estimation
 > >  >> after exact matching.
 > >  >
 > >  > This is something I have thought about in the past, good to see that
 > >  > implemented :)
 > >  >
 > >  >> Rename caching is per commit. I don't think abitrary tree-tree caching
 > >  >> is worth it.
 > >  >
 > >  > That could be a nice complement to my directory-rename patch.
 > >
 > >
 > > Has anybody thought about interaction between that caching and pathspec
 > >  limited operation?
 > >
 >
 > I didn't. But I think all out-of-pathspec diff pairs are removed
 > before it reaches diffcore_rename() so the cache has nothing to do
 > with it (except it still loads full cache for a commit).


Well, it could be that an out-of-pathspec pair would have a better
 score than an in-pathspec one.  Maybe cache recording should be turned
 off when doing pathspec limitation ?
Right, recording should be turned off or something. Let me see..
-- 
Duy

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Jeff King <hidden>
Date: 2016-06-15 22:45:37

On Sat, Nov 08, 2008 at 10:24:10AM +0100, Yann Dirson wrote:
Well, it could be that an out-of-pathspec pair would have a better
score than an in-pathspec one.  Maybe cache recording should be turned
off when doing pathspec limitation ?
One thing I notice is that the cache works at the level of "here is the
best rename for this commit." Maybe it could go down a level and say
"here is the inexact rename score between these blobs". Then you would
still find the best score between two blobs each time, but save the
really computationally intensive part (which is comparing the actual
_content_ of the blobs).

That should work in the face of path limiting or any other option,
because it is caching something immutable: this is the similarity score
between two pieces of content. And then you get arbitrary tree-to-tree
speedups for free, since such a cache would be valid for every commit.

The downsides are:

 - your cache is potentially bigger, since you are caching the score of
   every pair you look at, instead of just "good" pairs (OTOH, you are
   not doing a per-commit cache, which helps reduce the size)

 - you can still "lie" about a score to pre-seed imported SVN renames,
   but such lying will actually apply to all commits.

-Peff

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:45:37

On 11/8/08, Jeff King [off-list ref] wrote:
On Sat, Nov 08, 2008 at 10:24:10AM +0100, Yann Dirson wrote:

 > Well, it could be that an out-of-pathspec pair would have a better
 > score than an in-pathspec one.  Maybe cache recording should be turned
 > off when doing pathspec limitation ?


One thing I notice is that the cache works at the level of "here is the
 best rename for this commit." Maybe it could go down a level and say
 "here is the inexact rename score between these blobs". Then you would
 still find the best score between two blobs each time, but save the
 really computationally intensive part (which is comparing the actual
 _content_ of the blobs).
 That should work in the face of path limiting or any other option,
 because it is caching something immutable: this is the similarity score
 between two pieces of content. And then you get arbitrary tree-to-tree
 speedups for free, since such a cache would be valid for every commit.
I did that and realized the cost was not from each diff, in
--find-copies-harder case, but from the number of diffs you had to do.
Even with exact matching on linux-2.6.git, it could take significant
time (it was about 5 minutes in no-cache case, 1 minute without exact
match cache, and less than 1 sec if everything is cached).
 The downsides are:

  - your cache is potentially bigger, since you are caching the score of
   every pair you look at, instead of just "good" pairs (OTOH, you are
   not doing a per-commit cache, which helps reduce the size)
It is huge if you accidentially add --find-copies-harder to your
command, considering that every new file will be compared against
every files in tree (about 25k).
  - you can still "lie" about a score to pre-seed imported SVN renames,
   but such lying will actually apply to all commits.
-- 
Duy

Re: [PATCH 1/2] diffcore-rename: support rename cache

From: Jeff King <hidden>
Date: 2016-06-15 22:45:37

On Sat, Nov 08, 2008 at 07:00:10PM +0700, Nguyen Thai Ngoc Duy wrote:
quoted
 The downsides are:

  - your cache is potentially bigger, since you are caching the score of
   every pair you look at, instead of just "good" pairs (OTOH, you are
   not doing a per-commit cache, which helps reduce the size)
It is huge if you accidentially add --find-copies-harder to your
command, considering that every new file will be compared against
every files in tree (about 25k).
Hmm, yeah. I was thinking you might be able to do some kind of cut-off
on the caching (i.e., don't bother storing anything that didn't come
close). But you can't safely assume that because an entry isn't there,
it isn't worth seeing (since it might also just not have been computed
yet). You could still organize by commit, and then each commit is either
fully computed or not. But then you still have a pathspec problem.

One thing you could do is just compute the rename score between all
pairs, even if a pathspec is given, limit it to values over "0.5" (or
something low, but that eliminates the totally uninteresting cases), and
then store that as the complete cache for that commit (or tree pair, if
you want to support that).

Then you would have the full information and could do an arbitrary
pathspec limit on it. If you wanted to set the rename threshold below
0.5, then we would have to recompute without the cache (but in practice,
that should be rare).

The real downside is that you pay for the whole-tree detection when you
have asked for a pathspec (but only the first time, after which you can
always generate from cache).

Just thinking out loud...

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help