Skip to content

Revert "Merge pull request #276 from petli/memory-mapped-hit-counts" #322

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

Merged
merged 1 commit into from
Jan 27, 2019
Merged

Revert "Merge pull request #276 from petli/memory-mapped-hit-counts" #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 12 additions & 38 deletions src/coverlet.core/Coverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.IO.MemoryMappedFiles;
using System.Linq;

using Coverlet.Core.Enums;
Expand All @@ -27,15 +26,11 @@ public class Coverage
private bool _useSourceLink;
private List<InstrumenterResult> _results;

private readonly Dictionary<string, MemoryMappedFile> _resultMemoryMaps = new Dictionary<string, MemoryMappedFile>();

public string Identifier
{
get { return _identifier; }
}

internal IEnumerable<InstrumenterResult> Results => _results;

public Coverage(string module, string[] includeFilters, string[] includeDirectories, string[] excludeFilters, string[] excludedSourceFiles, string[] excludeAttributes, string mergeWith, bool useSourceLink)
{
_module = module;
Expand Down Expand Up @@ -82,26 +77,6 @@ public void PrepareModules()
}
}
}

foreach (var result in _results)
{
var size = (result.HitCandidates.Count + ModuleTrackerTemplate.HitsResultHeaderSize) * sizeof(int);

MemoryMappedFile mmap;

try
{
// Try using a named memory map not backed by a file (currently only supported on Windows)
mmap = MemoryMappedFile.CreateNew(result.HitsResultGuid, size);
}
catch (PlatformNotSupportedException)
{
// Fall back on a file-backed memory map
mmap = MemoryMappedFile.CreateFromFile(result.HitsFilePath, FileMode.CreateNew, null, size);
}

_resultMemoryMaps.Add(result.HitsResultGuid, mmap);
}
}

public CoverageResult GetCoverageResult()
Expand Down Expand Up @@ -208,6 +183,12 @@ private void CalculateCoverage()
{
foreach (var result in _results)
{
if (!File.Exists(result.HitsFilePath))
{
// File not instrumented, or nothing in it called. Warn about this?
continue;
}

List<Document> documents = result.Documents.Values.ToList();
if (_useSourceLink && result.SourceLink != null)
{
Expand All @@ -219,26 +200,20 @@ private void CalculateCoverage()
}
}

// Read hit counts from the memory mapped area, disposing it when done
using (var mmapFile = _resultMemoryMaps[result.HitsResultGuid])
using (var fs = new FileStream(result.HitsFilePath, FileMode.Open))
using (var br = new BinaryReader(fs))
{
var mmapAccessor = mmapFile.CreateViewAccessor();

var unloadStarted = mmapAccessor.ReadInt32(ModuleTrackerTemplate.HitsResultUnloadStarted * sizeof(int));
var unloadFinished = mmapAccessor.ReadInt32(ModuleTrackerTemplate.HitsResultUnloadFinished * sizeof(int));
int hitCandidatesCount = br.ReadInt32();

if (unloadFinished < unloadStarted)
{
throw new Exception($"Hit counts only partially reported for {result.Module}");
}
// TODO: hitCandidatesCount should be verified against result.HitCandidates.Count

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

for (int i = 0; i < result.HitCandidates.Count; ++i)
for (int i = 0; i < hitCandidatesCount; ++i)
{
var hitLocation = result.HitCandidates[i];
var document = documentsList[hitLocation.docIndex];
var hits = mmapAccessor.ReadInt32((i + ModuleTrackerTemplate.HitsResultHeaderSize) * sizeof(int));
int hits = br.ReadInt32();

if (hitLocation.isBranch)
{
Expand Down Expand Up @@ -281,7 +256,6 @@ private void CalculateCoverage()
}
}

// There's only a hits file on Linux, but if the file doesn't exist this is just a no-op
InstrumentationHelper.DeleteHitsFile(result.HitsFilePath);
}
}
Expand Down
14 changes: 4 additions & 10 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ internal class Instrumenter
private readonly string[] _excludedAttributes;
private readonly bool _isCoreLibrary;
private InstrumenterResult _result;
private FieldDefinition _customTrackerHitsFilePath;
private FieldDefinition _customTrackerHitsArray;
private FieldDefinition _customTrackerHitsMemoryMapName;
private FieldDefinition _customTrackerHitsFilePath;
private ILProcessor _customTrackerClassConstructorIl;
private TypeDefinition _customTrackerTypeDef;
private MethodReference _customTrackerRegisterUnloadEventsMethod;
Expand Down Expand Up @@ -59,7 +58,6 @@ public InstrumenterResult Instrument()
{
Module = Path.GetFileNameWithoutExtension(_module),
HitsFilePath = hitsFilePath,
HitsResultGuid = Guid.NewGuid().ToString(),
ModulePath = _module
};

Expand Down Expand Up @@ -127,8 +125,6 @@ private void InstrumentModule()
_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.Ldstr, _result.HitsResultGuid));
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsMemoryMapName));

if (containsAppContext)
{
Expand Down Expand Up @@ -174,12 +170,10 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)

_customTrackerTypeDef.Fields.Add(fieldClone);

if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsFilePath))
_customTrackerHitsFilePath = fieldClone;
else if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsMemoryMapName))
_customTrackerHitsMemoryMapName = fieldClone;
else if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsArray))
if (fieldClone.Name == "HitsArray")
_customTrackerHitsArray = fieldClone;
else if (fieldClone.Name == "HitsFilePath")
_customTrackerHitsFilePath = fieldClone;
}

foreach (MethodDefinition methodDef in moduleTrackerTemplate.Methods)
Expand Down
1 change: 0 additions & 1 deletion src/coverlet.core/Instrumentation/InstrumenterResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public InstrumenterResult()
public string Module;
public string[] AsyncMachineStateMethod;
public string HitsFilePath;
public string HitsResultGuid;
public string ModulePath;
public string SourceLink;
public Dictionary<string, Document> Documents { get; private set; }
Expand Down
92 changes: 35 additions & 57 deletions src/coverlet.template/ModuleTrackerTemplate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.IO.MemoryMappedFiles;
using System.Threading;

namespace Coverlet.Core.Instrumentation
Expand All @@ -17,12 +16,7 @@ namespace Coverlet.Core.Instrumentation
[ExcludeFromCodeCoverage]
public static class ModuleTrackerTemplate
{
public const int HitsResultHeaderSize = 2;
public const int HitsResultUnloadStarted = 0;
public const int HitsResultUnloadFinished = 1;

public static string HitsFilePath;
public static string HitsMemoryMapName;
public static int[] HitsArray;

static ModuleTrackerTemplate()
Expand Down Expand Up @@ -63,72 +57,56 @@ public static void UnloadModule(object sender, EventArgs e)

// The same module can be unloaded multiple times in the same process via different app domains.
// Use a global mutex to ensure no concurrent access.
using (var mutex = new Mutex(true, HitsMemoryMapName + "_Mutex", out bool createdNew))
using (var mutex = new Mutex(true, Path.GetFileNameWithoutExtension(HitsFilePath) + "_Mutex", out bool createdNew))
{
if (!createdNew)
mutex.WaitOne();

MemoryMappedFile memoryMap = null;

bool failedToCreateNewHitsFile = false;
try
{
try
using (var fs = new FileStream(HitsFilePath, FileMode.CreateNew))
using (var bw = new BinaryWriter(fs))
{
memoryMap = MemoryMappedFile.OpenExisting(HitsMemoryMapName);
}
catch (PlatformNotSupportedException)
{
memoryMap = MemoryMappedFile.CreateFromFile(HitsFilePath, FileMode.Open, null, (HitsArray.Length + HitsResultHeaderSize) * sizeof(int));
bw.Write(hitsArray.Length);
foreach (int hitCount in hitsArray)
{
bw.Write(hitCount);
}
}
}
catch
{
failedToCreateNewHitsFile = true;
}

// Tally hit counts from all threads in memory mapped area
var accessor = memoryMap.CreateViewAccessor();
using (var buffer = accessor.SafeMemoryMappedViewHandle)
if (failedToCreateNewHitsFile)
{
// Update the number of hits by adding value on disk with the ones on memory.
// This path should be triggered only in the case of multiple AppDomain unloads.
using (var fs = new FileStream(HitsFilePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None))
using (var br = new BinaryReader(fs))
using (var bw = new BinaryWriter(fs))
{
unsafe
int hitsLength = br.ReadInt32();
if (hitsLength != hitsArray.Length)
{
byte* pointer = null;
buffer.AcquirePointer(ref pointer);
try
{
var intPointer = (int*) pointer;

// Signal back to coverage analysis that we've started transferring hit counts.
// Use interlocked here to ensure a memory barrier before the Coverage class reads
// the shared data.
Interlocked.Increment(ref *(intPointer + HitsResultUnloadStarted));

for (var i = 0; i < hitsArray.Length; i++)
{
var count = hitsArray[i];

// By only modifying the memory map pages where there have been hits
// unnecessary allocation of all-zero pages is avoided.
if (count > 0)
{
var hitLocationArrayOffset = intPointer + i + HitsResultHeaderSize;

// No need to use Interlocked here since the mutex ensures only one thread updates
// the shared memory map.
*hitLocationArrayOffset += count;
}
}
throw new InvalidOperationException(
$"{HitsFilePath} has {hitsLength} entries but on memory {nameof(HitsArray)} has {hitsArray.Length}");
}

// Signal back to coverage analysis that all hit counts were successfully tallied.
Interlocked.Increment(ref *(intPointer + HitsResultUnloadFinished));
}
finally
{
buffer.ReleasePointer();
}
for (int i = 0; i < hitsLength; ++i)
{
int oldHitCount = br.ReadInt32();
bw.Seek(-sizeof(int), SeekOrigin.Current);
bw.Write(hitsArray[i] + oldHitCount);
}
}
}
finally
{
mutex.ReleaseMutex();
memoryMap?.Dispose();
}

// On purpose this is not under a try-finally: it is better to have an exception if there was any error writing the hits file
// this case is relevant when instrumenting corelib since multiple processes can be running against the same instrumented dll.
mutex.ReleaseMutex();
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/coverlet.template/coverlet.template.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>

</Project>
13 changes: 1 addition & 12 deletions test/coverlet.core.tests/CoverageTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,9 @@

using Coverlet.Core;
using System.Collections.Generic;
using System.Linq;
using Coverlet.Core.Instrumentation;
using Coverlet.Core.Tests.Instrumentation;

namespace Coverlet.Core.Tests
{
[Collection(nameof(ModuleTrackerTemplate))]
public class CoverageTests
{
[Fact]
Expand All @@ -26,21 +22,14 @@ public void TestCoverage()
File.Copy(module, Path.Combine(directory.FullName, Path.GetFileName(module)), true);
File.Copy(pdb, Path.Combine(directory.FullName, Path.GetFileName(pdb)), true);

// TODO: Mimic hits by calling ModuleTrackerTemplate.RecordHit before Unload
// TODO: Find a way to mimick hits

// Since Coverage only instruments dependancies, we need a fake module here
var testModule = Path.Combine(directory.FullName, "test.module.dll");

var coverage = new Coverage(testModule, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), string.Empty, false);
coverage.PrepareModules();

// The module hit tracker must signal to Coverage that it has done its job, so call it manually
var instrumenterResult = coverage.Results.Single();
ModuleTrackerTemplate.HitsArray = new int[instrumenterResult.HitCandidates.Count + ModuleTrackerTemplate.HitsResultHeaderSize];
ModuleTrackerTemplate.HitsFilePath = instrumenterResult.HitsFilePath;
ModuleTrackerTemplate.HitsMemoryMapName = instrumenterResult.HitsResultGuid;
ModuleTrackerTemplate.UnloadModule(null, null);

var result = coverage.GetCoverageResult();

Assert.NotEmpty(result.Modules);
Expand Down
Loading