On Fri, Apr 06, 2018 at 12:34:35PM +0200, Anders Selhammer wrote:
> +void stats_combine(struct stats *to, struct stats *from)
> +{
> +     if (!from->num) {
> +             return;
> +     }
> +     to->min = (to->num && to->min < from->min) ? to->min : from->min;
> +     to->max = (to->num && to->max > from->max) ? to->max : from->max;
> +     to->mean = (to->mean * to->num + from->mean * from->num) /
> +                (to->num + from->num);
> +     to->num          += from->num;
> +     to->sum_sqr      += from->sum_sqr;
> +     to->sum_diff_sqr += from->sum_diff_sqr;
> +}

This is going to require more work than the straightforward approach
of just feeding the current sample value into two sets of statistics,
one for the current 15 minute period and one for the current 24 hour
period.

Looking at Annex M, it is pretty specific about the intervals:

- moving 24 hour window of 15 minute non-overlapping sub-windows
- current and previous 24 hour non-overlapping intervals

Your implementation, with its lists of statistics, malloc'ed and
freed, inserted and deleted, is way too complicated.  All of the list
management and allocation error handling simply does a lot of work for
no benefit.  Let me suggest an easier way.

Introduce a moving window of sets of statistics:

        struct stats_series {
                int length;
                int index;
                int count;
                struct stats *stats;
        };

with methods like:

        struct stats_series *quarter_hour, *daily;

        quarter_hour = stats_series_create(96);
        daily = stats_series_create(2);

        stats_series_add_value(s, value)
        {
                stats_add_value(s->stats[s->index], value);
        }

        stats_series_advance(s)
        {
                s->index = (1 + s->index) % s->length;
                stats_reset(s->stats[s->index]);
                if (s->count < s->length) {
                        s->count++;
                }
        }

Then your main code is as simple as:

        void clock_path_delay(struct clock *c, tmv_t req, tmv_t rx)
        {
                tsproc_up_ts(c->tsproc, req, rx);
                if (c->performance_monitoring) {
                        x = tmv_dbl(tmv_sub(rx, req));
                        stats_series_add_value(c->qhour[SLAVE_MASTER_DELAY], x);
                        stats_series_add_value(c->daily[SLAVE_MASTER_DELAY], x);
                }
                ...
        }

        static void clock_pm_event(struct clock *c)
        {
                for (i = 0; i < N_SERIES; i++) {
                        stats_series_advance(c->qhour[i]);
                }
                counter++;
                if (counter == 96) {
                        for (i = 0; i < N_SERIES; i++) {
                                stats_series_advance(c->daily[i]);
                        }
                        counter = 0;
                }
        }

No more lists to manage.
No more malloc/free. (well, only once at the start)

See?

Anyhow, we can talk more about the design, but I really want to know
how these statistics are going to be reported before getting too far
along.

Thanks,
Richard


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to