[Frugalware-devel] pacman_parse_config patch

James Buren ryuo at frugalware.org
Mon Dec 28 15:36:47 CET 2009


> On Sun, Dec 27, 2009 at 03:31:29AM -0600, James Buren
> <ryuo at frugalware.org> wrote:
>> I've created a patch that I feel is a tad faster than the old code it
>> replaces.
>
> That's not about feelings, do benchmarks if you want to prove that new
> code is faster.
>
Alright, next time I'll perform tests to check if an "optimization" change
will actually do anything.
>> If you agree, please apply this patch. It replaces a call to
>> strlen(line) with line[0] == '\0' to check for an empty line. This makes
>> sense because if the line truely is empty, it will be 0 length, and if
>> so,
>> the first entry will have to be the null terminator ('\0'), so why not
>> just check for this and avoid the function call overhead?
>
> You obviously did not compare the asm output before and after your
> patch:
>
Well, I didn't know you could do that. How do you do that so I can perform
that test myself?
> ~/git/pacman-g2/lib/libpacman$ diff -u pacman.s.orig pacman.s
> --- pacman.s.orig       2009-12-27 18:25:36.000000000 +0100
> +++ pacman.s    2009-12-27 18:26:01.000000000 +0100
> @@ -11665,8 +11665,7 @@
>         movl    %eax, (%esp)
>         call    _pacman_strtrim
>         .loc 1 1204 0
> -       leal    -4153(%ebp), %eax
> -       movzbl  (%eax), %eax
> +       movzbl  -4153(%ebp), %eax
>         testb   %al, %al
>         je      .L411
>         movzbl  -4153(%ebp), %eax
>
> As you can see the compiler already optimized away the function call, so
> your patch did not really avoided any function call. ;)
>
> Anyway it made the code a bit more readable - the old version might
> suggest that the author thought strlen() accepts NULL as an argument,
> which is not the case - so I applied it.
> _______________________________________________
> Frugalware-devel mailing list
> Frugalware-devel at frugalware.org
> http://frugalware.org/mailman/listinfo/frugalware-devel
>




More information about the Frugalware-devel mailing list