Re: [PATCH 3/3] Diff overhaul, adding the other half...

quoted

quoted

quoted

quoted

"LT" == Linus Torvalds [off-list ref] writes:

LT> I love how I can just say "oh, keep in mind that we might
LT> want to.." and 24 hours later you did it.

Nah, it's more like 40 hours, but yes I am here to serve your
wishes ;-).

LT> I'm also somewhat surprised by the fact that it even seems
LT> to be usable on the kernel tree:

Surprised?  Correctness-wise and/or performance-wise?

On Sat, 21 May 2005, Junio C Hamano wrote:

LT> I'm also somewhat surprised by the fact that it even seems
LT> to be usable on the kernel tree:

Surprised?  Correctness-wise and/or performance-wise?

Performance-wise. It seems to be quite usable, even doing just a plain
"git-whatchanged -C" on the kernel with no limits on what it does.

Now, all of the actual test-cases I looked at were actually parts of
patches where the source file _had_ been modified, so I didn't see a case
where it selected any of the random 17,000 files that were _not_ modified,
and I didn't double-check further than your commit message saying that it 
really does that, so..

		Linus

On Sat, 21 May 2005, Linus Torvalds wrote:

Now, all of the actual test-cases I looked at were actually parts of
patches where the source file _had_ been modified, so I didn't see a case
where it selected any of the random 17,000 files that were _not_ modified,
and I didn't double-check further than your commit message saying that it 
really does that, so..

Oh, I decided to double-check, and no, it doesn't actually do a full copy 
check for diff-tree. Only for diff-cache and diff-files.

Which is a sensible default, and I note that you sent a separate email for 
testing the extreme case. I'll try that out too, just for fun,

		Linus

On Sat, 21 May 2005, Linus Torvalds wrote:

Which is a sensible default, and I note that you sent a separate email for 
testing the extreme case. I'll try that out too, just for fun,

Hmm.. It's not working well. Not only does it take a lot of CPU time (do 
an fsck first to make sure you're not seekign the disk all over the 
place), but it "finds" lots of things like this:

	diff --git a/drivers/usb/misc/emi62_fw_m.h b/drivers/media/dvb/bt8xx/dst_ca.h
	similarity index 99%
	copy from drivers/usb/misc/emi62_fw_m.h
	copy to drivers/media/dvb/bt8xx/dst_ca.h
	--- a/drivers/usb/misc/emi62_fw_m.h
	+++ b/drivers/media/dvb/bt8xx/dst_ca.h
	@@ -1,8853 +1,58 @@
	 /*
	- * This file is generated from three different files, provided by Emagic.
	- */
	-/* generated Tue Jun 3 21:36:11 EEST 2003 */
	+       CA-driver for TwinHan DST Frontend/Card
 
which looks quite bogus (it they aren't similar at all, and the diff is 
huge).

I think that your similarity check has a tendency to do bad things if one 
of the files is huge: in this case we have

	torvalds@ppc970:~/v2.6/linux> wc -c drivers/media/dvb/bt8xx/dst_ca.h drivers/usb/misc/emi62_fw_m.h
	  1591 drivers/media/dvb/bt8xx/dst_ca.h
	795679 drivers/usb/misc/emi62_fw_m.h
	797270 total

and you consider them "similar", probably because it turns out that 
a delta that just removes everything is very small (it's just a "delete 
bytes x-y") so you compare that "small" delta to a "large total file" and 
you think it's an almost perfect match.

Now, for _renames_ that is actually half-way the right thing to do, but 
for copies, you should compare the size not to the _sum_ of the two files, 
but to just the size of the file that you generate.

But it's a fun example ;)

		Linus