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

Reading stats for all running processes is inefficient #413

Open
conorbranagan opened this issue Aug 5, 2017 · 3 comments
Open

Reading stats for all running processes is inefficient #413

conorbranagan opened this issue Aug 5, 2017 · 3 comments

Comments

@conorbranagan
Copy link
Contributor

conorbranagan commented Aug 5, 2017

Hi,

We are using the gopsutil as part of a tool that's collecting information about all running processes. With the existing gopsutil public APIs in process_* you often end up doing the same work several times to get different data. For example calling p.Status() and p.UIDs() reads through /proc/$pid/status twice.

Right now we've forked and added an which adds an AllProcesses function (https://github.com/DataDog/gopsutil/blob/dd/process/process_linux.go#L794) that performs each proc file read once and then populates a FilledProcess. It's certainly not perfect and ideally we'd like to stay in sync with upstream and make this work well in the normal case.

So the questions are: have you given any thought to this use case? Is adding AllProcesses something that might be able to be accepted upstream OR could we change the existing API so it's possible to get all the fields we want with the fewest number of syscalls.

Thanks!

@shirou
Copy link
Owner

shirou commented Aug 6, 2017

Because if a user want to read just a NumFD, getting other information is waste. So I separate it.

However, I know this is not efficient if user like you want all information related to the process. Then, I agree with your AllProcesses and will accept if it is implemented all of the supported platforms.

And how about introduce a cache mechanism to AllProcesses?

@conorbranagan
Copy link
Contributor Author

Yes I you are definitely right that in most cases you're just getting a couple bits of information and pulling everything does not make sense.

That's great news that you might accept AllProcesses upstream. With that in mind I'll work on clean up what we have and getting it to support other platforms (so far we just have linux, mac and freebsd) and see what you think. Thanks for the quick response!

@shirou
Copy link
Owner

shirou commented Aug 7, 2017

I think cache can fascinate other users to use AllProcesses. Perhaps #286 might be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants