git sha: [email protected]
Author: Gaetan Rivet <[email protected]>
Subject: [RFC PATCH 03/12] histogram: Store sum of all measures
This patch adds a sum field to the histogram structure to track the
aggregate sum of sampled values for computing averages.
> diff --git a/lib/histogram.c b/lib/histogram.c
> index 85bb3661c3..f066a25905 100644
> --- a/lib/histogram.c
> +++ b/lib/histogram.c
> @@ -91,4 +91,5 @@ histogram_clear(struct histogram *hist)
> for (i = 0; i < HISTOGRAM_N_BINS; i++) {
> hist->bin[i] = 0;
> }
> + hist->sum = 0;
> }
> diff --git a/lib/histogram.h b/lib/histogram.h
> index 8c2a9ea5c7..c07707cb77 100644
> --- a/lib/histogram.h
> +++ b/lib/histogram.h
> @@ -35,11 +35,13 @@ extern "C" {
> struct histogram {
> uint32_t wall[HISTOGRAM_N_BINS];
> uint64_t bin[HISTOGRAM_N_BINS];
> + uint64_t sum;
> };
>
> static inline void
> histogram_add_sample(struct histogram *hist, uint32_t val)
> {
> + hist->sum += val;
Can this overflow when accumulating many large samples? The val parameter
is uint32_t but sum is uint64_t. With repeated additions of values near
UINT32_MAX, would the sum field eventually overflow?
> /* TODO: Can do better with binary search? */
> for (int i = 0; i < HISTOGRAM_N_BINS - 1; i++) {
> if (val <= hist->wall[i]) {
[ ... ]
Is the new sum field being initialized in histogram_init()? The patch
shows initialization in histogram_clear() but histogram_init() may also
need similar treatment to ensure the sum starts at zero for newly
created histograms.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
