Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Perf: create a tracker per module and use indexes to improve performance #181

Merged
merged 5 commits into from
Sep 1, 2018
Merged

Perf: create a tracker per module and use indexes to improve performance #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
69f67db
Terrible performance for heavily concurrent
Aug 20, 2018
e4caf1e
Adding missing new source file
Aug 20, 2018
58b89a5
Good perf test for 20_000
Aug 20, 2018
7dd7c76
Added tests to ModuleTrackerTemplate
Aug 22, 2018
28f2b0e
renames and cleanup
Aug 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 0 additions & 15 deletions coverlet.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.core.tests", "test
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.console", "src\coverlet.console\coverlet.console.csproj", "{F3DBE7C3-ABBB-4B8B-A6CB-A1D3D607163E}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.tracker", "src\coverlet.tracker\coverlet.tracker.csproj", "{F4273009-536D-4999-A126-B0A2E3AA3E70}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "coverlet.testsubject", "test\coverlet.testsubject\coverlet.testsubject.csproj", "{AE117FAA-C21D-4F23-917E-0C8050614750}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "coverlet.core.performancetest", "test\coverlet.core.performancetest\coverlet.core.performancetest.csproj", "{C68CF6DE-F86C-4BCF-BAB9-7A60C320E1F9}"
Expand Down Expand Up @@ -79,18 +77,6 @@ Global
{F3DBE7C3-ABBB-4B8B-A6CB-A1D3D607163E}.Release|x64.Build.0 = Release|Any CPU
{F3DBE7C3-ABBB-4B8B-A6CB-A1D3D607163E}.Release|x86.ActiveCfg = Release|Any CPU
{F3DBE7C3-ABBB-4B8B-A6CB-A1D3D607163E}.Release|x86.Build.0 = Release|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Debug|x64.ActiveCfg = Debug|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Debug|x64.Build.0 = Debug|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Debug|x86.ActiveCfg = Debug|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Debug|x86.Build.0 = Debug|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Release|Any CPU.Build.0 = Release|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Release|x64.ActiveCfg = Release|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Release|x64.Build.0 = Release|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Release|x86.ActiveCfg = Release|Any CPU
{F4273009-536D-4999-A126-B0A2E3AA3E70}.Release|x86.Build.0 = Release|Any CPU
{AE117FAA-C21D-4F23-917E-0C8050614750}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{AE117FAA-C21D-4F23-917E-0C8050614750}.Debug|Any CPU.Build.0 = Debug|Any CPU
{AE117FAA-C21D-4F23-917E-0C8050614750}.Debug|x64.ActiveCfg = Debug|Any CPU
Expand Down Expand Up @@ -124,7 +110,6 @@ Global
{FA73E423-9790-4F35-B018-3C4E3CA338BA} = {E877EBA4-E78B-4F7D-A2D3-1E070FED04CD}
{E7637CC6-43F7-461A-A0BF-3C14562419BD} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
{F3DBE7C3-ABBB-4B8B-A6CB-A1D3D607163E} = {E877EBA4-E78B-4F7D-A2D3-1E070FED04CD}
{F4273009-536D-4999-A126-B0A2E3AA3E70} = {E877EBA4-E78B-4F7D-A2D3-1E070FED04CD}
{AE117FAA-C21D-4F23-917E-0C8050614750} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
{C68CF6DE-F86C-4BCF-BAB9-7A60C320E1F9} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
EndGlobalSection
Expand Down
30 changes: 14 additions & 16 deletions src/coverlet.core/Coverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,32 +164,30 @@ private void CalculateCoverage()
List<Document> documents = result.Documents.Values.ToList();

using (var fs = new FileStream(result.HitsFilePath, FileMode.Open))
using (var sr = new StreamReader(fs))
using (var br = new BinaryReader(fs))
{
string row;
while ((row = sr.ReadLine()) != null)
int hitCandidatesCount = br.ReadInt32();

// TODO: hitCandidatesCount should be verified against result.HitCandidates.Count

var documentsList = result.Documents.Values.ToList();

for (int i = 0; i < hitCandidatesCount; ++i)
{
var info = row.Split(',');
// Ignore malformed lines
if (info.Length != 5)
continue;
var hitLocation = result.HitCandidates[i];

bool isBranch = info[0] == "B";
var document = documents[int.Parse(info[1])];
var document = documentsList[hitLocation.docIndex];

int start = int.Parse(info[2]);
int hits = int.Parse(info[4]);
int hits = br.ReadInt32();

if (isBranch)
if (hitLocation.isBranch)
{
int ordinal = int.Parse(info[3]);
var branch = document.Branches[(start, ordinal)];
var branch = document.Branches[(hitLocation.start, hitLocation.end)];
branch.Hits += hits;
}
else
{
int end = int.Parse(info[3]);
for (int j = start; j <= end; j++)
for (int j = hitLocation.start; j <= hitLocation.end; j++)
{
var line = document.Lines[j];
line.Hits += hits;
Expand Down
11 changes: 0 additions & 11 deletions src/coverlet.core/Helpers/InstrumentationHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ public static bool HasPdb(string module)
}
}

public static void CopyCoverletDependency(string module)
{
var moduleFileName = Path.GetFileName(module);
if (Path.GetFileName(typeof(Coverage).Assembly.Location) == moduleFileName)
return;

var directory = Path.GetDirectoryName(module);
var assembly = typeof(Coverlet.Tracker.CoverageTracker).Assembly;
File.Copy(assembly.Location, Path.Combine(directory, Path.GetFileName(assembly.Location)), true);
}

public static void BackupOriginalModule(string module, string identifier)
{
var backupPath = GetBackupPath(module, identifier);
Expand Down
155 changes: 128 additions & 27 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
Expand All @@ -7,7 +8,6 @@
using Coverlet.Core.Attributes;
using Coverlet.Core.Helpers;
using Coverlet.Core.Symbols;
using Coverlet.Tracker;

using Mono.Cecil;
using Mono.Cecil.Cil;
Expand All @@ -22,8 +22,12 @@ internal class Instrumenter
private readonly string[] _excludeFilters;
private readonly string[] _includeFilters;
private readonly string[] _excludedFiles;
private readonly static Lazy<MethodInfo> _markExecutedMethodLoader = new Lazy<MethodInfo>(GetMarkExecutedMethod);
private InstrumenterResult _result;
private FieldDefinition _customTrackerHitsArray;
private FieldDefinition _customTrackerHitsFilePath;
private ILProcessor _customTrackerClassConstructorIl;
private TypeDefinition _customTrackerTypeDef;
private MethodReference _customTrackerRecordHitMethod;

public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles)
{
Expand Down Expand Up @@ -51,7 +55,6 @@ public InstrumenterResult Instrument()
};

InstrumentModule();
InstrumentationHelper.CopyCoverletDependency(_module);

return _result;
}
Expand All @@ -67,20 +70,119 @@ private void InstrumentModule()
using (var module = ModuleDefinition.ReadModule(stream, parameters))
{
var types = module.GetTypes();
AddCustomModuleTrackerToModule(module);

foreach (TypeDefinition type in types)
{
var actualType = type.DeclaringType ?? type;
if (!actualType.CustomAttributes.Any(IsExcludeAttribute)
&& actualType.Namespace != "Coverlet.Core.Instrumentation.Tracker"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not add an ExcludeFromCodeCoverage attribute to all the methods in that class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you already removed it 👍 Thanks, it was not needed as you noticed

&& !InstrumentationHelper.IsTypeExcluded(_module, actualType.FullName, _excludeFilters)
&& InstrumentationHelper.IsTypeIncluded(_module, actualType.FullName, _includeFilters))
InstrumentType(type);
}

// Fixup the custom tracker class constructor, according to all instrumented types
Instruction lastInstr = _customTrackerClassConstructorIl.Body.Instructions.Last();
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Ldc_I4, _result.HitCandidates.Count));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Newarr, module.TypeSystem.Int32));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsArray));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsFilePath));

module.Write(stream);
}
}
}

private void AddCustomModuleTrackerToModule(ModuleDefinition module)
{
using (AssemblyDefinition coverletInstrumentationAssembly = AssemblyDefinition.ReadAssembly(typeof(Instrumenter).Assembly.Location))
{
TypeDefinition moduleTrackerTemplate = coverletInstrumentationAssembly.MainModule.GetType(
"Coverlet.Core.Instrumentation", nameof(ModuleTrackerTemplate));

_customTrackerTypeDef = new TypeDefinition(
"Coverlet.Core.Instrumentation.Tracker", Path.GetFileNameWithoutExtension(module.Name) + "_" + _identifier, moduleTrackerTemplate.Attributes);

_customTrackerTypeDef.BaseType = module.TypeSystem.Object;
foreach (FieldDefinition fieldDef in moduleTrackerTemplate.Fields)
{
var fieldClone = new FieldDefinition(fieldDef.Name, fieldDef.Attributes, fieldDef.FieldType);
fieldClone.FieldType = module.ImportReference(fieldClone.FieldType);

_customTrackerTypeDef.Fields.Add(fieldClone);

if (fieldClone.Name == "HitsArray")
_customTrackerHitsArray = fieldClone;
else if (fieldClone.Name == "HitsFilePath")
_customTrackerHitsFilePath = fieldClone;
}

foreach (MethodDefinition methodDef in moduleTrackerTemplate.Methods)
{
MethodDefinition methodOnCustomType = new MethodDefinition(methodDef.Name, methodDef.Attributes, methodDef.ReturnType);

if (methodDef.Name == "RecordHit")
{
foreach (var parameter in methodDef.Parameters)
{
methodOnCustomType.Parameters.Add(new ParameterDefinition(module.ImportReference(parameter.ParameterType)));
}
}

foreach (var variable in methodDef.Body.Variables)
{
methodOnCustomType.Body.Variables.Add(new VariableDefinition(module.ImportReference(variable.VariableType)));
}

methodOnCustomType.Body.InitLocals = methodDef.Body.InitLocals;

ILProcessor ilProcessor = methodOnCustomType.Body.GetILProcessor();
if (methodDef.Name == ".cctor")
_customTrackerClassConstructorIl = ilProcessor;

foreach (Instruction instr in methodDef.Body.Instructions)
{
if (instr.Operand is MethodReference methodReference)
{
if (!methodReference.FullName.Contains(moduleTrackerTemplate.Namespace))
{
// External method references, just import then
instr.Operand = module.ImportReference(methodReference);
}
else
{
// Move to the custom type
instr.Operand = new MethodReference(
methodReference.Name, methodReference.ReturnType, _customTrackerTypeDef);
}
}
else if (instr.Operand is FieldReference fieldReference)
{
instr.Operand = _customTrackerTypeDef.Fields.Single(fd => fd.Name == fieldReference.Name);
}
else if (instr.Operand is TypeReference typeReference)
{
instr.Operand = module.ImportReference(typeReference);
}

ilProcessor.Append(instr);
}

foreach (var handler in methodDef.Body.ExceptionHandlers)
methodOnCustomType.Body.ExceptionHandlers.Add(handler);

_customTrackerTypeDef.Methods.Add(methodOnCustomType);
}

module.Types.Add(_customTrackerTypeDef);
}

Debug.Assert(_customTrackerHitsArray != null);
Debug.Assert(_customTrackerClassConstructorIl != null);
}

private void InstrumentType(TypeDefinition type)
{
var methods = type.GetMethods();
Expand Down Expand Up @@ -143,7 +245,7 @@ private void InstrumentIL(MethodDefinition method)
foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers)
ReplaceExceptionHandlerBoundary(handler, instruction, target);

index += 3;
index += 2;
}

foreach (var _branchTarget in targetedBranchPoints)
Expand All @@ -164,7 +266,7 @@ private void InstrumentIL(MethodDefinition method)
foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers)
ReplaceExceptionHandlerBoundary(handler, instruction, target);

index += 3;
index += 2;
}

index++;
Expand All @@ -188,17 +290,10 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
document.Lines.Add(i, new Line { Number = i, Class = method.DeclaringType.FullName, Method = method.FullName });
}

string marker = $"L,{document.Index},{sequencePoint.StartLine},{sequencePoint.EndLine}";
var entry = (false, document.Index, sequencePoint.StartLine, sequencePoint.EndLine);
_result.HitCandidates.Add(entry);

var pathInstr = Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath);
var markInstr = Instruction.Create(OpCodes.Ldstr, marker);
var callInstr = Instruction.Create(OpCodes.Call, processor.Body.Method.Module.ImportReference(_markExecutedMethodLoader.Value));

processor.InsertBefore(instruction, callInstr);
processor.InsertBefore(callInstr, markInstr);
processor.InsertBefore(markInstr, pathInstr);

return pathInstr;
return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count - 1);
}

private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor processor, Instruction instruction, BranchPoint branchPoint)
Expand All @@ -225,17 +320,28 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
}
);

string marker = $"B,{document.Index},{branchPoint.StartLine},{branchPoint.Ordinal}";
var entry = (true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal);
_result.HitCandidates.Add(entry);

return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count - 1);
}

private Instruction AddInstrumentationInstructions(MethodDefinition method, ILProcessor processor, Instruction instruction, int hitEntryIndex)
{
if (_customTrackerRecordHitMethod == null)
{
_customTrackerRecordHitMethod = new MethodReference(
"RecordHit", method.Module.TypeSystem.Void, _customTrackerTypeDef);
_customTrackerRecordHitMethod.Parameters.Add(new ParameterDefinition(method.Module.TypeSystem.Int32));
}

var pathInstr = Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath);
var markInstr = Instruction.Create(OpCodes.Ldstr, marker);
var callInstr = Instruction.Create(OpCodes.Call, processor.Body.Method.Module.ImportReference(_markExecutedMethodLoader.Value));
var indxInstr = Instruction.Create(OpCodes.Ldc_I4, hitEntryIndex);
var callInstr = Instruction.Create(OpCodes.Call, _customTrackerRecordHitMethod);

processor.InsertBefore(instruction, callInstr);
processor.InsertBefore(callInstr, markInstr);
processor.InsertBefore(markInstr, pathInstr);
processor.InsertBefore(callInstr, indxInstr);

return pathInstr;
return indxInstr;
}

private static void ReplaceInstructionTarget(Instruction instruction, Instruction oldTarget, Instruction newTarget)
Expand Down Expand Up @@ -301,10 +407,5 @@ private static Mono.Cecil.Cil.MethodBody GetMethodBody(MethodDefinition method)
return null;
}
}

private static MethodInfo GetMarkExecutedMethod()
{
return typeof(CoverageTracker).GetMethod(nameof(CoverageTracker.MarkExecuted));
}
}
}
5 changes: 3 additions & 2 deletions src/coverlet.core/Instrumentation/InstrumenterResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public Document()

public string Path;
public int Index;

public Dictionary<int, Line> Lines { get; private set; }
public Dictionary<(int Line, int Ordinal), Branch> Branches { get; private set; }
}
Expand All @@ -38,11 +37,13 @@ internal class InstrumenterResult
public InstrumenterResult()
{
Documents = new Dictionary<string, Document>();
}
HitCandidates = new List<(bool isBranch, int docIndex, int start, int end)>();
}

public string Module;
public string HitsFilePath;
public string ModulePath;
public Dictionary<string, Document> Documents { get; private set; }
public List<(bool isBranch, int docIndex, int start, int end)> HitCandidates { get; private set; }
}
}