Re: [PATCH] cleans up builtin-mv
- From
- David Rientjes <rientjes@google.com>
- Date
- Aug 18, 2006, 19:01 UTC
- Message-ID
- <Pine.LNX.4.63.0608181143040.30274@chino.corp.google.com>
- In-Reply-To
- <200608182035.47208.Josef.Weidendorfer@gmx.de>
On Fri, 18 Aug 2006, Josef Weidendorfer wrote:
Show 9 quoted lines
> 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