From 2fb0d73e93cffb3b56cd29fea746a4886e6cd7ee Mon Sep 17 00:00:00 2001 From: Peter Liljenberg Date: Fri, 28 Dec 2018 18:16:07 +0100 Subject: [PATCH] Use file-backed mmaps on Linux --- src/coverlet.core/Coverage.cs | 21 ++++- .../Helpers/InstrumentationHelper.cs | 7 ++ .../Instrumentation/Instrumenter.cs | 13 ++- .../Instrumentation/InstrumenterResult.cs | 1 + .../Instrumentation/ModuleTrackerTemplate.cs | 84 +++++++++++-------- test/coverlet.core.tests/CoverageTests.cs | 5 +- .../Helpers/InstrumentationHelperTests.cs | 9 ++ .../ModuleTrackerTemplateTests.cs | 28 +++++-- 8 files changed, 122 insertions(+), 46 deletions(-) diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 2316644..705bdca 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -89,8 +89,22 @@ namespace Coverlet.Core foreach (var result in _results) { - var count = result.HitCandidates.Count + HitsResultHeaderSize; - _resultMemoryMaps.Add(result.HitsResultGuid, MemoryMappedFile.CreateNew(result.HitsResultGuid, count * sizeof(int))); + var size = (result.HitCandidates.Count + 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); } } @@ -270,6 +284,9 @@ namespace Coverlet.Core document.Value.Branches.Remove(branchToRemove.Key); } } + + // 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); } } diff --git a/src/coverlet.core/Helpers/InstrumentationHelper.cs b/src/coverlet.core/Helpers/InstrumentationHelper.cs index 4109426..fcc54d5 100644 --- a/src/coverlet.core/Helpers/InstrumentationHelper.cs +++ b/src/coverlet.core/Helpers/InstrumentationHelper.cs @@ -104,6 +104,13 @@ namespace Coverlet.Core.Helpers }, retryStrategy, 10); } + public static void DeleteHitsFile(string path) + { + // Retry hitting the hits file - retry up to 10 times, since the file could be locked + // See: https://github.com/tonerdo/coverlet/issues/25 + var retryStrategy = CreateRetryStrategy(); + RetryHelper.Retry(() => File.Delete(path), retryStrategy, 10); + } public static bool IsValidFilterExpression(string filter) { diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 264eee5..05d25ee 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -24,6 +24,7 @@ namespace Coverlet.Core.Instrumentation private readonly string[] _excludedFiles; private readonly string[] _excludedAttributes; private InstrumenterResult _result; + private FieldDefinition _customTrackerHitsFilePath; private FieldDefinition _customTrackerHitsArraySize; private FieldDefinition _customTrackerHitsMemoryMapName; private ILProcessor _customTrackerClassConstructorIl; @@ -44,9 +45,15 @@ namespace Coverlet.Core.Instrumentation public InstrumenterResult Instrument() { + string hitsFilePath = Path.Combine( + Path.GetTempPath(), + Path.GetFileNameWithoutExtension(_module) + "_" + _identifier + ); + _result = new InstrumenterResult { Module = Path.GetFileNameWithoutExtension(_module), + HitsFilePath = hitsFilePath, HitsResultGuid = Guid.NewGuid().ToString(), ModulePath = _module }; @@ -91,6 +98,8 @@ namespace Coverlet.Core.Instrumentation // Fixup the custom tracker class constructor, according to all instrumented types Instruction firstInstr = _customTrackerClassConstructorIl.Body.Instructions.First(); _customTrackerClassConstructorIl.InsertBefore(firstInstr, Instruction.Create(OpCodes.Nop)); + _customTrackerClassConstructorIl.InsertBefore(firstInstr, Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath)); + _customTrackerClassConstructorIl.InsertBefore(firstInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsFilePath)); _customTrackerClassConstructorIl.InsertBefore(firstInstr, Instruction.Create(OpCodes.Ldc_I4, _result.HitCandidates.Count)); _customTrackerClassConstructorIl.InsertBefore(firstInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsArraySize)); _customTrackerClassConstructorIl.InsertBefore(firstInstr, Instruction.Create(OpCodes.Ldstr, _result.HitsResultGuid)); @@ -119,7 +128,9 @@ namespace Coverlet.Core.Instrumentation _customTrackerTypeDef.Fields.Add(fieldClone); - if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsMemoryMapName)) + if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsFilePath)) + _customTrackerHitsFilePath = fieldClone; + else if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsMemoryMapName)) _customTrackerHitsMemoryMapName = fieldClone; else if (fieldClone.Name == nameof(ModuleTrackerTemplate.HitsArraySize)) _customTrackerHitsArraySize = fieldClone; diff --git a/src/coverlet.core/Instrumentation/InstrumenterResult.cs b/src/coverlet.core/Instrumentation/InstrumenterResult.cs index aad3f43..4f51cdb 100644 --- a/src/coverlet.core/Instrumentation/InstrumenterResult.cs +++ b/src/coverlet.core/Instrumentation/InstrumenterResult.cs @@ -41,6 +41,7 @@ namespace Coverlet.Core.Instrumentation } public string Module; + public string HitsFilePath; public string HitsResultGuid; public string ModulePath; public string SourceLink; diff --git a/src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs b/src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs index 34a12bc..d591aca 100644 --- a/src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs +++ b/src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs @@ -18,6 +18,7 @@ namespace Coverlet.Core.Instrumentation [ExcludeFromCodeCoverage] public static class ModuleTrackerTemplate { + public static string HitsFilePath; public static string HitsMemoryMapName; public static int HitsArraySize; @@ -88,54 +89,62 @@ namespace Coverlet.Core.Instrumentation if (!createdNew) mutex.WaitOne(); + MemoryMappedFile memoryMap = null; + try { - // Tally hit counts from all threads in memory mapped area - using (var memoryMap = MemoryMappedFile.OpenExisting(HitsMemoryMapName)) + try { - var accessor = memoryMap.CreateViewAccessor(); - using (var buffer = accessor.SafeMemoryMappedViewHandle) + memoryMap = MemoryMappedFile.OpenExisting(HitsMemoryMapName); + } + catch (PlatformNotSupportedException) + { + memoryMap = MemoryMappedFile.CreateFromFile(HitsFilePath, FileMode.Open, null, (HitsArraySize + Coverage.HitsResultHeaderSize) * sizeof(int)); + } + + // Tally hit counts from all threads in memory mapped area + var accessor = memoryMap.CreateViewAccessor(); + using (var buffer = accessor.SafeMemoryMappedViewHandle) + { + unsafe { - unsafe + byte* pointer = null; + buffer.AcquirePointer(ref pointer); + try { - 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 + Coverage.HitsResultUnloadStarted)); + + for (var i = 0; i < HitsArraySize; i++) { - var intPointer = (int*) pointer; + var count = 0; - // 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 + Coverage.HitsResultUnloadStarted)); - - for (var i = 0; i < HitsArraySize; i++) + foreach (var threadHits in threads) { - var count = 0; - - foreach (var threadHits in threads) - { - count += threadHits[i]; - } - - if (count > 0) - { - // There's a header of one int before the hit counts - var hitLocationArrayOffset = intPointer + i + Coverage.HitsResultHeaderSize; - - // No need to use Interlocked here since the mutex ensures only one thread updates - // the shared memory map. - *hitLocationArrayOffset += count; - } + count += threadHits[i]; } - // Signal back to coverage analysis that all hit counts were successfully tallied. - Interlocked.Increment(ref *(intPointer + Coverage.HitsResultUnloadFinished)); - } - finally - { - buffer.ReleasePointer(); + if (count > 0) + { + // There's a header of one int before the hit counts + var hitLocationArrayOffset = intPointer + i + Coverage.HitsResultHeaderSize; + + // No need to use Interlocked here since the mutex ensures only one thread updates + // the shared memory map. + *hitLocationArrayOffset += count; + } } + + // Signal back to coverage analysis that all hit counts were successfully tallied. + Interlocked.Increment(ref *(intPointer + Coverage.HitsResultUnloadFinished)); + } + finally + { + buffer.ReleasePointer(); } } } @@ -143,6 +152,7 @@ namespace Coverlet.Core.Instrumentation finally { mutex.ReleaseMutex(); + memoryMap?.Dispose(); } } diff --git a/test/coverlet.core.tests/CoverageTests.cs b/test/coverlet.core.tests/CoverageTests.cs index ad1d1a3..f6bc9f7 100644 --- a/test/coverlet.core.tests/CoverageTests.cs +++ b/test/coverlet.core.tests/CoverageTests.cs @@ -34,7 +34,10 @@ namespace Coverlet.Core.Tests coverage.PrepareModules(); // The module hit tracker must signal to Coverage that it has done its job, so call it manually - ModuleTrackerTemplate.HitsMemoryMapName = coverage.Results.Single().HitsResultGuid; + var instrumenterResult = coverage.Results.Single(); + ModuleTrackerTemplate.HitsArraySize = instrumenterResult.HitCandidates.Count; + ModuleTrackerTemplate.HitsFilePath = instrumenterResult.HitsFilePath; + ModuleTrackerTemplate.HitsMemoryMapName = instrumenterResult.HitsResultGuid; ModuleTrackerTemplate.UnloadModule(null, null); var result = coverage.GetCoverageResult(); diff --git a/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs b/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs index e009a39..af6fa73 100644 --- a/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs +++ b/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs @@ -53,6 +53,15 @@ namespace Coverlet.Core.Helpers.Tests Assert.False(InstrumentationHelper.IsValidFilterExpression(null)); } + [Fact] + public void TestDeleteHitsFile() + { + var tempFile = Path.GetTempFileName(); + Assert.True(File.Exists(tempFile)); + + InstrumentationHelper.DeleteHitsFile(tempFile); + Assert.False(File.Exists(tempFile)); + } public static IEnumerable GetExcludedFilesReturnsEmptyArgs => new[] diff --git a/test/coverlet.core.tests/Instrumentation/ModuleTrackerTemplateTests.cs b/test/coverlet.core.tests/Instrumentation/ModuleTrackerTemplateTests.cs index 7317b85..6605d36 100644 --- a/test/coverlet.core.tests/Instrumentation/ModuleTrackerTemplateTests.cs +++ b/test/coverlet.core.tests/Instrumentation/ModuleTrackerTemplateTests.cs @@ -1,4 +1,5 @@ using Coverlet.Core.Instrumentation; +using Coverlet.Core.Helpers; using System; using System.Collections.Generic; using System.IO; @@ -17,7 +18,6 @@ namespace Coverlet.Core.Tests.Instrumentation public ModuleTrackerTemplateTestsFixture() { _semaphore.Wait(); - ModuleTrackerTemplate.HitsMemoryMapName = Guid.NewGuid().ToString(); } public void Dispose() @@ -33,18 +33,36 @@ namespace Coverlet.Core.Tests.Instrumentation // Prevent parallel execution of these tests private static readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1); - private MemoryMappedFile _mmap; + private readonly MemoryMappedFile _mmap; public ModuleTrackerTemplateTests() { _semaphore.Wait(); - _mmap = MemoryMappedFile.CreateNew(ModuleTrackerTemplate.HitsMemoryMapName, 100 * sizeof(int)); + + ModuleTrackerTemplate.HitsArraySize = 4; + ModuleTrackerTemplate.HitsMemoryMapName = Guid.NewGuid().ToString(); + ModuleTrackerTemplate.HitsFilePath = Path.Combine(Path.GetTempPath(), $"coverlet.test_{ModuleTrackerTemplate.HitsMemoryMapName}"); + + var size = (ModuleTrackerTemplate.HitsArraySize + Coverage.HitsResultHeaderSize) * sizeof(int); + + try + { + _mmap = MemoryMappedFile.CreateNew(ModuleTrackerTemplate.HitsMemoryMapName, size); + } + catch (PlatformNotSupportedException) + { + _mmap = MemoryMappedFile.CreateFromFile(ModuleTrackerTemplate.HitsFilePath, FileMode.CreateNew, null, size); + } } public void Dispose() { - _mmap.Dispose(); + var hitsFilePath = ModuleTrackerTemplate.HitsFilePath; + _semaphore.Release(); + _mmap.Dispose(); + + InstrumentationHelper.DeleteHitsFile(hitsFilePath); } [Fact] @@ -125,7 +143,7 @@ namespace Coverlet.Core.Tests.Instrumentation // then dropped by UnloadModule the hit counting must be done // in a new thread for each test - ModuleTrackerTemplate.HitsArraySize = hitCounts.Length; + Assert.Equal(ModuleTrackerTemplate.HitsArraySize, hitCounts.Length); var thread = new Thread(() => {