[ovs-dev] [PATCH ovs] lib: export calc_percentile function

[Aaron Conole]

Xavier Simonart <xsimonar at redhat.com> writes:

> export calc_percentile function (and percentile struct) so that
> it can be used in other libraries such as OVN.
>
> Signed-off-by: Xavier Simonart <xsimonar at redhat.com>
> ---

What is the intent to use this in other libraries?  It would be nice to
understand why just running the existing stopwatch API doesn't work
(maybe you cannot start samples).  AFAIK, OVN uses the stopwatch API
just fine?

Perhaps it could make sense to expose the functionality in a different
fashion like:

  void stopwatch_add_sample(const char *, unsigned long long);

This seems a useful API and doesn't expose all of the internal
information, but I don't know if it really is needed.  Can you expand
why you need calc_percentile exposed?

>  lib/stopwatch.c | 24 +-----------------------
>  lib/stopwatch.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/lib/stopwatch.c b/lib/stopwatch.c
> index f5602163b..003c3a05f 100644
> --- a/lib/stopwatch.c
> +++ b/lib/stopwatch.c
> @@ -35,25 +35,6 @@ struct average {
>      double alpha;   /* Weight given to new samples */
>  };
>  
> -#define MARKERS 5
> -
> -/* Number of samples to collect before reporting P-square calculated
> - * percentile
> - */
> -#define P_SQUARE_MIN 50
> -
> -/* The naming of these fields is based on the naming used in the
> - * P-square algorithm paper.
> - */
> -struct percentile {
> -    int n[MARKERS];
> -    double n_prime[MARKERS];
> -    double q[MARKERS];
> -    double dn[MARKERS];
> -    unsigned long long samples[P_SQUARE_MIN];
> -    double percentile;
> -};
> -
>  struct stopwatch {
>      enum stopwatch_units units;
>      unsigned long long n_samples;
> @@ -107,10 +88,7 @@ comp_samples(const void *left, const void *right)
>      return *right_d > *left_d ? -1 : *right_d < *left_d;
>  }
>  
> -/* Calculate the percentile using the P-square algorithm. For more
> - * information, see https://www1.cse.wustl.edu/~jain/papers/ftp/psqr.pdf
> - */
> -static void
> +void
>  calc_percentile(unsigned long long n_samples, struct percentile *pctl,
>                  unsigned long long new_sample)
>  {
> diff --git a/lib/stopwatch.h b/lib/stopwatch.h
> index 91abd64e4..efb9a9e8a 100644
> --- a/lib/stopwatch.h
> +++ b/lib/stopwatch.h
> @@ -36,6 +36,32 @@ struct stopwatch_stats {
>                                      (alpha 0.01). */
>  };
>  
> +#define MARKERS 5
> +
> +/* Number of samples to collect before reporting P-square calculated
> + * percentile
> + */
> +#define P_SQUARE_MIN 50
> +
> +/* The naming of these fields is based on the naming used in the
> + * P-square algorithm paper.
> + */
> +struct percentile {
> +    int n[MARKERS];
> +    double n_prime[MARKERS];
> +    double q[MARKERS];
> +    double dn[MARKERS];
> +    unsigned long long samples[P_SQUARE_MIN];
> +    double percentile;
> +};
> +
> +/* Calculate the percentile using the P-square algorithm. For more
> + * information, see https://www1.cse.wustl.edu/~jain/papers/ftp/psqr.pdf
> + */
> +void
> +calc_percentile(unsigned long long n_samples, struct percentile *pctl,
> +                unsigned long long new_sample);
> +
>  /* Create a new stopwatch.
>   * The "units" are not used for any calculations but are printed when
>   * statistics are requested.