Skip to content

Change hits file instrumentation to be memory mapped #241

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 48 additions & 47 deletions src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,73 +164,74 @@ private void CalculateCoverage()
{
foreach (var result in _results)
{
if (!File.Exists(result.HitsFilePath))
if (!Directory.Exists(result.HitsDirectoryPath))
{
// File not instrumented, or nothing in it called. Warn about this?
// File not instrumented. Warn about this?
continue;
}

List<Document> documents = result.Documents.Values.ToList();

using (var fs = new FileStream(result.HitsFilePath, FileMode.Open))
using (var br = new BinaryReader(fs))
foreach (var file in Directory.EnumerateFiles(result.HitsDirectoryPath))
{
int hitCandidatesCount = br.ReadInt32();

// TODO: hitCandidatesCount should be verified against result.HitCandidates.Count
using (var fs = new FileStream(Path.Combine(result.HitsDirectoryPath, file), FileMode.Open))
using (var br = new BinaryReader(fs))
{
int hitCandidatesCount = br.ReadInt32();

var documentsList = result.Documents.Values.ToList();
// TODO: hitCandidatesCount should be verified against result.HitCandidates.Count

for (int i = 0; i < hitCandidatesCount; ++i)
{
var hitLocation = result.HitCandidates[i];
for (int i = 0; i < hitCandidatesCount; ++i)
{
var hitLocation = result.HitCandidates[i];

var document = documentsList[hitLocation.docIndex];
var document = documents[hitLocation.docIndex];

int hits = br.ReadInt32();
int hits = br.ReadInt32();

if (hitLocation.isBranch)
{
var branch = document.Branches[(hitLocation.start, hitLocation.end)];
branch.Hits += hits;
}
else
{
for (int j = hitLocation.start; j <= hitLocation.end; j++)
if (hitLocation.isBranch)
{
var branch = document.Branches[(hitLocation.start, hitLocation.end)];
branch.Hits += hits;
}
else
{
var line = document.Lines[j];
line.Hits += hits;
for (int j = hitLocation.start; j <= hitLocation.end; j++)
{
var line = document.Lines[j];
line.Hits += hits;
}
}
}
}
}

// for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance)
// we'll remove all MoveNext() not covered branch
foreach (var document in result.Documents)
{
List<KeyValuePair<(int, int), Branch>> branchesToRemove = new List<KeyValuePair<(int, int), Branch>>();
foreach (var branch in document.Value.Branches)
{
//if one branch is covered we search the other one only if it's not covered
if (CecilSymbolHelper.IsMoveNext(branch.Value.Method) && branch.Value.Hits > 0)
{
foreach (var moveNextBranch in document.Value.Branches)
{
if (moveNextBranch.Value.Method == branch.Value.Method && moveNextBranch.Value != branch.Value && moveNextBranch.Value.Hits == 0)
{
branchesToRemove.Add(moveNextBranch);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
document.Value.Branches.Remove(branchToRemove.Key);
}
// for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance)
// we'll remove all MoveNext() not covered branch
foreach (var document in result.Documents)
{
List<KeyValuePair<(int, int), Branch>> branchesToRemove = new List<KeyValuePair<(int, int), Branch>>();
foreach (var branch in document.Value.Branches)
{
//if one branch is covered we search the other one only if it's not covered
if (CecilSymbolHelper.IsMoveNext(branch.Value.Method) && branch.Value.Hits > 0)
{
foreach (var moveNextBranch in document.Value.Branches)
{
if (moveNextBranch.Value.Method == branch.Value.Method && moveNextBranch.Value != branch.Value && moveNextBranch.Value.Hits == 0)
{
branchesToRemove.Add(moveNextBranch);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
document.Value.Branches.Remove(branchToRemove.Key);
}
}

InstrumentationHelper.DeleteHitsFile(result.HitsFilePath);
InstrumentationHelper.DeleteHitsDirectory(result.HitsDirectoryPath);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/coverlet.core/Helpers/InstrumentationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ public static void RestoreOriginalModule(string module, string identifier)
}, retryStrategy, 10);
}

public static void DeleteHitsFile(string path)
public static void DeleteHitsDirectory(string path)
{
// Retry hitting the hits file - retry up to 10 times, since the file could be locked
// Retry hitting the hits directory - retry up to 10 times, since the files could be locked
// See: https://github.com/tonerdo/coverlet/issues/25
var retryStrategy = CreateRetryStrategy();
RetryHelper.Retry(() => File.Delete(path), retryStrategy, 10);
RetryHelper.Retry(() => Directory.Delete(path, true), retryStrategy, 10);
}

public static bool IsValidFilterExpression(string filter)
Expand Down
32 changes: 16 additions & 16 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Reflection;

using Coverlet.Core.Attributes;
using Coverlet.Core.Helpers;
Expand All @@ -23,8 +22,8 @@ internal class Instrumenter
private readonly string[] _includeFilters;
private readonly string[] _excludedFiles;
private InstrumenterResult _result;
private FieldDefinition _customTrackerHitsArray;
private FieldDefinition _customTrackerHitsFilePath;
private FieldDefinition _customTrackerHitsArraySize;
private FieldDefinition _customTrackerHitsDirectoryPath;
private ILProcessor _customTrackerClassConstructorIl;
private TypeDefinition _customTrackerTypeDef;
private MethodReference _customTrackerRecordHitMethod;
Expand All @@ -42,15 +41,17 @@ public Instrumenter(string module, string identifier, string[] excludeFilters, s

public InstrumenterResult Instrument()
{
string hitsFilePath = Path.Combine(
string hitsDirectoryPath = Path.Combine(
Path.GetTempPath(),
Path.GetFileNameWithoutExtension(_module) + "_" + _identifier
);

Directory.CreateDirectory(hitsDirectoryPath);

_result = new InstrumenterResult
{
Module = Path.GetFileNameWithoutExtension(_module),
HitsFilePath = hitsFilePath,
HitsDirectoryPath = hitsDirectoryPath,
ModulePath = _module
};

Expand Down Expand Up @@ -88,10 +89,9 @@ private void InstrumentModule()
// 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));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsArraySize));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Ldstr, _result.HitsDirectoryPath));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsDirectoryPath));

module.Write(stream);
}
Expand All @@ -116,17 +116,17 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)

_customTrackerTypeDef.Fields.Add(fieldClone);

if (fieldClone.Name == "HitsArray")
_customTrackerHitsArray = fieldClone;
else if (fieldClone.Name == "HitsFilePath")
_customTrackerHitsFilePath = fieldClone;
if (fieldClone.Name == nameof(ModuleTrackerTemplate.hitsArraySize))
_customTrackerHitsArraySize = fieldClone;
else if (fieldClone.Name == nameof(ModuleTrackerTemplate.hitsDirectoryPath))
_customTrackerHitsDirectoryPath = fieldClone;
}

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

if (methodDef.Name == "RecordHit")
if (methodDef.Name == nameof(ModuleTrackerTemplate.RecordHit))
{
foreach (var parameter in methodDef.Parameters)
{
Expand Down Expand Up @@ -179,7 +179,7 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)
{
handler.CatchType = module.ImportReference(handler.CatchType);
}

methodOnCustomType.Body.ExceptionHandlers.Add(handler);
}

Expand All @@ -189,7 +189,7 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)
module.Types.Add(_customTrackerTypeDef);
}

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

Expand Down
2 changes: 1 addition & 1 deletion src/coverlet.core/Instrumentation/InstrumenterResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public InstrumenterResult()
}

public string Module;
public string HitsFilePath;
public string HitsDirectoryPath;
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; }
Expand Down
Loading