Re: ??? Re: [PATCH/RFC v1 1/6] symlinks.c: small cleanup and optimisation
* Kjetil Barvik <barvik@broadpark.no> writes:
Show 8 quoted lines
> + /*
> + * Is the cached path string a substring of 'name', is 'name'
> + * a substring of the cached path string, or is 'name' and the
> + * cached path string the exact same string?
> + */
> + if (i >= max_len && ((i < len && name[i] == '/') ||
> + (i < cache.len && cache.path[i] == '/') ||
> + (len == cache.len))) {* Junio C Hamano <gitster@pobox.com> writes:
> As you described in your commit log message, what this really wants to
> check is (i == max_len). By saying (i >= max_len) you are losing
> readability, optimizing for compilers (that do not notice this)
When the compiler see the source line:
'while (i < max_len && name[i] == cache.path[i])'
it will, from what I know about compilers and machine instructions for
intel cpu's, generate the following pseudo instructions for this line
(before more compiler optimisation is done):
1 test !(i < max_len) /* which is (i >= max_len) */
2 jump-if-not-zero first_instruction_address_outside_the_loop
3 "some instructions to retrieve some memory locations and test
name[i] == cache.path[i]"
4 jump-if-not-zero instruction-address-to-continue-to-loop So, I thought that if I could use the exact same test as the compiler
have to generate for line 1, then I would maybe saved a test, and
maybe also a jump instruction.
Compiling with 'gcc -Os' (optimise for text segment size) I was able to
save 4 bytes for this file, which I think shows that gcc is able to
take advantage of this trick.
> and pessimizing for human readers (like me, who had to spend a few
> dozens of seconds to realize what you are doing, to speculate why you
> might have thought this would be a good idea, and writing this
> paragraph). I do not know if it is a good trade-off.
But, it was not easy to come up with a real test which shows a
noticeable time difference from this trick, so, I guess this is only a
trick for the kernel people (at most), so I revert it and will use
'==' for version 2, such that we keep a little better readability.
-- kjetil
ps! Compiling with '-march=core2 -O2 -g0 -s -fomit-frame-pointer', the
difference in text segment was 64 bytes when using this trick.