Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified fixed cpu usage percentage #99

Merged
merged 3 commits into from
Aug 31, 2020
Merged

Modified fixed cpu usage percentage #99

merged 3 commits into from
Aug 31, 2020

Conversation

rrodriguez81
Copy link
Contributor

@rrodriguez81 rrodriguez81 commented Aug 29, 2020

Turbo should be enabled when one thread reach 100% of usage, in original version there was a fixed value of 25%, but this only works when there're 4 threads, in CPUs with more threads

Reference: #98

@rrodriguez81
Copy link
Contributor Author

Wait a bit please, I've detected that on charging state could be improved aswell. I'll do another pr in a few minutes.

@rrodriguez81
Copy link
Contributor Author

The commit has joined to previous PR.

@AdnanHodzic
Copy link
Owner

No worries, take your time, I'll thoroughly look into this in the morning.

@Red-Eyed
Copy link
Contributor

@AdnanHodzic I checked this PR:
I just loaded my single core on 100% by command: python -c "while True: pass" and it switches to turbo

@rrodriguez81
Copy link
Contributor Author

@Red-Eyed how many threads can handle your CPU? is your laptop in charging state? If there are only 4 is normal, but if the CPU can handle more than 4 CPUs is normal that the CPU load doesn't reach 25% (at least on an AMD Ryzen 3700U, because I haven't tested with an Intel CPU).

In new portable AMD CPUs with more than 4C/8T, with an only 25% usage the problem will more noticeable.

@Red-Eyed
Copy link
Contributor

Red-Eyed commented Aug 31, 2020

Hello @rrodriguez81

how many threads can handle your CPU?

Updated
8

is your laptop in charging state?

Discharging

@rrodriguez81
Copy link
Contributor Author

Hello @Red-Eyed :) ,

Not sure if because the CPU model, but in my case it didn't launch turbo. I used the stress tool for generate a 100% load:

sudo stress --cpu 1 --timeout 120

@Red-Eyed
Copy link
Contributor

Just tried sudo stress --cpu 1 --timeout 20 and it works like a charm: 20 secs with turbo boost: on than it switched to turbo boost: off
I have CPU: AMD Ryzen 5 3500U with Radeon Vega Mobile Gfx

@Red-Eyed
Copy link
Contributor

BTW, are you able to switch to turbo anyway?

@rrodriguez81
Copy link
Contributor Author

@Red-Eyed with my modified version works fine, with original version the turbo doesn't get activated unless I use two threads, for example:
sudo stress --cpu 2 --timeout 20

@Red-Eyed
Copy link
Contributor

@AdnanHodzic it seems that this PR should use this psutil.cpu_percent(percpu=True)
psutil.cpu_percent(percpu=True) will return list with cpu load per core

@Red-Eyed
Copy link
Contributor

@Red-Eyed with my modified version works fine, with original version the turbo doesn't get activated unless I use two threads, for example:
sudo stress --cpu 2 --timeout 20

Yeah, it seems that I got what u are trying to say...

@Red-Eyed
Copy link
Contributor

Red-Eyed commented Aug 31, 2020

@AdnanHodzic
I think that this code example should work correct:

if psutil.cpu_percent(percpu=False) >= 25.0 or isclose(max(psutil.cpu_percent(percpu=True)), 100):
    turbo(True)

My apologise, @rrodriguez81 this is your PR. Try the above code snippet
before using isclose
you need to add
from math import isclose

@AdnanHodzic
Copy link
Owner

AdnanHodzic commented Aug 31, 2020

I can also confirm it works as expected on Intel. I.e sudo stress --cpu 1 --timeout 15 works like a charm on i7-8565U.

@AdnanHodzic
I think that this code example should work correct:

if psutil.cpu_percent(percpu=False) >= 25.0 or isclose(max(psutil.cpu_percent(percpu=True)), 100):
    turbo(True)

Are you saying to replace (current) i.e:

cpuload > 100 / CPUS

with:

psutil.cpu_percent(percpu=False) >= 25.0

and same result will be achieved?

@Red-Eyed
Copy link
Contributor

@AdnanHodzic actually, could you please remind me at what conditions turbo should be enabled?

@rrodriguez81
Copy link
Contributor Author

@AdnanHodzic , this is just for launch turbo faster when the power supply is connected.

@Red-Eyed don't worry, all constructive ideas are welcomed :) .

@rrodriguez81
Copy link
Contributor Author

@Red-Eyed I think that there is a problem with you piece of code, it returns this error:
Traceback (most recent call last): File "/usr/bin/auto-cpufreq", line 115, in <module> main() File "/usr/lib/python3.8/site-packages/click/core.py", line 829, in __call__ return self.main(*args, **kwargs) File "/usr/lib/python3.8/site-packages/click/core.py", line 782, in main rv = self.invoke(ctx) File "/usr/lib/python3.8/site-packages/click/core.py", line 1066, in invoke return ctx.invoke(self.callback, **ctx.params) File "/usr/lib/python3.8/site-packages/click/core.py", line 610, in invoke return callback(*args, **kwargs) File "/usr/bin/auto-cpufreq", line 51, in main set_autofreq() File "/usr/lib/python3.8/site-packages/source/core.py", line 411, in set_autofreq set_powersave() File "/usr/lib/python3.8/site-packages/source/core.py", line 308, in set_powersave elif psutil.cpu_percent(percpu=False) >= 25.0 or isclose(max(psutil.cpu_percent(percpu=True), 100)): TypeError: '>' not supported between instances of 'int' and 'list'

@Red-Eyed
Copy link
Contributor

Yep, just a typo

if psutil.cpu_percent(percpu=False) >= 25.0 or isclose(max(psutil.cpu_percent(percpu=True)), 100):
    turbo(True)

@AdnanHodzic
Copy link
Owner

AdnanHodzic commented Aug 31, 2020

@AdnanHodzic actually, could you please remind me at what conditions turbo should be enabled?

Before this PR (and I think those values were good and we should keep them):

power-save/battery:

load1m > CPUS / 7
cpuload > 25

performance:

load1m >= CPUS / 5
cpuload > 20

And I quite like what @rrodriguez81 did, as it's very simple yet powerful solution. But I'm also interested in outcome of:

if psutil.cpu_percent(percpu=False) >= 25.0 or isclose(max(psutil.cpu_percent(percpu=True)), 100):
    turbo(True)

I'm thinking that maybe we should make turbo more "trigger happy" when in performance mode, i.e:

load1m >= CPUS / 4
cpuload > 15

Although this might be too aggressive.

@AdnanHodzic , this is just for launch turbo faster when the power supply is connected.

No worries, I deleted my commented in meantime since I figured what you did :)

@rrodriguez81
Copy link
Contributor Author

@Red-Eyed your solution works aswell, not sure why but it takes a few seconds to launch turbo.

capture
capture2

@AdnanHodzic regarding your idea about performance mode, if you set a static value, could be difficult to trigger turbo with CPUs with more than 8 threads.

For example: For 8 threads CPUS: 100 / 8 = the turbo is triggered with 12.5 CPU load, but if there is a Ryzen 4800H, with 8 cores and 16 threads, 100 / 16 , 100% CPU usage of one thread is 6,25% total of total CPU load.

Not sure if I'm explaining correctly (English is not my native language)

@Red-Eyed
Copy link
Contributor

@Red-Eyed your solution works aswell, not sure why but it takes a few seconds to launch turbo.

capture
capture2

@AdnanHodzic regarding your idea about performance mode, if you set a static value, could be difficult to trigger turbo with CPUs with more than 8 threads.

For example: For 8 threads CPUS: 100 / 8 = the turbo is triggered with 12.5 CPU load, but if there is a Ryzen 4800H, with 8 cores and 16 threads, 100 / 16 , 100% CPU usage of one thread is 6,25% total of total CPU load.

Not sure if I'm explaining correctly (English is not my native language)

That's why I proposed that code snippet that is not hardcoded to num of threads

@AdnanHodzic
Copy link
Owner

AdnanHodzic commented Aug 31, 2020

@Red-Eyed your solution works aswell, not sure why but it takes a few seconds to launch turbo.
capture
capture2
@AdnanHodzic regarding your idea about performance mode, if you set a static value, could be difficult to trigger turbo with CPUs with more than 8 threads.
For example: For 8 threads CPUS: 100 / 8 = the turbo is triggered with 12.5 CPU load, but if there is a Ryzen 4800H, with 8 cores and 16 threads, 100 / 16 , 100% CPU usage of one thread is 6,25% total of total CPU load.
Not sure if I'm explaining correctly (English is not my native language)

That's why I proposed that code snippet that is not hardcoded to num of threads

Agreed, having it hard coded is terrible. If it's fine with you @rrodriguez81 let's go with @Red-Eyed eye proposal as it's very clear what's happening. But since this your PR I'd like you to commit these changes, let's just figure out this first:

@Red-Eyed your solution works aswell, not sure why but it takes a few seconds to launch turbo.

@rrodriguez81
Copy link
Contributor Author

@AdnanHodzic @Red-Eyed not sure why, but with 100 / CPUS the turbo get triggered faster than with @Red-Eyed ' s provided solution.

With my solution the turbo is triggered instantly, and with @Red-Eyed sometimes takes a few seconds (3 or 4 seconds not too much). Maybe because the isclose() is an approximation or only needs an higher value.

But I'm agree using @Red-Eyed if is more clear for the code.

@Red-Eyed
Copy link
Contributor

Red-Eyed commented Aug 31, 2020

@AdnanHodzic @Red-Eyed not sure why, but with 100 / CPUS the turbo get triggered faster than with @Red-Eyed ' s provided solution.

With my solution the turbo is triggered instantly, and with @Red-Eyed sometimes takes a few seconds (3 or 4 seconds not too much). Maybe because the isclose() is an approximation or only needs an higher value.

But I'm agree using @Red-Eyed if is more clear for the code.

Maybe psutil takes such a delay
regarding isclose it's just approximation as you cannot strongly compare floats
instead of isclose, u can try

if psutil.cpu_percent(percpu=False) >= 25.0 or max(psutil.cpu_percent(percpu=True)) > 99:
    turbo(True)

Copy link
Contributor

@Red-Eyed Red-Eyed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems fine.

@rrodriguez81
Copy link
Contributor Author

@Red-Eyed the same problem, it trigger turbo slower than the 100 / CPUS solution. It takes to me a few seconds aswell.

@Red-Eyed
Copy link
Contributor

@Red-Eyed the same problem, it trigger turbo slower than the 100 / CPUS solution. It takes to me a few seconds aswell.

imho, i don't think that a few seconds is a problem..

@Red-Eyed
Copy link
Contributor

measured the time, and it's okay
image

@Red-Eyed
Copy link
Contributor

so it seems that psutil takes some average that requires some time

@rrodriguez81
Copy link
Contributor Author

A few seconds modifying a core speed to 2.3Ghz o 3.7Ghz for example, could be the duration of the process that get the 100% of a CPU thread. The more dynamic the turbo is launched, the more agile is the notebook.

@AdnanHodzic
Copy link
Owner

it seems fine.

Also seems fine on Intel.

so it seems that psutil takes some average that requires some time

This is also fine as I also want to switch so snap using latest Python and Ubuntu 20.04 as base in near future which might also fix things + one of the future psutil lib updates might also address this.

Either way, thank you both and especially to @rrodriguez81 for your first contribution 🥳

@Red-Eyed
Copy link
Contributor

A few seconds modifying a core speed to 2.3Ghz o 3.7Ghz for example, could be the duration of the process that get the 100% of a CPU thread. The more dynamic the turbo is launched, the more agile is the notebook.

Just wonder are you sure that switching turbo with high frequency is energy efficient or healthy idk?

@Red-Eyed
Copy link
Contributor

interesting, which version of psutil do u use?
python -c 'import psutil; print(psutil.__version__)'
mine is 5.7.2

@rrodriguez81
Copy link
Contributor Author

@Red-Eyed I think that in the actual stable version, it tries to launch turbo as soon as is needed, but it didn't have in mind all kinds or CPUs. But if you think that is a good idea to delay it a bit to be more power efficent is ok for me aswell.

The psutil version I'm using is the same as yours, I'm using latest Linux Manjaro distro.

@AdnanHodzic thanks to you and all the project contributors, projects like this makes LInux better :) .

@AdnanHodzic
Copy link
Owner

Before I merge this with master, one last observation, for me on Intel (i7-8565U ) with psutil 5.7.2.

stress --cpu 8 --io 4 --vm 4 --timeout 15

On performance will trigger turbo in a 1 or less, which is good.

sudo stress --cpu 1 --timeout 15

On performance it can take up to >= 5 seconds to trigger turbo, this is expected now right?

@Red-Eyed
Copy link
Contributor

I found one more parameter
@AdnanHodzic would be nice if you try it.
psutil.cpu_percent(percpu=False, interval=0.01) so, just add interval=0.01 to each psutil.cpu_percent call and take a look

@AdnanHodzic
Copy link
Owner

I found one more parameter
@AdnanHodzic would be nice if you try it.
psutil.cpu_percent(percpu=False, interval=0.01) so, just add interval=0.01 to each psutil.cpu_percent call and take a look

No this makes it worse, as turbo boost is too "trigger happy", it's almost on all the time in both performance and powersave for me.

Let's leave it as it is for now and then we can improve it later on. Merging ...

@Red-Eyed
Copy link
Contributor

Okay

@AdnanHodzic AdnanHodzic merged commit 5e6139b into AdnanHodzic:master Aug 31, 2020
AdnanHodzic pushed a commit that referenced this pull request Sep 2, 2020
* A fixed value of 25% CPU Load is not correct for enabling turbo, it should be enabled when one thread reach 100%

* A fixed turbo on chargin state

* Improved 1 thread CPU 100% load detection solution

Co-authored-by: Roberto Rodríguez <rrodriguez@datiobd.com>
AdnanHodzic pushed a commit that referenced this pull request Sep 2, 2020
* A fixed value of 25% CPU Load is not correct for enabling turbo, it should be enabled when one thread reach 100%

* A fixed turbo on chargin state

* Improved 1 thread CPU 100% load detection solution

Co-authored-by: Roberto Rodríguez <rrodriguez@datiobd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants