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

Add memory.ItemizedPercent #845

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add memory.ItemizedPercent #845

wants to merge 1 commit into from

Conversation

yudai
Copy link

@yudai yudai commented Feb 21, 2020

Hi, thank you for the handy library.


This new method returns a new struct PercentStat that holds CPU usage
percentages for different work loads (e.g. user, system and so on).
This commit also exposes CalculatePercent() and CaluculateItemizedPercent()
that return usage percentages from two TimesStats so that users
can calculate percentages themselves as well.

@shirou
Copy link
Owner

shirou commented Feb 23, 2020

Thank you for your PR. But I think we should keep this library as simple as possible. And it seems this PR provide utility function, so how about to create another library which uses gopsutil?

@yudai
Copy link
Author

yudai commented Feb 25, 2020

Thank you for the reply. I understand keeping a library simple is always the way to go.

However, I'm afraid I assume this is a common use case. Actually I just found psutil has the same functionality.

https://psutil.readthedocs.io/en/latest/#psutil.cpu_times_percent
giampaolo/psutil#362

What if we simply rename ItemizedPercent() added in this PR to CPUTimesPercent() and make it return TimesStat instead of new PercentStat (IMO, it's better/safer to separate TimeStat and PercentStat as they store different types of values, but it's not critical to me)?

CalculatePercent() and CaluculateItemizedPercent() are just to expose existing logic in cpu.go for uses. We can drop these from the PR if you'd like.

If the idea above works for you, I'll update my code. (EDIT: updated)

This new method returns a TimesStat that holds CPU usage
percentages for different work loads (e.g. user, system and so on).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants