[Ffmpeg-devel] [PATCH] optimize ff_eval

[Michael Niedermayer]

Hi

On Thu, Oct 26, 2006 at 08:45:02PM +0200, Oded Shimon wrote:
> I took the liberty of optimizing ff_eval for multiple runs by seperating 
> the parsing/compiling and runtime steps.
> 
> before:
> 97730 dezicycles in ff_eval, 1023 runs, 1 skips
> after:
> 13643 dezicycles in ff_parse, 1023 runs, 1 skips

great :)
what about the eval.o size?


> 
> 
> What should I do with this? Deperecate or keep old ff_eval? update current 
> calls for ff_eval to this new thing?

hmm, id say deprecate "now" and remove with the next major version increase
and update all in a seperate patch as soon as this is cleaned up reviewed and
commited


> 
> Patch is very obviously not cleaned up (there isn't even a free function 
> yet), just shows what I did...

understood, still some random comments below


[...]

> Index: libavcodec/eval.c
> ===================================================================
> --- libavcodec/eval.c	(revision 6797)
> +++ libavcodec/eval.c	(working copy)
> @@ -57,8 +57,6 @@
>      char **error;
>  } Parser;
>  
> -static double evalExpression(Parser *p);
> -
>  static int8_t si_prefixes['z' - 'E' + 1]={
>      ['y'-'E']= -24,
>      ['z'-'E']= -21,
> @@ -126,23 +124,64 @@
>      return 1;
>  }
>  
> -static double evalPrimary(Parser *p){
> -    double d, d2=NAN;
> +typedef struct expr_s expr_t;
> +struct expr_s {
> +    int type;
> +    double value; // is sign in other types
> +    int const_index;

> +    double (*func0)(double);
> +    double (*func1)(void *, double);
> +    double (*func2)(void *, double, double);

IMHO union or void* 


> +    expr_t * param[2];
> +};
> +
> +static double evalsquish(          double d           ) { return 1/(1+exp(4*d)); }
> +static double evalgauss (          double d           ) { return exp(-d*d/2)/sqrt(2*M_PI); }
> +static double evalmod   (void * a, double d, double d2) { return d - floor(d/d2)*d2; }
> +static double evalmax   (void * a, double d, double d2) { return d >  d2 ?   d : d2; }
> +static double evalmin   (void * a, double d, double d2) { return d <  d2 ?   d : d2; }
> +static double evalgt    (void * a, double d, double d2) { return d >  d2 ? 1.0 : 0.0; }
> +static double evalgte   (void * a, double d, double d2) { return d >= d2 ? 1.0 : 0.0; }
> +static double evaleq    (void * a, double d, double d2) { return d == d2 ? 1.0 : 0.0; }
> +static double evalpow   (void * a, double d, double d2) { return pow(d, d2); }
> +static double evalmul   (void * a, double d, double d2) { return d * d2; }
> +static double evaldiv   (void * a, double d, double d2) { return d / d2; }
> +static double evaladd   (void * a, double d, double d2) { return d + d2; }
> +
> +static double eval_expr(Parser * p, expr_t * e) {
> +    if (e->type == 0) return e->value;
> +    else switch(e->type) {
> +        case 1: return e->value * p->const_value[e->const_index];
> +        case 2: return e->value * e->func0(eval_expr(p, e->param[0]));
> +        case 3: return e->value * e->func1(p->opaque, eval_expr(p, e->param[0]));
> +        case 4: return e->value * e->func2(p->opaque, eval_expr(p, e->param[0]), eval_expr(p, e->param[1]));
> +        default: assert(0); return 0;
> +    }
> +}

now while this looks nice, i think it isnt optimal, what about putting all 
the "non user" functions in the switch/case? i think it might be faster
if not then iam fine with the above 


> +
> +static expr_t * parse_expr(Parser *p);
> +
> +static expr_t * parse_primary(Parser *p) {
> +    expr_t * d = calloc(1, sizeof(expr_t));

av_malloc()/av_free()

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is