Skip to content

Commit 69f67db

Browse files
author
Paulo Janotti
committed
Terrible performance for heavily concurrent
1 parent 30c000b commit 69f67db

File tree

11 files changed

+143
-110
lines changed

11 files changed

+143
-110
lines changed

src/coverlet.core/Coverage.cs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -164,32 +164,30 @@ private void CalculateCoverage()
164164
List<Document> documents = result.Documents.Values.ToList();
165165

166166
using (var fs = new FileStream(result.HitsFilePath, FileMode.Open))
167-
using (var sr = new StreamReader(fs))
167+
using (var br = new BinaryReader(fs))
168168
{
169-
string row;
170-
while ((row = sr.ReadLine()) != null)
169+
int hitCandidatesCount = br.ReadInt32();
170+
171+
// TODO: hitCandidatesCount should be verified against result.HitCandidates.Count
172+
173+
var documentsList = result.Documents.Values.ToList();
174+
175+
for (int i = 0; i < hitCandidatesCount; ++i)
171176
{
172-
var info = row.Split(',');
173-
// Ignore malformed lines
174-
if (info.Length != 5)
175-
continue;
177+
var hitLocation = result.HitCandidates[i];
176178

177-
bool isBranch = info[0] == "B";
178-
var document = documents[int.Parse(info[1])];
179+
var document = documentsList[hitLocation.docIndex];
179180

180-
int start = int.Parse(info[2]);
181-
int hits = int.Parse(info[4]);
181+
int hits = br.ReadInt32();
182182

183-
if (isBranch)
183+
if (hitLocation.isBranch)
184184
{
185-
int ordinal = int.Parse(info[3]);
186-
var branch = document.Branches[(start, ordinal)];
185+
var branch = document.Branches[(hitLocation.start, hitLocation.end)];
187186
branch.Hits += hits;
188187
}
189188
else
190189
{
191-
int end = int.Parse(info[3]);
192-
for (int j = start; j <= end; j++)
190+
for (int j = hitLocation.start; j <= hitLocation.end; j++)
193191
{
194192
var line = document.Lines[j];
195193
line.Hits += hits;

src/coverlet.core/Helpers/InstrumentationHelper.cs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,6 @@ public static bool HasPdb(string module)
4040
}
4141
}
4242

43-
public static void CopyCoverletDependency(string module)
44-
{
45-
var moduleFileName = Path.GetFileName(module);
46-
if (Path.GetFileName(typeof(Coverage).Assembly.Location) == moduleFileName)
47-
return;
48-
49-
var directory = Path.GetDirectoryName(module);
50-
var assembly = typeof(Coverlet.Tracker.CoverageTracker).Assembly;
51-
File.Copy(assembly.Location, Path.Combine(directory, Path.GetFileName(assembly.Location)), true);
52-
}
53-
5443
public static void BackupOriginalModule(string module, string identifier)
5544
{
5645
var backupPath = GetBackupPath(module, identifier);

src/coverlet.core/Instrumentation/Instrumenter.cs

Lines changed: 124 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics;
23
using System.Diagnostics.CodeAnalysis;
34
using System.IO;
45
using System.Linq;
@@ -7,7 +8,6 @@
78
using Coverlet.Core.Attributes;
89
using Coverlet.Core.Helpers;
910
using Coverlet.Core.Symbols;
10-
using Coverlet.Tracker;
1111

1212
using Mono.Cecil;
1313
using Mono.Cecil.Cil;
@@ -22,8 +22,11 @@ internal class Instrumenter
2222
private readonly string[] _excludeFilters;
2323
private readonly string[] _includeFilters;
2424
private readonly string[] _excludedFiles;
25-
private readonly static Lazy<MethodInfo> _markExecutedMethodLoader = new Lazy<MethodInfo>(GetMarkExecutedMethod);
2625
private InstrumenterResult _result;
26+
private FieldDefinition _customModuleTrackerHitsArray;
27+
private FieldDefinition _customModuleTrackerHitsFilePath;
28+
private ILProcessor _customModuleTrackerClassConstructorIl;
29+
private MethodReference _cachedInterlockedIncMethod;
2730

2831
public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles)
2932
{
@@ -51,7 +54,6 @@ public InstrumenterResult Instrument()
5154
};
5255

5356
InstrumentModule();
54-
InstrumentationHelper.CopyCoverletDependency(_module);
5557

5658
return _result;
5759
}
@@ -67,20 +69,109 @@ private void InstrumentModule()
6769
using (var module = ModuleDefinition.ReadModule(stream, parameters))
6870
{
6971
var types = module.GetTypes();
72+
AddCustomModuleTrackerToModule(module);
73+
7074
foreach (TypeDefinition type in types)
7175
{
7276
var actualType = type.DeclaringType ?? type;
7377
if (!actualType.CustomAttributes.Any(IsExcludeAttribute)
78+
&& actualType.Namespace != "Coverlet.Core.Instrumentation.Tracker"
7479
&& !InstrumentationHelper.IsTypeExcluded(_module, actualType.FullName, _excludeFilters)
7580
&& InstrumentationHelper.IsTypeIncluded(_module, actualType.FullName, _includeFilters))
7681
InstrumentType(type);
7782
}
7883

84+
// Fixup the custom tracker class constructor, according to all instrumented types
85+
Instruction lastInstr = _customModuleTrackerClassConstructorIl.Body.Instructions.Last();
86+
_customModuleTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Ldc_I4, _result.HitCandidates.Count));
87+
_customModuleTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Newarr, module.TypeSystem.Int32));
88+
_customModuleTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customModuleTrackerHitsArray));
89+
_customModuleTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath));
90+
_customModuleTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customModuleTrackerHitsFilePath));
91+
7992
module.Write(stream);
8093
}
8194
}
8295
}
8396

97+
private void AddCustomModuleTrackerToModule(ModuleDefinition module)
98+
{
99+
using (AssemblyDefinition xad = AssemblyDefinition.ReadAssembly(typeof(Instrumenter).Assembly.Location))
100+
{
101+
TypeDefinition moduleTrackerTemplate = xad.MainModule.GetType(
102+
"Coverlet.Core.Instrumentation", nameof(ModuleTrackerTemplate));
103+
104+
TypeDefinition customTrackerTypeDef = new TypeDefinition(
105+
"Coverlet.Core.Instrumentation.Tracker", Path.GetFileNameWithoutExtension(module.Name) + "_" + _identifier, moduleTrackerTemplate.Attributes);
106+
107+
customTrackerTypeDef.BaseType = module.TypeSystem.Object;
108+
foreach (FieldDefinition fieldDef in moduleTrackerTemplate.Fields)
109+
{
110+
var fieldClone = new FieldDefinition(fieldDef.Name, fieldDef.Attributes, fieldDef.FieldType);
111+
customTrackerTypeDef.Fields.Add(fieldClone);
112+
113+
if (fieldClone.Name == "HitsArray")
114+
_customModuleTrackerHitsArray = fieldClone;
115+
else if (fieldClone.Name == "HitsFilePath")
116+
_customModuleTrackerHitsFilePath = fieldClone;
117+
}
118+
119+
foreach (MethodDefinition methodDef in moduleTrackerTemplate.Methods)
120+
{
121+
MethodDefinition methodOnCustomType = new MethodDefinition(methodDef.Name, methodDef.Attributes, methodDef.ReturnType);
122+
123+
foreach (var variable in methodDef.Body.Variables)
124+
{
125+
methodOnCustomType.Body.Variables.Add(new VariableDefinition(module.ImportReference(variable.VariableType)));
126+
}
127+
128+
methodOnCustomType.Body.InitLocals = methodDef.Body.InitLocals;
129+
130+
ILProcessor ilProcessor = methodOnCustomType.Body.GetILProcessor();
131+
if (methodDef.Name == ".cctor")
132+
_customModuleTrackerClassConstructorIl = ilProcessor;
133+
134+
foreach (Instruction instr in methodDef.Body.Instructions)
135+
{
136+
if (instr.Operand is MethodReference methodReference)
137+
{
138+
if (!methodReference.FullName.Contains(moduleTrackerTemplate.Namespace))
139+
{
140+
// External method references, just import then
141+
instr.Operand = module.ImportReference(methodReference);
142+
}
143+
else
144+
{
145+
// Move to the custom type
146+
instr.Operand = new MethodReference(
147+
methodReference.Name, methodReference.ReturnType, customTrackerTypeDef);
148+
}
149+
}
150+
else if (instr.Operand is FieldReference fieldReference)
151+
{
152+
instr.Operand = customTrackerTypeDef.Fields.Single(fd => fd.Name == fieldReference.Name);
153+
}
154+
else if (instr.Operand is TypeReference typeReference)
155+
{
156+
instr.Operand = module.ImportReference(typeReference);
157+
}
158+
159+
ilProcessor.Append(instr);
160+
}
161+
162+
foreach (var handler in methodDef.Body.ExceptionHandlers)
163+
methodOnCustomType.Body.ExceptionHandlers.Add(handler);
164+
165+
customTrackerTypeDef.Methods.Add(methodOnCustomType);
166+
}
167+
168+
module.Types.Add(customTrackerTypeDef);
169+
}
170+
171+
Debug.Assert(_customModuleTrackerHitsArray != null);
172+
Debug.Assert(_customModuleTrackerClassConstructorIl != null);
173+
}
174+
84175
private void InstrumentType(TypeDefinition type)
85176
{
86177
var methods = type.GetMethods();
@@ -143,7 +234,7 @@ private void InstrumentIL(MethodDefinition method)
143234
foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers)
144235
ReplaceExceptionHandlerBoundary(handler, instruction, target);
145236

146-
index += 3;
237+
index += 5;
147238
}
148239

149240
foreach (var _branchTarget in targetedBranchPoints)
@@ -164,7 +255,7 @@ private void InstrumentIL(MethodDefinition method)
164255
foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers)
165256
ReplaceExceptionHandlerBoundary(handler, instruction, target);
166257

167-
index += 3;
258+
index += 5;
168259
}
169260

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

191-
string marker = $"L,{document.Index},{sequencePoint.StartLine},{sequencePoint.EndLine}";
192-
193-
var pathInstr = Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath);
194-
var markInstr = Instruction.Create(OpCodes.Ldstr, marker);
195-
var callInstr = Instruction.Create(OpCodes.Call, processor.Body.Method.Module.ImportReference(_markExecutedMethodLoader.Value));
196-
197-
processor.InsertBefore(instruction, callInstr);
198-
processor.InsertBefore(callInstr, markInstr);
199-
processor.InsertBefore(markInstr, pathInstr);
282+
var entry = (false, document.Index, sequencePoint.StartLine, sequencePoint.EndLine);
283+
_result.HitCandidates.Add(entry);
200284

201-
return pathInstr;
285+
return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count);
202286
}
203287

204288
private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor processor, Instruction instruction, BranchPoint branchPoint)
@@ -225,17 +309,34 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
225309
}
226310
);
227311

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

230-
var pathInstr = Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath);
231-
var markInstr = Instruction.Create(OpCodes.Ldstr, marker);
232-
var callInstr = Instruction.Create(OpCodes.Call, processor.Body.Method.Module.ImportReference(_markExecutedMethodLoader.Value));
315+
return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count);
316+
}
317+
318+
private Instruction AddInstrumentationInstructions(MethodDefinition method, ILProcessor processor, Instruction instruction, int hitEntryIndex)
319+
{
320+
if (_cachedInterlockedIncMethod == null)
321+
{
322+
_cachedInterlockedIncMethod = new MethodReference(
323+
"Increment", method.Module.TypeSystem.Int32, method.Module.ImportReference(typeof(System.Threading.Interlocked)));
324+
_cachedInterlockedIncMethod.Parameters.Add(new ParameterDefinition(new ByReferenceType(method.Module.TypeSystem.Int32)));
325+
}
233326

234-
processor.InsertBefore(instruction, callInstr);
235-
processor.InsertBefore(callInstr, markInstr);
236-
processor.InsertBefore(markInstr, pathInstr);
327+
var sfldInstr = Instruction.Create(OpCodes.Ldsfld, _customModuleTrackerHitsArray);
328+
var indxInstr = Instruction.Create(OpCodes.Ldc_I4, hitEntryIndex);
329+
var arefInstr = Instruction.Create(OpCodes.Ldelema, method.Module.TypeSystem.Int32);
330+
var callInstr = Instruction.Create(OpCodes.Call, _cachedInterlockedIncMethod);
331+
var popInstr = Instruction.Create(OpCodes.Pop);
237332

238-
return pathInstr;
333+
processor.InsertBefore(instruction, popInstr);
334+
processor.InsertBefore(popInstr, callInstr);
335+
processor.InsertBefore(callInstr, arefInstr);
336+
processor.InsertBefore(arefInstr, indxInstr);
337+
processor.InsertBefore(indxInstr, sfldInstr);
338+
339+
return sfldInstr;
239340
}
240341

241342
private static void ReplaceInstructionTarget(Instruction instruction, Instruction oldTarget, Instruction newTarget)
@@ -301,10 +402,5 @@ private static Mono.Cecil.Cil.MethodBody GetMethodBody(MethodDefinition method)
301402
return null;
302403
}
303404
}
304-
305-
private static MethodInfo GetMarkExecutedMethod()
306-
{
307-
return typeof(CoverageTracker).GetMethod(nameof(CoverageTracker.MarkExecuted));
308-
}
309405
}
310406
}

src/coverlet.core/Instrumentation/InstrumenterResult.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public Document()
2828

2929
public string Path;
3030
public int Index;
31-
3231
public Dictionary<int, Line> Lines { get; private set; }
3332
public Dictionary<(int Line, int Ordinal), Branch> Branches { get; private set; }
3433
}
@@ -38,11 +37,13 @@ internal class InstrumenterResult
3837
public InstrumenterResult()
3938
{
4039
Documents = new Dictionary<string, Document>();
41-
}
40+
HitCandidates = new List<(bool isBranch, int docIndex, int start, int end)>();
41+
}
4242

4343
public string Module;
4444
public string HitsFilePath;
4545
public string ModulePath;
4646
public Dictionary<string, Document> Documents { get; private set; }
47+
public List<(bool isBranch, int docIndex, int start, int end)> HitCandidates { get; private set; }
4748
}
4849
}

src/coverlet.core/coverlet.core.csproj

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,4 @@
1212
<PackageReference Include="Microsoft.Extensions.FileSystemGlobbing" Version="2.0.1" />
1313
</ItemGroup>
1414

15-
<ItemGroup>
16-
<ProjectReference Include="$(MSBuildThisFileDirectory)..\coverlet.tracker\coverlet.tracker.csproj" />
17-
</ItemGroup>
18-
1915
</Project>

src/coverlet.tracker/Extensions/DictionaryExtensions.cs

Lines changed: 0 additions & 18 deletions
This file was deleted.

src/coverlet.tracker/Properties/AssemblyInfo.cs

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/coverlet.tracker/coverlet.tracker.csproj

Lines changed: 0 additions & 7 deletions
This file was deleted.
-596 Bytes
Binary file not shown.

test/coverlet.core.performancetest/PerformanceTest.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ namespace coverlet.core.performancetest
1414
/// </summary>
1515
public class PerformanceTest
1616
{
17-
[Theory(Skip = "Only enabled when explicitly testing performance.")]
17+
[Theory(/*Skip = "Only enabled when explicitly testing performance."*/)]
1818
[InlineData(150)]
19+
// [InlineData(20_000)]
1920
public void TestPerformance(int iterations)
2021
{
2122
var big = new BigClass();

0 commit comments

Comments
 (0)