Skip to content

Commit

Permalink
Fix some of the issues around pgo data collection of stack trace data (…
Browse files Browse the repository at this point in the history 
…dotnet#59970)

- Improve assembly name acquisition in dotnet-pgo
  - Use assembly name from AssemblyLoad event, and do not attempt to figure it out from the Module load event
- Collect MemoryRegionInfo for all method regions, not just the first.
- Add a concept to check a guid associated with the associated dll files to avoid data tearing
- Improve parsing of instantiated method signatures when reading an R2R file
  - Handle the subtly different format which is use for type signatures in the InstantiationMethodSignature portion of an R2R file
  • Loading branch information
@davidwrighton
davidwrighton authored Oct 8, 2021
1 parent ce4eb48 commit 6e0e2d0
Show file tree
Hide file tree
Showing 7 changed files with 250 additions and 64 deletions.
3 changes: 1 addition & 2 deletions src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ private void ParseInstanceMethodEntrypointsCustom<TType, TMethod, TGenericContex
while (!curParser.IsNull())
{
IAssemblyMetadata mdReader = GetGlobalMetadata();
var decoder = new R2RSignatureDecoder<TType, TMethod, TGenericContext>(provider, default(TGenericContext), mdReader.MetadataReader, this, (int)curParser.Offset);
var decoder = new R2RSignatureDecoder<TType, TMethod, TGenericContext>(provider, default(TGenericContext), mdReader?.MetadataReader, this, (int)curParser.Offset);

TMethod customMethod = decoder.ParseMethod();

Expand All @@ -887,7 +887,6 @@ private void ParseInstanceMethodEntrypointsCustom<TType, TMethod, TGenericContex
ReadyToRunMethod r2rMethod = _runtimeFunctionToMethod[runtimeFunctionId];
if (!Object.ReferenceEquals(customMethod, null) && !foundMethods.ContainsKey(customMethod))
foundMethods.Add(customMethod, r2rMethod);
foundMethods.Add(customMethod, r2rMethod);
curParser = allEntriesEnum.GetNext();
}
}
Expand Down
100 changes: 78 additions & 22 deletions src/coreclr/tools/dotnet-pgo/MethodMemoryMap.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Text;
Expand All @@ -20,15 +21,38 @@ internal class MethodMemoryMap
private readonly ulong[] _infoKeys;
public readonly MemoryRegionInfo[] _infos;

struct JittedID : IEquatable<JittedID>
{
public JittedID(long methodID, long reJITID)
{
MethodID = methodID;
ReJITID = reJITID;
}

public readonly long MethodID;
public readonly long ReJITID;

public override int GetHashCode() => HashCode.Combine(MethodID, ReJITID);
public override bool Equals([NotNullWhen(true)] object obj) => obj is JittedID id ? Equals(id) : false;

public bool Equals(JittedID other)
{
if (other.MethodID != MethodID)
return false;
return other.ReJITID == ReJITID;
}
}

public MethodMemoryMap(
TraceProcess p,
TraceTypeSystemContext tsc,
TraceRuntimeDescToTypeSystemDesc idParser,
int clrInstanceID)
int clrInstanceID,
Logger logger)
{
// Capture the addresses of jitted code
List<MemoryRegionInfo> infos = new List<MemoryRegionInfo>();
Dictionary<long, MemoryRegionInfo> info = new Dictionary<long, MemoryRegionInfo>();
Dictionary<JittedID, MemoryRegionInfo> info = new Dictionary<JittedID, MemoryRegionInfo>();
foreach (var e in p.EventsInProcess.ByEventType<MethodLoadUnloadTraceData>())
{
if (e.ClrInstanceID != clrInstanceID)
Expand All @@ -47,13 +71,16 @@ public MethodMemoryMap(

if (method != null)
{
infos.Add(new MemoryRegionInfo
JittedID jittedID = new JittedID(e.MethodID, 0);
if (!info.ContainsKey(jittedID))
{
StartAddress = e.MethodStartAddress,
EndAddress = e.MethodStartAddress + checked((uint)e.MethodSize),
MethodID = e.MethodID,
Method = method,
});
info.Add(jittedID, new MemoryRegionInfo
{
StartAddress = e.MethodStartAddress,
EndAddress = e.MethodStartAddress + checked((uint)e.MethodSize),
Method = method,
});
}
}
}

Expand All @@ -67,25 +94,28 @@ public MethodMemoryMap(
MethodDesc method = null;
try
{
method = idParser.ResolveMethodID(e.MethodID);
method = idParser.ResolveMethodID(e.MethodID, throwIfNotFound: false);
}
catch
{
}

if (method != null)
{
infos.Add(new MemoryRegionInfo
JittedID jittedID = new JittedID(e.MethodID, e.ReJITID);
if (!info.ContainsKey(jittedID))
{
StartAddress = e.MethodStartAddress,
EndAddress = e.MethodStartAddress + checked((uint)e.MethodSize),
MethodID = e.MethodID,
Method = method,
});
info.Add(jittedID, new MemoryRegionInfo
{
StartAddress = e.MethodStartAddress,
EndAddress = e.MethodStartAddress + checked((uint)e.MethodSize),
Method = method,
});
}
}
}

var sigProvider = new R2RSignatureTypeProvider(tsc);
var sigProvider = new R2RSignatureTypeProviderForGlobalTables(tsc);
foreach (var module in p.LoadedModules)
{
if (module.FilePath == "")
Expand Down Expand Up @@ -120,18 +150,40 @@ public MethodMemoryMap(
}
}
}
catch { }
catch
{
logger.PrintWarning($"Failed to load method entry points from R2R module {module.FilePath}");
}
}

// Can have duplicate events, so pick first for each
var byMethodID = infos.GroupBy(i => i.MethodID).ToDictionary(g => g.Key, g => g.First());
// Associate NativeToILMap with MethodLoad event found Memory Regions
foreach (MethodILToNativeMapTraceData e in p.EventsInProcess.ByEventType<MethodILToNativeMapTraceData>())
{
if (byMethodID.TryGetValue(e.MethodID, out MemoryRegionInfo inf))
if (info.TryGetValue(new JittedID(e.MethodID, e.ReJITID), out MemoryRegionInfo inf))
inf.NativeToILMap = NativeToILMap.FromEvent(e);
}

_infos = byMethodID.Values.OrderBy(i => i.StartAddress).ToArray();
// Sort the R2R data by StartAddress
MemoryRegionInfoStartAddressComparer startAddressComparer = new MemoryRegionInfoStartAddressComparer();
infos.Sort(startAddressComparer);

// For each method found via MethodLoad events, check to see if it exists in the infos array, and if it does not, build a list to add
List<MemoryRegionInfo> memoryRegionsToAdd = new List<MemoryRegionInfo>();
foreach (var methodLoadInfo in info.Values)
{
int searchResult = infos.BinarySearch(methodLoadInfo, startAddressComparer);
if (searchResult < 0)
{
memoryRegionsToAdd.Add(methodLoadInfo);
}
}

// Add the regions from the MethodLoad events, and keep the overall array sorted
infos.AddRange(memoryRegionsToAdd);
infos.Sort(startAddressComparer);

_infos = infos.ToArray();

_infoKeys = _infos.Select(i => i.StartAddress).ToArray();

#if DEBUG
Expand Down Expand Up @@ -164,13 +216,17 @@ public MemoryRegionInfo GetInfo(ulong ip)
}

public MethodDesc GetMethod(ulong ip) => GetInfo(ip)?.Method;

private class MemoryRegionInfoStartAddressComparer : IComparer<MemoryRegionInfo>
{
int IComparer<MemoryRegionInfo>.Compare(MemoryRegionInfo x, MemoryRegionInfo y) => x.StartAddress.CompareTo(y.StartAddress);
}
}

public class MemoryRegionInfo
{
public ulong StartAddress { get; set; }
public ulong EndAddress { get; set; }
public long MethodID { get; set; }
public MethodDesc Method { get; set; }
public NativeToILMap NativeToILMap { get; set; }
}
Expand Down
19 changes: 17 additions & 2 deletions src/coreclr/tools/dotnet-pgo/ModuleLoadLogger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,28 @@ public ModuleLoadLogger(Logger logger)

Logger _logger;

[ThreadStatic]
private static int s_loadFailuresAreErrors = 0;

public class LoadFailuresAsErrors : IDisposable
{
public LoadFailuresAsErrors()
{
s_loadFailuresAreErrors++;
}
public void Dispose()
{
s_loadFailuresAreErrors--;
}
}

public void LogModuleLoadFailure(string simpleName, string filePath)
{
if (_simpleNamesReported.Add(simpleName))
{
string str = $"Failed to load assembly '{simpleName}' from '{filePath}'";

if (String.Compare("System.Private.CoreLib", simpleName, StringComparison.OrdinalIgnoreCase) == 0)
if (s_loadFailuresAreErrors != 0 || String.Compare("System.Private.CoreLib", simpleName, StringComparison.OrdinalIgnoreCase) == 0)
{
_logger.PrintError(str);
}
Expand All @@ -41,7 +56,7 @@ public void LogModuleLoadFailure(string simpleName)
{
string str = $"Failed to load assembly '{simpleName}'";

if (String.Compare("System.Private.CoreLib", simpleName, StringComparison.OrdinalIgnoreCase) == 0)
if (s_loadFailuresAreErrors != 0 || String.Compare("System.Private.CoreLib", simpleName, StringComparison.OrdinalIgnoreCase) == 0)
{
_logger.PrintError(str);
}
Expand Down
76 changes: 70 additions & 6 deletions src/coreclr/tools/dotnet-pgo/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -948,9 +948,11 @@ static int InnerProcessTraceFileMain(CommandLineOptions commandLineOptions)
return -5;
}

if (!p.EventsInProcess.ByEventType<MethodJittingStartedTraceData>().Any())
if (!p.EventsInProcess.ByEventType<MethodJittingStartedTraceData>().Any() &&
!p.EventsInProcess.ByEventType<R2RGetEntryPointTraceData>().Any() &&
!p.EventsInProcess.ByEventType< SampledProfileTraceData>().Any())
{
PrintError($"No managed jit starting data\nWas the trace collected with provider at least \"Microsoft-Windows-DotNETRuntime:0x4000080018:5\"?");
PrintError($"No data in trace for process\nWas the trace collected with provider at least \"Microsoft-Windows-DotNETRuntime:0x4000080018:5\"?");
return -5;
}

Expand Down Expand Up @@ -1014,7 +1016,9 @@ static int InnerProcessTraceFileMain(CommandLineOptions commandLineOptions)
filePathError = true;
}
else
tsc.GetModuleFromPath(fileReference.FullName);
{
tsc.GetModuleFromPath(fileReference.FullName, throwIfNotLoadable: false);
}
}
catch (Internal.TypeSystem.TypeSystemException.BadImageFormatException)
{
Expand All @@ -1034,7 +1038,63 @@ static int InnerProcessTraceFileMain(CommandLineOptions commandLineOptions)

TraceRuntimeDescToTypeSystemDesc idParser = new TraceRuntimeDescToTypeSystemDesc(p, tsc, clrInstanceId.Value);

SortedDictionary<int, ProcessedMethodData> methodsToAttemptToPrepare = new SortedDictionary<int, ProcessedMethodData>();
int mismatchErrors = 0;
foreach (var e in p.EventsInProcess.ByEventType<ModuleLoadUnloadTraceData>())
{
ModuleDesc loadedModule = idParser.ResolveModuleID(e.ModuleID, false);
if (loadedModule == null)
{
PrintWarning($"Unable to find loaded module {e.ModuleILFileName} to verify match");
continue;
}

EcmaModule ecmaModule = loadedModule as EcmaModule;
if (ecmaModule == null)
{
continue;
}

bool matched = false;
bool mismatch = false;
foreach (var debugEntry in ecmaModule.PEReader.ReadDebugDirectory())
{
if (debugEntry.Type == DebugDirectoryEntryType.CodeView)
{
var codeViewData = ecmaModule.PEReader.ReadCodeViewDebugDirectoryData(debugEntry);
if (codeViewData.Path.EndsWith("ni.pdb"))
continue;
if (codeViewData.Guid != e.ManagedPdbSignature)
{
PrintError($"Dll mismatch between assembly located at \"{e.ModuleILPath}\" during trace collection and module \"{tsc.PEReaderToFilePath(ecmaModule.PEReader)}\"");
mismatchErrors++;
mismatch = true;
continue;
}
else
{
matched = true;
}
}
}

if (!matched && !mismatch)
{
PrintMessage($"Unable to validate match between assembly located at \"{e.ModuleILPath}\" during trace collection and module \"{tsc.PEReaderToFilePath(ecmaModule.PEReader)}\"");
}

// TODO find some way to match on MVID as only some dlls have managed pdbs, and this won't find issues with embedded pdbs
}

if (mismatchErrors != 0)
{
PrintError($"{mismatchErrors} mismatch error(s) found");
return -1;
}

// Now that the modules are validated run Init to prepare for the rest of execution
idParser.Init();

SortedDictionary<long, ProcessedMethodData> methodsToAttemptToPrepare = new SortedDictionary<long, ProcessedMethodData>();

if (commandLineOptions.ProcessR2REvents)
{
Expand Down Expand Up @@ -1133,7 +1193,8 @@ MethodMemoryMap GetMethodMemMap()
p,
tsc,
idParser,
clrInstanceId.Value);
clrInstanceId.Value,
s_logger);
}

return methodMemMap;
Expand All @@ -1155,6 +1216,9 @@ MethodMemoryMap GetMethodMemMap()
MethodMemoryMap mmap = GetMethodMemMap();
foreach (var e in p.EventsInProcess.ByEventType<SampledProfileTraceData>())
{
if ((e.TimeStampRelativeMSec < commandLineOptions.ExcludeEventsBefore) && (e.TimeStampRelativeMSec > commandLineOptions.ExcludeEventsAfter))
continue;

var callstack = e.CallStack();
if (callstack == null)
continue;
Expand Down Expand Up @@ -1191,7 +1255,7 @@ MethodMemoryMap GetMethodMemMap()
if (!methodsListedToPrepare.Contains(nextMethod))
{
methodsListedToPrepare.Add(nextMethod);
methodsToAttemptToPrepare.Add((int)e.EventIndex, new ProcessedMethodData(e.TimeStampRelativeMSec, nextMethod, "SampleMethodCaller"));
methodsToAttemptToPrepare.Add(0x100000000 + (int)e.EventIndex, new ProcessedMethodData(e.TimeStampRelativeMSec, nextMethod, "SampleMethodCaller"));
}

if (!callGraph.TryGetValue(nextMethod, out var innerDictionary))
Expand Down
Loading

0 comments on commit 6e0e2d0

Please sign in to comment.