[FFmpeg-devel] [PATCH 1/2] avcodec/fft_template: improve performance of the ff_fft_init in fft_template

[Steven Liu]

> On Dec 16, 2018, at 21:47, Moritz Barsnick <barsnick at gmx.net> wrote:
> 
> On Sun, Dec 16, 2018 at 21:19:17 +0800, Steven Liu wrote:
>> move the two if condition out of the loop, that can less n-1 times than condition
>> in loop.
> [...]
>>             k = -split_radix_permutation(i, n, s->inverse) & (n-1);
>> -            if (s->revtab)
>>                 s->revtab[k] = j;
>> -            if (s->revtab32)
>> -                s->revtab32[k] = j;
>>         }
> 
> Does this really improve performance? You only save one if-check per
> loop (as only one of s->revtab and s->revtab32 is defined). Benchmarks?
> 
>> +            if (s->revtab32) {
>> +                for(i=0; i<n; i++) {
>> +                    int k;
>> +                    j = i;
>> +                    if (s->fft_permutation == FF_FFT_PERM_SWAP_LSBS)
>> +                        j = (j&~3) | ((j>>1)&1) | ((j<<1)&2);
>> +                        k = -split_radix_permutation(i, n, s->inverse) & (n-1);
>> +                s->revtab32[k] = j;
>> +            }
>> +            }
> 
> On the other hand, all the code is duplicated. Is there a more elegant
> way to do this?

Hi Moritz,

For example:

Original logic:
int n = 32;
for (i = 0 ; i < n; i++) {
    if (check1) do_something;
    if (check2) do_something;
}

If check1 is true and check2 is false, this loop 32 times, check condition 64 times;

After this patch:
int n = 32;
if (check1)
for (i = 0 ; i < n; i++) {
    do_something;
}
if (check2)
for (i = 0 ; i < n; i++) {
    do_something;
}
If check1 is true and check2 is false, this loop 32 times, check condition  2 times

And there should only one of the two condition is true,
because the s->revtab or s->revtab32 is allocation at begin of ff_fft_init,
and if nbits <= 16 the s->revtab maybe true else s->revtab32 is true.

So i modify it here.

I think maybe have more elegant way to do this if this logic is ok.



> 
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Thanks
Steven