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

Uses psutils to get the cpu utilization #30

Merged
merged 7 commits into from
Feb 16, 2024
Merged

Conversation

ribalba
Copy link
Member

@ribalba ribalba commented Feb 2, 2024

While working on the cloud machine script it was quite annoying to have to keep the c script around. It looks like the c metrics provider and the psutils implementation give similar results. Also I can't see much difference on overhead on my development machine.

I think this will also make the ci scripts easier as we don't need to compile the metric provider all the time. I have the feeling I am missing something though. Why didn't we do this from the beginning?

@ribalba ribalba requested a review from ArneTR February 2, 2024 20:39
@ArneTR
Copy link
Member

ArneTR commented Feb 3, 2024

uh, nice! Will merge that in when I do the rest of the auto functionality.

The parameter will need to be renamed than.

A funny note on a typo: "cpu_utalisation" Your autocorrect has created a german crippled variant of the english word that gave me quite a laugh

@ArneTR
Copy link
Member

ArneTR commented Feb 3, 2024

A question on this: I suppose you have made this so you can easily use it on macOS.

How does psutils internally get the CPU % on macOS?

@ribalba
Copy link
Member Author

ribalba commented Feb 4, 2024

  • Hahah, just inventing new words here.
  • The actual reason I added it so that I don't need to create a statically linked version of the metrics provider as I don't want gcc on the ec2 machine just for the reporter. Seemed to be overkill.

I did some research how psutils works.

  • On Mac it uses the host_statistics call [0]. I still need to check how htop does it.
  • Under linux it just reads the procfs /stats file and parses it as does the metrics provider. Why did you do this in c and not in python. The difference of opening a file and parsing the lines shouldn't be that different?

https://web.mit.edu/darwin/src/modules/xnu/osfmk/man/host_statistics.html

@ArneTR
Copy link
Member

ArneTR commented Feb 5, 2024

sadly the psutil function suffers from the same issues as the C code I wrote (most likely because they use the same syscall).

This is what I get when running with 100ms. A lot of unexpected zeros:

Screenshot 2024-02-05 at 10 38 33 AM

@ArneTR
Copy link
Member

ArneTR commented Feb 5, 2024

I also flagged this here: giampaolo/psutil#1700

@dan-mm
Copy link
Contributor

dan-mm commented Feb 5, 2024

We can also use this to decouple the XGBoostModel / SDIA metrics providers from our cpu-util metric providers, which would be quite nice. Especially for Mac, where we don't have a cpu-util provider

@ribalba
Copy link
Member Author

ribalba commented Feb 15, 2024

@ArneTR can we merge this so I don't have to refer to a branch in my blog article?

@ArneTR
Copy link
Member

ArneTR commented Feb 15, 2024

it still needs macOS code, or not? Currently this only really works under linux

@ribalba ribalba merged commit 5557f3d into main Feb 16, 2024
1 check passed
@ribalba ribalba deleted the add-cpu-utalisation branch February 16, 2024 11:33
@ArneTR
Copy link
Member

ArneTR commented Feb 16, 2024

splendid! ❤️

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