Re: [PATCH] cleans up builtin-mv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 18 Aug 2006, Josef Weidendorfer wrote:

> Can you explain your reasoning in more detail?
> C compiles to native code. Bash itself first has to
> parse the script. How on earth can this be faster than native code?
> 
> I simply do not understand this discussion about implementation language,
> especially in this case where most of the work is probably done changing
> git's index (the add's and rm's of tree entries). Of course it could have
> been done in /bin/sh, but it wasn't (it started as git-rename.perl).
> 

It's not faster than native code, it's faster than the current 
implementation of builtin-mv.  And when you're working with terabytes of 
data like I am, I would prefer to use something fast.

> Hmm... I suppose Dscho's argument was that this "... >=0" is a standard way
> to code an assignment inside of an expression.
> 

That argument is unjustified since the only advantage of putting it in an 
expression is to not evaluate it if the lstat failed (and not fail by 
means of ENOENT because copy_pathspec guarantees all results have strlen > 
0).  So "length" is set unnecessarily only if lstat fails which should 
never happen if copy_pathspec does it's job with correct arguments.  I'm 
willing to sacrifice that if the _working_ case is faster (and 
significantly faster) especially since this is an iteration and is 
directly tied to the command's speed.

The comparison to 0 simply creates a cmpl $0, x(%ebp) that will always be 
true and a jump to a label that never needed to exist.

Likewise, the additional declaration and initilization of a completely 
redundant case call to strlen slows us down FOR EVERY ITERATION OF THE 
MOVE:
	movl	%eax, x(%ebp)
	movl	(x*2)(%ebp), %eax
	movl	$-1, %ecx
	movl	%eax, (x*4)(%ebp)
	movb	%0, %al
	cld
	movl	(x*4)(%ebp), %edi
	repnz
	scasb
	movl	%ecx, %eax
	notl	%eax
	decl	%eax

And then repeat that same call again because of its miscall later on when 
it's already been assigned to a variable.

		David
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]