[bitbake-devel][PATCH 1/2] runqueue: fix PSI check calculation

[bitbake-devel][PATCH 1/2] runqueue: fix PSI check calculation

[Chen Qi]

The current PSI check calculation does not take into consideration
the possibility of the time interval between last check and current
check being much larger than 1s. In fact, the current behavior does
not match what the manual says about BB_PRESSURE_MAX_XXX, even if
the value is set to upper limit, 1000000, we still get many blocks
on new task launch. The difference between 'total' should be divided
by the time interval if it's larger than 1s.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 bitbake/lib/bb/runqueue.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
index e629ab7e7b..02f1474540 100644
--- a/bitbake/lib/bb/runqueue.py
+++ b/bitbake/lib/bb/runqueue.py
@@ -198,15 +198,20 @@ class RunQueueScheduler(object):
                 curr_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
                 curr_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
                 curr_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
-                exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) > self.rq.max_cpu_pressure
-                exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) > self.rq.max_io_pressure
-                exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) > self.rq.max_memory_pressure
                 now = time.time()
-                if now - self.prev_pressure_time > 1.0:
+                tdiff = now - self.prev_pressure_time
+                if tdiff > 1.0:
+                    exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) / tdiff > self.rq.max_cpu_pressure
+                    exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) / tdiff > self.rq.max_io_pressure
+                    exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) / tdiff > self.rq.max_memory_pressure
                     self.prev_cpu_pressure = curr_cpu_pressure
                     self.prev_io_pressure = curr_io_pressure
                     self.prev_memory_pressure = curr_memory_pressure
                     self.prev_pressure_time = now
+                else:
+                    exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) > self.rq.max_cpu_pressure
+                    exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) > self.rq.max_io_pressure
+                    exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) > self.rq.max_memory_pressure
             return (exceeds_cpu_pressure or exceeds_io_pressure or exceeds_memory_pressure)
         return False
 
-- 
2.37.1

[bitbake-devel][PATCH 2/2] runqueue.py: use 'full' line for PSI check

[Chen Qi]

According to kernel PSI doc[1], for /proc/pressure/* interface files,
the first line is the 'some' line and the second line is the 'full'
line.

Quoting from the doc:
"""
The "some" line indicates the share of time in which at least some tasks
are stalled on a given resource.

The "full" line indicates the share of time in which all non-idle tasks
are stalled on a given resource simultaneously. In this state actual CPU
cycles are going to waste, and a workload that spends extended time in this
state is considered to be thrashing. This has severe impact on performance,
and it’s useful to distinguish this situation from a state where some tasks
are stalled but the CPU is still doing productive work.
"""

We can see that the 'full' line is a better measurement to check if things are
slowed down as a whole.

Also, the /proc/pressure/cpu's 'full' line may not be available on some systems,
so fall back to the 'some' line.

[1] https://www.kernel.org/doc/html/latest/accounting/psi.html

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 bitbake/lib/bb/runqueue.py | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
index 02f1474540..e3198dcdeb 100644
--- a/bitbake/lib/bb/runqueue.py
+++ b/bitbake/lib/bb/runqueue.py
@@ -173,10 +173,13 @@ class RunQueueScheduler(object):
                 with open("/proc/pressure/cpu") as cpu_pressure_fds, \
                     open("/proc/pressure/io") as io_pressure_fds, \
                     open("/proc/pressure/memory") as memory_pressure_fds:
-
-                    self.prev_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
-                    self.prev_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
-                    self.prev_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
+                    cpu_lines = cpu_pressure_fds.readlines()
+                    if len(cpu_lines) == 1:
+                        self.prev_cpu_pressure = cpu_lines[0].split()[4].split("=")[1]
+                    else:
+                        self.prev_cpu_pressure = cpu_lines[1].split()[4].split("=")[1]
+                    self.prev_io_pressure = io_pressure_fds.readlines()[1].split()[4].split("=")[1]
+                    self.prev_memory_pressure = memory_pressure_fds.readlines()[1].split()[4].split("=")[1]
                     self.prev_pressure_time = time.time()
                 self.check_pressure = True
             except:
@@ -194,10 +197,14 @@ class RunQueueScheduler(object):
             with open("/proc/pressure/cpu") as cpu_pressure_fds, \
                 open("/proc/pressure/io") as io_pressure_fds, \
                 open("/proc/pressure/memory") as memory_pressure_fds:
-                # extract "total" from /proc/pressure/{cpu|io}
-                curr_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
-                curr_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
-                curr_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
+                # extract "total" from /proc/pressure/{cpu|io|memory}
+                cpu_lines = cpu_pressure_fds.readlines()
+                if len(cpu_lines) == 1:
+                    curr_cpu_pressure = cpu_lines[0].split()[4].split("=")[1]
+                else:
+                    curr_cpu_pressure = cpu_lines[1].split()[4].split("=")[1]
+                curr_io_pressure = io_pressure_fds.readlines()[1].split()[4].split("=")[1]
+                curr_memory_pressure = memory_pressure_fds.readlines()[1].split()[4].split("=")[1]
                 now = time.time()
                 tdiff = now - self.prev_pressure_time
                 if tdiff > 1.0:
-- 
2.37.1

Re: [bitbake-devel][PATCH 1/2] runqueue: fix PSI check calculation

[Randy MacLeod]

[-- Attachment #1: Type: text/plain, Size: 4222 bytes --]

On 2023-04-06 23:07, Chen Qi via lists.openembedded.org wrote:
> The current PSI check calculation does not take into consideration
> the possibility of the time interval between last check and current
> check being much larger than 1s. In fact, the current behavior does
> not match what the manual says about BB_PRESSURE_MAX_XXX, even if
> the value is set to upper limit, 1000000, we still get many blocks
> on new task launch. The difference between 'total' should be divided
> by the time interval if it's larger than 1s.


Yes!
I had a patch  to do this but wanted to write a test case using stress .

Anyway, it's clearly better and won't allow the occasional new job to
slip in when the time diff is small and the pressure hasn't been 
recalculated
by the kernel. It will mean that pressure regulated builds may be a bit 
slower
but I doubt we'll even be able to measure that.

Thanks Qi,

../Randy


>
> Signed-off-by: Chen Qi<Qi.Chen@windriver.com>
> ---
>   bitbake/lib/bb/runqueue.py | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
> index e629ab7e7b..02f1474540 100644
> --- a/bitbake/lib/bb/runqueue.py
> +++ b/bitbake/lib/bb/runqueue.py
> @@ -198,15 +198,20 @@ class RunQueueScheduler(object):
>                   curr_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
>                   curr_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
>                   curr_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
> -                exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) > self.rq.max_cpu_pressure
> -                exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) > self.rq.max_io_pressure
> -                exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) > self.rq.max_memory_pressure
>                   now = time.time()
> -                if now - self.prev_pressure_time > 1.0:
> +                tdiff = now - self.prev_pressure_time
> +                if tdiff > 1.0:
> +                    exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) / tdiff > self.rq.max_cpu_pressure
> +                    exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) / tdiff > self.rq.max_io_pressure
> +                    exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) / tdiff > self.rq.max_memory_pressure
>                       self.prev_cpu_pressure = curr_cpu_pressure
>                       self.prev_io_pressure = curr_io_pressure
>                       self.prev_memory_pressure = curr_memory_pressure
>                       self.prev_pressure_time = now
> +                else:
> +                    exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) > self.rq.max_cpu_pressure
> +                    exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) > self.rq.max_io_pressure
> +                    exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) > self.rq.max_memory_pressure
>               return (exceeds_cpu_pressure or exceeds_io_pressure or exceeds_memory_pressure)
>           return False
>   
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14682):https://lists.openembedded.org/g/bitbake-devel/message/14682
> Mute This Topic:https://lists.openembedded.org/mt/98118922/3616765
> Group Owner:bitbake-devel+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/bitbake-devel/unsub  [randy.macleod@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

-- 
# Randy MacLeod
# Wind River Linux

[-- Attachment #2: Type: text/html, Size: 5483 bytes --]

Re: [bitbake-devel][PATCH 2/2] runqueue.py: use 'full' line for PSI check

[Randy MacLeod]

[-- Attachment #1: Type: text/plain, Size: 5061 bytes --]

On 2023-04-06 23:07, Chen Qi via lists.openembedded.org wrote:
> According to kernel PSI doc[1], for /proc/pressure/* interface files,
> the first line is the 'some' line and the second line is the 'full'
> line.
>
> Quoting from the doc:
> """
> The "some" line indicates the share of time in which at least some tasks
> are stalled on a given resource.
>
> The "full" line indicates the share of time in which all non-idle tasks
> are stalled on a given resource simultaneously. In this state actual CPU
> cycles are going to waste, and a workload that spends extended time in this
> state is considered to be thrashing. This has severe impact on performance,
> and it’s useful to distinguish this situation from a state where some tasks
> are stalled but the CPU is still doing productive work.
> """
>
> We can see that the 'full' line is a better measurement to check if things are
> slowed down as a whole.
>
> Also, the /proc/pressure/cpu's 'full' line may not be available on some systems,
> so fall back to the 'some' line.
>
> [1]https://www.kernel.org/doc/html/latest/accounting/psi.html

NACK...

I'd need to dig up or reproduce some test cases but I believe that
we should NOT use 'full' instead of 'some'. We want to have a *lower* 
threshold

to defer work when the system resources are experiencing pressure.
We don't want to wait for the system to get to a state where all of the
resources are under pressure.


There may be use cases involving containers where the 'full' PSI info
is more useful or even essential but need a motivating example.


../Randy

>
> Signed-off-by: Chen Qi<Qi.Chen@windriver.com>
> ---
>   bitbake/lib/bb/runqueue.py | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
> index 02f1474540..e3198dcdeb 100644
> --- a/bitbake/lib/bb/runqueue.py
> +++ b/bitbake/lib/bb/runqueue.py
> @@ -173,10 +173,13 @@ class RunQueueScheduler(object):
>                   with open("/proc/pressure/cpu") as cpu_pressure_fds, \
>                       open("/proc/pressure/io") as io_pressure_fds, \
>                       open("/proc/pressure/memory") as memory_pressure_fds:
> -
> -                    self.prev_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
> -                    self.prev_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
> -                    self.prev_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
> +                    cpu_lines = cpu_pressure_fds.readlines()
> +                    if len(cpu_lines) == 1:
> +                        self.prev_cpu_pressure = cpu_lines[0].split()[4].split("=")[1]
> +                    else:
> +                        self.prev_cpu_pressure = cpu_lines[1].split()[4].split("=")[1]
> +                    self.prev_io_pressure = io_pressure_fds.readlines()[1].split()[4].split("=")[1]
> +                    self.prev_memory_pressure = memory_pressure_fds.readlines()[1].split()[4].split("=")[1]
>                       self.prev_pressure_time = time.time()
>                   self.check_pressure = True
>               except:
> @@ -194,10 +197,14 @@ class RunQueueScheduler(object):
>               with open("/proc/pressure/cpu") as cpu_pressure_fds, \
>                   open("/proc/pressure/io") as io_pressure_fds, \
>                   open("/proc/pressure/memory") as memory_pressure_fds:
> -                # extract "total" from /proc/pressure/{cpu|io}
> -                curr_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
> -                curr_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
> -                curr_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
> +                # extract "total" from /proc/pressure/{cpu|io|memory}
> +                cpu_lines = cpu_pressure_fds.readlines()
> +                if len(cpu_lines) == 1:
> +                    curr_cpu_pressure = cpu_lines[0].split()[4].split("=")[1]
> +                else:
> +                    curr_cpu_pressure = cpu_lines[1].split()[4].split("=")[1]
> +                curr_io_pressure = io_pressure_fds.readlines()[1].split()[4].split("=")[1]
> +                curr_memory_pressure = memory_pressure_fds.readlines()[1].split()[4].split("=")[1]
>                   now = time.time()
>                   tdiff = now - self.prev_pressure_time
>                   if tdiff > 1.0:
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14683):https://lists.openembedded.org/g/bitbake-devel/message/14683
> Mute This Topic:https://lists.openembedded.org/mt/98118923/3616765
> Group Owner:bitbake-devel+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/bitbake-devel/unsub  [randy.macleod@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

-- 
# Randy MacLeod
# Wind River Linux

[-- Attachment #2: Type: text/html, Size: 6717 bytes --]

Re: [bitbake-devel][PATCH 1/2] runqueue: fix PSI check calculation

[contrib]

[-- Attachment #1: Type: text/plain, Size: 4972 bytes --]



> On Apr 7, 2023, at 11:02 AM, Randy MacLeod <Randy.MacLeod@windriver.com> wrote:
> 
> On 2023-04-06 23:07, Chen Qi via lists.openembedded.org <http://lists.openembedded.org/> wrote:
>> The current PSI check calculation does not take into consideration
>> the possibility of the time interval between last check and current
>> check being much larger than 1s. In fact, the current behavior does
>> not match what the manual says about BB_PRESSURE_MAX_XXX, even if
>> the value is set to upper limit, 1000000, we still get many blocks
>> on new task launch. The difference between 'total' should be divided
>> by the time interval if it's larger than 1s.

Some ideas about this:

The original 1-second interval is to prevent the time interval between two pressure checks too
large, but now we can detect longer time intervals and act accordingly, we probably can do
the same thing for short time intervals.

This may also reduce the build time a bit while having similar CPU pressure - a lot of half a
second can stack up significantly.

It might be worth testing both solutions.

Zheng
> 
> Yes!
> I had a patch  to do this but wanted to write a test case using stress .
> 
> Anyway, it's clearly better and won't allow the occasional new job to
> slip in when the time diff is small and the pressure hasn't been recalculated
> by the kernel. It will mean that pressure regulated builds may be a bit slower
> but I doubt we'll even be able to measure that.
> 
> Thanks Qi,
> 
> ../Randy
> 
> 
> 
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com> <mailto:Qi.Chen@windriver.com>
>> ---
>>  bitbake/lib/bb/runqueue.py | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>> 
>> diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
>> index e629ab7e7b..02f1474540 100644
>> --- a/bitbake/lib/bb/runqueue.py
>> +++ b/bitbake/lib/bb/runqueue.py
>> @@ -198,15 +198,20 @@ class RunQueueScheduler(object):
>>                  curr_cpu_pressure = cpu_pressure_fds.readline().split()[4].split("=")[1]
>>                  curr_io_pressure = io_pressure_fds.readline().split()[4].split("=")[1]
>>                  curr_memory_pressure = memory_pressure_fds.readline().split()[4].split("=")[1]
>> -                exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) > self.rq.max_cpu_pressure
>> -                exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) > self.rq.max_io_pressure
>> -                exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) > self.rq.max_memory_pressure
>>                  now = time.time()
>> -                if now - self.prev_pressure_time > 1.0:
>> +                tdiff = now - self.prev_pressure_time
>> +                if tdiff > 1.0:
>> +                    exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) / tdiff > self.rq.max_cpu_pressure
>> +                    exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) / tdiff > self.rq.max_io_pressure
>> +                    exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) / tdiff > self.rq.max_memory_pressure
>>                      self.prev_cpu_pressure = curr_cpu_pressure
>>                      self.prev_io_pressure = curr_io_pressure
>>                      self.prev_memory_pressure = curr_memory_pressure
>>                      self.prev_pressure_time = now
>> +                else:
>> +                    exceeds_cpu_pressure =  self.rq.max_cpu_pressure and (float(curr_cpu_pressure) - float(self.prev_cpu_pressure)) > self.rq.max_cpu_pressure
>> +                    exceeds_io_pressure =  self.rq.max_io_pressure and (float(curr_io_pressure) - float(self.prev_io_pressure)) > self.rq.max_io_pressure
>> +                    exceeds_memory_pressure = self.rq.max_memory_pressure and (float(curr_memory_pressure) - float(self.prev_memory_pressure)) > self.rq.max_memory_pressure
>>              return (exceeds_cpu_pressure or exceeds_io_pressure or exceeds_memory_pressure)
>>          return False
>>  
>> 
>> 
> 
> -- 
> # Randy MacLeod
> # Wind River Linux
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14684): https://lists.openembedded.org/g/bitbake-devel/message/14684
> Mute This Topic: https://lists.openembedded.org/mt/98118922/7355053
> Group Owner: bitbake-devel+owner@lists.openembedded.org <mailto:bitbake-devel+owner@lists.openembedded.org>
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [contrib@zhengqiu.net <mailto:contrib@zhengqiu.net>]
> -=-=-=-=-=-=-=-=-=-=-=-


[-- Attachment #2: Type: text/html, Size: 17197 bytes --]