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 snapshot feature for collecting process info at a point in time #517

Closed
wants to merge 1 commit into from
Closed

Conversation

codesmith14
Copy link

  • capture all information about a process at a given point in time as a snapshot
  • de-duplicate underlying calls to fill in the snapshot
  • allow a filter system for only filling the snapshot with desired information

Hi,
I am proposing a new structure called Snapshot that inherits from Process, but loads all information at the time the snapshot is created.
The goals of this addition are to:

  • reduce the time it takes to collect process information
    I see a ~30% reduction ( 2.8 ms compared to 3.9 ms) in the average time to load all information for a single process.
  • maintain feature parity with Process
  • de-duplicate the calls to "fill" functions in Process.

There is also a filtering system so static snapshots can be taken with only a subset of process information.

This proposed solution was written to avoid making large changes to the existing Process design. I appreciate any suggestions you may have for achieving the goals I listed above.

If this is of no interest to this project, then I ask that you consider exposing the underlying fill functions. This would allow the snapshot feature to be developed in a separate downstream project.

Thanks!

@Lomanic
Copy link
Collaborator

Lomanic commented Apr 28, 2018

Thanks for your work and this good idea @charliesignalfx!

Caching process stats was discussed in #413, prepopulating a process with given fields in #286.

Your implementation is interesting, quite different than what was discussed before, but limited and Linux-only currently, it might require a bit of code duplication and even double the work to have the same API for Snapshot and Process types… for each OS gopsutil supports. Plus, it does not solve another problem we have on Windows when reading all processes stats (highly inefficient to call WMI for each process when we could want a "snapshot" of all the running processes with a single WMI call, re #394, #250).

I don't think we will include this new Snapshot type (as-is at least, too much work to re-implement on other platforms, much work to keep its API against Process API evolutions), but we might need this code to implement some NewProcessWithFields(pid, fields) function (#286).

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