Skip to content

Commit

Permalink
Make C# and Python process identification virtually identical
Browse files Browse the repository at this point in the history 
This change resolves a few differences in the output of the C# program
that identifies Chrome processes and the original Python version. The
two tools now give identical results (except for the new lost-events
warning) on 481 out of 485 traces tested, and the four discrepancies are
on traces with weird process-parentage data that doesn't really make
sense anyway.

The results in -c mode (showing CPU usage) remain because trace
processing reports on CPU usage differently, but those differences are
known and expected and not generally significant.
  • Loading branch information
@randomascii
randomascii committed Jul 22, 2019
1 parent 4b87031 commit 169dcfb
Showing 1 changed file with 43 additions and 50 deletions.
93 changes: 43 additions & 50 deletions TraceProcessors/IdentifyChromeProcesses/IdentifyChromeProcesses.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,40 +60,32 @@ public ProcessSummary(string imageName, string exePath, string commandLine, uint
}

// Get a list of Chrome processes. Most of the logic is from getting a
// a full path name. Don't return a Dictionary keyed by PID because
// a full exe path name. Don't return a Dictionary keyed by PID because
// that will lose records whenever there is PID reuse, which is often.
static List<ProcessSummary> GetProcessSummaries(IProcessDataSource processData)
{
var processSummaries = new List<ProcessSummary>();

foreach (var process in processData.Processes)
{
string commandLine = process.CommandLine;
uint pid = process.Id;
uint parentPid = process.ParentId;
string imageName = process.ImageName;
string exe_path = "";
if (imageName != "chrome.exe")
continue;

// Try to find the path to the executable by matching the
// image name with individual image file names.
//for (int i = 0; i < process.Images.Count; ++i)
foreach (var image in process.Images)
{
// When processing a trace with AllowLostEvents some records
// may be missing.
try
{
if (imageName == image.FileName) //process.Images[i].FileName)
{
exe_path = image.Path; // process.Images[i].Path;
break;
}
}
catch (System.InvalidOperationException)
{ /* Ignore */ }
}
uint pid = process.Id;
uint parentPid = process.ParentId;
// Extract the exePath from the command line. It is also possible to
// grab it from the process.Images.Path attribute but that sometimes
// gives weird results, is not clearly better, and makes comparing
// with the Python script more difficult.
// The exePath may or may not be quoted, and may or not have the .exe
// suffix but if we strip quotes or split on spaces it works.
string commandLine = process.CommandLine != null ? process.CommandLine : "<Unknown>";
string exe_path;
if (commandLine.StartsWith("\""))
exe_path = commandLine.Substring(1, commandLine.IndexOf('\"', 1) - 1);
else
exe_path = commandLine.Split(' ')[0];

processSummaries.Add(new ProcessSummary(imageName, exe_path, commandLine, pid, parentPid));
}
Expand Down Expand Up @@ -230,22 +222,25 @@ static void ProcessTrace(ITraceProcessor trace, bool showCPUUsage, bool eventsLo
destType = "gpu???";
}

// The browserPid *should* always be present, but when events are lost then
// all bets are off.
if (!eventsLost || parentPids.ContainsKey(browserPid))
// The browserPid *should* always be present, but in a large enough corpus
// of traces, all bets are off. This assumption failed in a 20-hour heap
// snapshot trace.
if (parentPids.ContainsKey(browserPid))
{
// The childPids["browser"] entry needs to be appended to its
// parent/grandparent process since that is its browser process.
uint parentPid = parentPids[browserPid];
// Create the destination type if necessary (needed for gpu???,
// not needed for crashpad).
if (!processesByBrowserPid[parentPid].ContainsKey(destType))
processesByBrowserPid[parentPid][destType] = new List<uint>();
processesByBrowserPid[parentPid][destType].Add(childPids[childType][0]);
// not needed for crashpad). Handle missing data.
if (processesByBrowserPid.ContainsKey(parentPid))
{
if (!processesByBrowserPid[parentPid].ContainsKey(destType))
processesByBrowserPid[parentPid][destType] = new List<uint>();
processesByBrowserPid[parentPid][destType].Add(childPids[childType][0]);

// Remove the fake 'browser' entry so that we don't try to print it.
processesByBrowserPid.Remove(browserPid);
continue;
// Remove the fake 'browser' entry so that we don't try to print it.
processesByBrowserPid.Remove(browserPid);
}
}
}
}
Expand All @@ -268,8 +263,7 @@ static void ProcessTrace(ITraceProcessor trace, bool showCPUUsage, bool eventsLo
// short this is probably faster than using a dictionary.
if (!names.Contains(slice.Process.ImageName))
continue;
CPUUsageDetails last;
exec_times.TryGetValue(slice.Process.Id, out last);
exec_times.TryGetValue(slice.Process.Id, out CPUUsageDetails last);
last.imageName = slice.Process.ImageName;
last.ns += slice.Duration.Nanoseconds;
last.contextSwitches += 1;
Expand Down Expand Up @@ -312,8 +306,7 @@ static void ProcessTrace(ITraceProcessor trace, bool showCPUUsage, bool eventsLo
var detailsSubTotal = new CPUUsageDetails();
foreach (uint pid in type.Value)
{
CPUUsageDetails details;
exec_times.TryGetValue(pid, out details);
exec_times.TryGetValue(pid, out CPUUsageDetails details);
detailsTotal.ns += details.ns;
detailsTotal.contextSwitches += details.contextSwitches;

Expand All @@ -334,10 +327,9 @@ static void ProcessTrace(ITraceProcessor trace, bool showCPUUsage, bool eventsLo
}
else
{
// The browserPid *should* always be present, but when events are lost then
// all bets are off.
string browserPath = "<unknown path>";
if (!eventsLost || pathByBrowserPid.ContainsKey(browserPid))
// See earlier note about how the browserPid may be missing.
string browserPath = "Unknown parent";
if (pathByBrowserPid.ContainsKey(browserPid))
browserPath = pathByBrowserPid[browserPid];
Console.WriteLine("{0} ({1}) - {2} processes", browserPath, browserPid, totalProcesses);
}
Expand All @@ -364,8 +356,7 @@ static void ProcessTrace(ITraceProcessor trace, bool showCPUUsage, bool eventsLo
if (showCPUUsage)
{
Console.Write("\n ");
CPUUsageDetails details;
exec_times.TryGetValue(pid, out details);
exec_times.TryGetValue(pid, out CPUUsageDetails details);
if (details.contextSwitches > 0)
Console.Write("{0,5} - {1,6} context switches, {2,8:0.00} ms CPU", pid, details.contextSwitches, details.ns / 1e6);
else
Expand Down Expand Up @@ -414,20 +405,22 @@ static void Main(string[] args)
{
using (ITraceProcessor trace = TraceProcessor.Create(traceName))
ProcessTrace(trace, showCPUUsage, false);
return;
}
catch (System.InvalidOperationException e)
{
// Note that wpaexporter doesn't seem to have a way to handle this,
// which is one advantage of TraceProcessing.
// which is one advantage of TraceProcessing. Note that traces with
// lost events are "corrupt" in some sense so the results will be
// unpredictable.
Console.WriteLine(e.Message);
Console.WriteLine("Trying again with AllowLostEvents and AllowTimeInversion specified. Results may be less reliable.");
Console.WriteLine();

var settings = new TraceProcessorSettings();
settings.AllowLostEvents = true;
settings.AllowTimeInversion = true;
using (ITraceProcessor trace = TraceProcessor.Create(traceName, settings))
ProcessTrace(trace, showCPUUsage, true);
}
var settings = new TraceProcessorSettings();
settings.AllowLostEvents = true;
settings.AllowTimeInversion = true;
using (ITraceProcessor trace = TraceProcessor.Create(traceName, settings))
ProcessTrace(trace, showCPUUsage, true);
}
}

0 comments on commit 169dcfb

Please sign in to comment.