add vmmem process lookup functionality + windows API abstraction#2613
Open
rawahars wants to merge 1 commit intomicrosoft:mainfrom
Open
add vmmem process lookup functionality + windows API abstraction#2613rawahars wants to merge 1 commit intomicrosoft:mainfrom
rawahars wants to merge 1 commit intomicrosoft:mainfrom
Conversation
The original implementation used a brute-force approach to locate the Hyper-V memory management process. It relied on enumerating every single Process ID (PID) on the system and attempting to open each one to query its image path. This meant the system was executing expensive OS-level calls and generating ignored access-denied errors hundreds of times for unrelated processes, just to check if the name matched "vmmem". Ref- https://github.com/microsoft/hcsshim/blob/178d662f94b76b24fda758dfc41a7d9e6a2ee614/internal/uvm/stats.go#L76 The new enhancement significantly optimizes this discovery process by using windows API `CreateToolhelp32Snapshot`, which captures process names upfront without opening the processes. Now, the code iterates through the snapshot and pre-filters by the executable name. It only executes the expensive `OpenProcess` and security token checks (`LookupAccount`) if the process is already confirmed to be named `vmmem` or `vmmem.exe`. Furthermore, the updated code introduces an interface to wrap the Windows OS calls. This dependency injection allows the system calls to be easily mocked, making the function fully unit-testable without requiring a live Hyper-V virtual machine. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The original implementation used a brute-force approach to locate the Hyper-V memory management process (
vmmem). It relied on enumerating every single Process ID (PID) on the system and attempting to open each one to query its image path. This meant the system was executing expensive OS-level calls and generating ignored access-denied errors hundreds of times for unrelated processes, just to check if the name matched"vmmem".Reference-
hcsshim/internal/uvm/stats.go
Line 76 in 178d662
The new enhancement significantly optimizes this discovery process by using windows API
CreateToolhelp32Snapshot, which captures process names upfront without opening the processes. Now, the code iterates through the snapshot and pre-filters by the executable name. It only executes the expensiveOpenProcessand security token checks (LookupAccount) if the process is already confirmed to be namedvmmemorvmmem.exe.Furthermore, the updated code introduces an interface to wrap the Windows OS calls. This dependency injection allows the system calls to be easily mocked, making the function fully unit-testable without requiring a live Hyper-V virtual machine.
Testing
vmmemprocess and list process stats. Both matched.windowsmocks and runs the scenarios.Notes