-
-
Notifications
You must be signed in to change notification settings - Fork 18
Faster way for performance monitor to get child processes #7574
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |||||||||||||||||||||||
| using System.IO; | ||||||||||||||||||||||||
| using System.Linq; | ||||||||||||||||||||||||
| using System.Management; | ||||||||||||||||||||||||
| using System.Runtime.InteropServices; | ||||||||||||||||||||||||
| using System.Windows.Forms; | ||||||||||||||||||||||||
| using Bloom.Api; | ||||||||||||||||||||||||
| using Bloom.ToPalaso; | ||||||||||||||||||||||||
|
|
@@ -348,12 +349,9 @@ public PerfPoint(bool refreshSubprocessList) | |||||||||||||||||||||||
| if (refreshSubprocessList) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| CleanupSubprocessList(); | ||||||||||||||||||||||||
| var subsubProcs = GetSubProcesses(new List<Process> { proc }); | ||||||||||||||||||||||||
| while (subsubProcs.Any()) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| _subProcesses.AddRange(subsubProcs); | ||||||||||||||||||||||||
| subsubProcs = GetSubProcesses(subsubProcs); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| var descendants = GetAllDescendantProcesses(proc); | ||||||||||||||||||||||||
| if (descendants.Any()) | ||||||||||||||||||||||||
| _subProcesses.AddRange(descendants); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| // Enhance: we could report the bytes of each sub-process, but that would be a lot of data. | ||||||||||||||||||||||||
| // Or: we could report the total bytes of all sub-processes, but would that be helpful? | ||||||||||||||||||||||||
|
|
@@ -372,49 +370,215 @@ public PerfPoint(bool refreshSubprocessList) | |||||||||||||||||||||||
| Debug.WriteLine($"PerfPoint created in {(whenReady - when).TotalMilliseconds}ms"); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private static List<Process> GetSubProcesses(List<Process> processes) | ||||||||||||||||||||||||
| private static List<Process> GetAllDescendantProcesses(Process parent) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| var subProcesses = new List<Process>(); | ||||||||||||||||||||||||
| foreach (var proc in processes) | ||||||||||||||||||||||||
| try | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| if (SIL.PlatformUtilities.Platform.IsWindows) | ||||||||||||||||||||||||
| return GetAllDescendantProcessesWindows(parent.Id); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| catch | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| // If the fast Windows approach fails for any reason, fall back to WMI below. | ||||||||||||||||||||||||
| Debug.WriteLine( | ||||||||||||||||||||||||
| $"Failed to get descendant processes for {parent.Id} using Windows API" | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| try | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| return GetAllDescendantProcessesWmi(parent.Id); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| catch | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| // Just give up. Our memory report won't include subprocesses. | ||||||||||||||||||||||||
| Debug.WriteLine( | ||||||||||||||||||||||||
| $"Failed to get descendant processes for {parent.Id} at all. Memory used report won't be accurate" | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| return new List<Process>(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private static List<Process> GetAllDescendantProcessesWindows(int parentPid) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| // Build a parent->children map in one pass over a toolhelp snapshot, then walk descendants. | ||||||||||||||||||||||||
| var childMap = GetChildProcessMapWindows(); | ||||||||||||||||||||||||
| var descendantPids = new HashSet<int>(); | ||||||||||||||||||||||||
| var queue = new Queue<int>(); | ||||||||||||||||||||||||
| queue.Enqueue(parentPid); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| while (queue.Count > 0) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| var pid = queue.Dequeue(); | ||||||||||||||||||||||||
| if (!childMap.TryGetValue(pid, out var children)) | ||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| foreach (var childPid in children) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| if (descendantPids.Add(childPid)) | ||||||||||||||||||||||||
| queue.Enqueue(childPid); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
Comment on lines
+416
to
+422
|
||||||||||||||||||||||||
| foreach (var childPid in children) | |
| { | |
| if (descendantPids.Add(childPid)) | |
| queue.Enqueue(childPid); | |
| } | |
| } | |
| foreach (var childPid in children.Where(childPid => descendantPids.Add(childPid))) | |
| { | |
| queue.Enqueue(childPid); | |
| } | |
| } |
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.
Don't want to risk changing this. The current code is not difficult to understand, and I'm not 100% sure that there is no difference between checking when the pid is enqueued and checking when it is dequeued.
Copilot
AI
Jan 14, 2026
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.
When CreateToolhelp32Snapshot fails, the function silently returns an empty map, which will result in no child processes being found. Consider logging this failure for diagnostics, as it could indicate a system issue or permission problem that might affect performance monitoring accuracy.
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.
Not new or changed code. Don't want to mess with it.
andrew-polk marked this conversation as resolved.
Show resolved
Hide resolved
andrew-polk marked this conversation as resolved.
Show resolved
Hide resolved
andrew-polk marked this conversation as resolved.
Show resolved
Hide resolved
andrew-polk marked this conversation as resolved.
Show resolved
Hide resolved
andrew-polk marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Jan 14, 2026
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.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
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.
See above
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.
The catch block silently swallows all exceptions when the Windows approach fails. Consider logging the exception for diagnostics to help identify why the optimized path failed and whether the fallback is being triggered frequently.
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.
Done