-
Notifications
You must be signed in to change notification settings - Fork 259
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
wcow-process: Query Stats directly from shim #1362
Conversation
This change adds in a optimization workaround for cases where we query HCs for just statistics through our internal PropertiesV2 interface. The optimization we can make here if the user is asking for Statistics only, and this is due to an inefficient way that HCS calculates the private working set total for a given container. HCS calls NtQuerySystemInformation with the class SystemProcessInformation which returns an array containing system information for *every* process running on the machine. They then grab the pids that are running in the container and filter down the entries in the array to only what's running in that silo and start tallying up the total. This doesn't work well as performance should get worse if more processess are running on the machine in general and not just in the container. All of the additional information besides the WorkingSetPrivateSize field is ignored as well which isn't great and is wasted work to fetch. HCS only let's you grab statistics in an all or nothing fashion, so we can't just grab the private working set ourselves and ask for everything else seperately. The optimization we can make here is to open the silo ourselves and do the same queries for the rest of the info, as well as calculating the private working set in a more efficient manner by: 1. Find the pids running in the silo 2. Get a process handle for every process (only need PROCESS_QUERY_LIMITED_INFORMATION access) 3. Call NtQueryInformationProcess on each process with the class ProcessVmCounters 4. Tally up the total using the field PrivateWorkingSetSize in VM_COUNTERS_EX2. This change additionally: * Changes the jobcontainers package to use this new way to calculate the private working set. * Change the query the StorageStats method in the jobobject package uses to grab IO counters to match what HCS queries. Signed-off-by: Daniel Canter <[email protected]>
* Change to explicitly using Nanosecond units for timestamps and not relying on time.Duration to be ns. * Add RLock locking back to QueryStorageStats method * Get rid of SYSTEM check and just bail if job object open fails. Signed-off-by: Daniel Canter <[email protected]>
I like the idea. Can we not just query the working set size associated to the JO handle for the container in one pass? It seems like that should be a feature if it isnt already :) |
At least not that I could find, I've asked some peeps to look around if there's something hidden.. |
Add check to make sure we don't count a processes working set if its pid was re-used. Signed-off-by: Daniel Canter <[email protected]>
@kevpar ptal when you get a chance |
Get rid of timestamp approach in favor of just calling IsProcessInJob after opening the process handle. Signed-off-by: Daniel Canter <[email protected]>
Don't leak process handles. Signed-off-by: Daniel Canter <[email protected]>
@kevpar Let me know if the latest update looks good here |
I think these changes go beyond "optimizing". For clarify, we should phrase the commit differently so the impact is clear. I'd suggest something like "wcow-process: Query stats directly from shim". |
@kevpar I can change the squashed commit msg on check-in to be more clear here |
Can we change the PR title to? I think that's included in the squash commit title. |
PrivateWorkingSet -> QueryPrivateWorkingSet to match the other job object methods. Signed-off-by: Daniel Canter <[email protected]>
* Check len(type) == 0 instead * Signed-off-by: Daniel Canter <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Rearrange in process queries to account for other query types in the future. Signed-off-by: Daniel Canter <[email protected]>
@kevpar Ok, hopefully the last rendition of this :) |
* Log if we failed to grab stats * Add more context for if the in proc query fails as it would be hard to tell what happened. * Rephrase PropertiesV2 comment to not say "container" as it works on virtual machines as well. * Return an error if we encounter an unknown fallback property type. Signed-off-by: Daniel Canter <[email protected]>
* Return the properties returned from HCS and fill in what we've gathered in-proc instead of vice versa. Signed-off-by: Daniel Canter <[email protected]>
* %w on error * Change warning log to Info as it's expected to happen for some types. * Comment on that processlist querying in the shim is for the future. Signed-off-by: Daniel Canter <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This change adds in functionality to query statistics directly in the shim instead of reaching out to HCS. One of the main motivators behind this was poor performance for tallying up the private working set total for the container in HCS. HCS calls NtQuerySystemInformation with the class SystemProcessInformation which returns an array containing system information for every process running on the machine. They then grab the pids that are running in the container and filter down the entries in the array to only what's running in that silo and start tallying up the total. This doesn't work well as performance should get worse if more processes are running on the machine in general and not just in the container. All of the additional information besides the WorkingSetPrivateSize field is ignored as well which isn't great and is wasted work to fetch. HCS only let's you grab statistics in an all or nothing fashion, so we can't just grab the private working set ourselves and ask for everything else separately. We can open the silo ourselves and do the same queries for the rest of the info, as well as calculating the private working set in a more efficient manner by: 1. Find the pids running in the silo 2. Get a process handle for every process (only need PROCESS_QUERY_LIMITED_INFORMATION access) 3. Call NtQueryInformationProcess on each process with the class ProcessVmCounters 4. Tally up the total using the field PrivateWorkingSetSize in VM_COUNTERS_EX2. This change additionally: - Changes the jobcontainers package to use this new way to calculate the private working set. - Change the query the StorageStats method in the jobobject package uses to grab IO counters to match what HCS queries. Signed-off-by: Daniel Canter <[email protected]> (cherry picked from commit 4a1216a) Signed-off-by: Daniel Canter <[email protected]>
- updates go-winio package to take in changes that allow us to use guid on linux - creates a new package for iochannel - removes windows specific error handling and syscall references This has some "TODO"s in it since this is prototype work Related work items: microsoft#1362
This change adds in functionality to query statistics directly in the shim instead of reaching out to HCS. One of the main motivators behind this was poor performance for tallying up the private working set total for the container in HCS. HCS calls NtQuerySystemInformation with the class SystemProcessInformation which returns an array containing system information for every process running on the machine. They then grab the pids that are running in the container and filter down the entries in the array to only what's running in that silo and start tallying up the total. This doesn't work well as performance should get worse if more processes are running on the machine in general and not just in the container. All of the additional information besides the WorkingSetPrivateSize field is ignored as well which isn't great and is wasted work to fetch. HCS only let's you grab statistics in an all or nothing fashion, so we can't just grab the private working set ourselves and ask for everything else separately. We can open the silo ourselves and do the same queries for the rest of the info, as well as calculating the private working set in a more efficient manner by: 1. Find the pids running in the silo 2. Get a process handle for every process (only need PROCESS_QUERY_LIMITED_INFORMATION access) 3. Call NtQueryInformationProcess on each process with the class ProcessVmCounters 4. Tally up the total using the field PrivateWorkingSetSize in VM_COUNTERS_EX2. This change additionally: - Changes the jobcontainers package to use this new way to calculate the private working set. - Change the query the StorageStats method in the jobobject package uses to grab IO counters to match what HCS queries. Signed-off-by: Daniel Canter <[email protected]>
This change adds in functionality to query statistics directly in the shim instead of reaching out to HCS. One of the main motivators behind this was poor performance for tallying up the private working set total for the container in HCS.
HCS calls NtQuerySystemInformation with the class SystemProcessInformation which returns an array containing system information for every process running on the machine. They then grab the pids that are running in the container and filter down the entries in the array to only what's running in that silo and start tallying up the total. This doesn't work well as performance should get worse if more processes are running on the machine in general and not just in the container. All of the additional information besides the WorkingSetPrivateSize field is ignored as well which isn't great and is wasted work to fetch.
HCS only let's you grab statistics in an all or nothing fashion, so we can't just grab the private working set ourselves and ask for everything else separately. We can open the silo ourselves and do the same queries for the rest of the info, as well as calculating the private working set in a more efficient manner by:
PROCESS_QUERY_LIMITED_INFORMATION access)
ProcessVmCounters
VM_COUNTERS_EX2.
This change additionally:
private working set.
to grab IO counters to match what HCS queries.
Some quick test results between grabbing the same stats inProc vs. calling PropertiesV2 and only asking for statistics: