Re: [RFT][PATCH v1 5/5] cpuidle: menu: Avoid discarding useful information

[Rafael J. Wysocki]

On Mon, Feb 17, 2025 at 2:39 PM Christian Loehle
<christian.loehle@xxxxxxx> wrote:
>
> On 2/6/25 14:29, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > When giving up on making a high-confidence prediction,
> > get_typical_interval() always returns UINT_MAX which means that the
> > next idle interval prediction will be based entirely on the time till
> > the next timer.  However, the information represented by the most
> > recent intervals may not be completely useless in those cases.
> >
> > Namely, the largest recent idle interval is an upper bound on the
> > recently observed idle duration, so it is reasonable to assume that
> > the next idle duration is unlikely to exceed it.  Moreover, this is
> > still true after eliminating the suspected outliers if the sample
> > set still under consideration is at least as large as 50% of the
> > maximum sample set size.
> >
> > Accordingly, make get_typical_interval() return the current maximum
> > recent interval value in that case instead of UINT_MAX.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > ---
> >  drivers/cpuidle/governors/menu.c |   13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > --- a/drivers/cpuidle/governors/menu.c
> > +++ b/drivers/cpuidle/governors/menu.c
> > @@ -190,8 +190,19 @@
> >        * This can deal with workloads that have long pauses interspersed
> >        * with sporadic activity with a bunch of short pauses.
> >        */
> > -     if ((divisor * 4) <= INTERVALS * 3)
> > +     if (divisor * 4 <= INTERVALS * 3) {
> > +             /*
> > +              * If there are sufficiently many data points still under
> > +              * consideration after the outliers have been eliminated,
> > +              * returning without a prediction would be a mistake because it
> > +              * is likely that the next interval will not exceed the current
> > +              * maximum, so return the latter in that case.
> > +              */
> > +             if (divisor >= INTERVALS / 2)
> > +                     return max;
> > +
> >               return UINT_MAX;
> > +     }
> >
> >       /* Update the thresholds for the next round. */
> >       if (avg - min > max - avg)
> >
>
> You might want to amend the description at the top of menu.c then given that
> this now returns something without any meaning in a statistical significance
> way. Similar to admin-guide doc.

OK, I'll send a documentation update patch on top of this.

> As reported by the tests, this does improve performance a lot in scenarios of
> short intervals (where passing the statistical test is hard).

Yes, that pretty much is what the SPECjbb critical-jOPS improvement means.

> Teo exploits the idle state residencies for this (i.e. as long as they fall
> into the same bin, they are equal for our means), this can be viewed as the
> menu equivalent to it, without relying on idle states.
>
> Reviewed-by: Christian Loehle <christian.loehle@xxxxxxx>

Thank you!