[FFmpeg-devel] [PATCH 2/4] avcodec/xpm: Minor speed increase for mod_strcspn() {string, reject}==0

[Jose Da Silva]

On February 24, 2021 05:56:17 AM Moritz Barsnick wrote:
> On Mon, Feb 22, 2021 at 20:32:14 -0800, Jose Da Silva wrote:
> > -    for (i = 0; string && string[i]; i++) {
> > +    if (string == 0)
> 
> "if (!string)" is the preferred style for pointers.

Works for me. Updated patches reworked for today's date.


> But I don't see the advantage - isn't the loop interrupted immediately
> anyway if string == NULL? (Same for the other loop you changed.)

Yes, ...but... let's now look inside the for loop with string != NULL

If we read 100 character before reaching reject, that is 100 extra tests.
Technically speaking, we could remove this test since the function is 
private to xpmdec.c, but if the function is useful enough to be globally 
used (instead of private here), it is worth having the extra check as a 
safety-check. one_test vs no_test is not a crushing impact on speed, but it 
is still better than loopin and testing "i" times.

The other loop is inside the first loop, so this effect is multiplied.
If we attempt to reject, say... ",}" then we test "reject!=0" three times
If we need to search through ten characters in string, that makes it about 
thirty tests. Take this outside the for loop, and that is now ten tests.

It is a minor speed improvement, and I thought it was worth resolving these 
low hanging fruit first. I wanted to avoid too much churn which is likely 
when submitting a larger set of patches :-)

In terms of churn - Hopefully the "if (!string)" was the only catch here.


> Inside the loops, the additional checks for validity of "string"
> seem redundant either way, though:
> 
>   while ( string && string[i] && string[i] != '\n' )

...well, if we are already paying attention to string&reject in the outside 
loops, we might as well go ahead and fix the same/similar things here too.
These are also minor speed improvements, and two fewer if/then/else in the 
compiled binary sense too.

Joe