diff --git a/AltCover.Engine/Cobertura.fs b/AltCover.Engine/Cobertura.fs index fc996e3a..d9599d5b 100644 --- a/AltCover.Engine/Cobertura.fs +++ b/AltCover.Engine/Cobertura.fs @@ -1,6 +1,7 @@ namespace AltCover open System +open System.Collections.Generic open System.IO open System.Xml.Linq open System.Globalization @@ -76,12 +77,15 @@ module internal Cobertura = [] - let extractSource (values: (string * #(string seq)) seq) = + let extractSource (values: ((string * string seq) * string list) seq) = let starter = values |> Seq.head match Seq.length values with | 1 -> starter |> fst | _ -> // extract longest common prefix + let files = + values |> Seq.map fst |> Seq.collect snd + let l = values |> Seq.map (fun (_, split) -> Seq.length split) @@ -107,10 +111,11 @@ module internal Cobertura = |> Path.Combine let trimmed = prefix.Length - let source = fst starter + let source = starter |> fst |> fst - source.Substring(0, trimmed).Trim('\\') - + String([| Path.DirectorySeparatorChar |]) + (source.Substring(0, trimmed).Trim('\\') + + String([| Path.DirectorySeparatorChar |]), + files) [ Seq.map (fun s -> s.Attribute(attribute.X).Value) - |> Seq.filter (fun a -> a |> String.IsNullOrWhiteSpace |> not) - |> Seq.map Path.GetDirectoryName - |> Seq.filter (String.IsNullOrWhiteSpace >> not) - |> Seq.distinct - |> Seq.sort - let groupable = - rawsources |> Seq.map (fun x -> (x, splitPath x)) + let table = + Dictionary() + + let rawsources = + files + |> Seq.filter (fun a -> + let fail = + a |> String.IsNullOrWhiteSpace |> not + + if fail then + table[Option.ofObj a] <- String.Empty + + fail) + |> Seq.map (fun a -> a, Path.GetDirectoryName a) + |> Seq.filter (fun (a, d) -> + let fail = + d |> String.IsNullOrWhiteSpace |> not + + if fail then + table[Option.ofObj a] <- a + + fail) + |> Seq.groupBy snd + |> Seq.map (fun (a, s) -> a, s |> Seq.map fst) + |> Seq.sortBy fst // seq of (directory, files full names) - let groups = + let groupable = // seq of ((directory, files full names), facets) + rawsources + |> Seq.map (fun x -> (x, x |> fst |> splitPath)) + + let groups = // seq of (root, seq of ((directory, files full names), facets)) groupable |> Seq.groupBy (snd >> grouping) - let results = + let results = // throws away everything but the sources groups |> Seq.map (snd >> extractSource) results |> Seq.iter (fun f -> target.Descendants("sources".X) - |> Seq.iter _.Add(XElement("source".X, XText(f)))) + |> Seq.iter _.Add(XElement("source".X, XText(fst f)))) - results - |> Seq.map (fun p -> p.Replace('\\', '/').Trim('/')) + results // TODO - make look-up table + |> Seq.iter (fun (p, files) -> + let prefix = + 1 + p.Replace('\\', '/').Trim('/').Length + + files + |> Seq.iter (fun f -> table[Some f] <- f.Substring(prefix)) + + ) + + table let internal nCover (report: XDocument) (packages: XElement) = @@ -239,13 +275,8 @@ module internal Cobertura = (hits, total) ((name, document: string), method) = - let normalized = document.Replace('\\', '/') - let filename = - sources - |> Seq.tryFind (fun s -> normalized.StartsWith(s, StringComparison.Ordinal)) - |> Option.map (fun start -> document.Substring(start.Length + 1)) - |> Option.defaultValue document + sources[Option.ofObj document] let ``class`` = XElement( @@ -511,13 +542,9 @@ module internal Cobertura = ((name, fileid), methodSet) = let document = files |> Map.find fileid - let normalized = document.Replace('\\', '/') let filename = - sources - |> Seq.tryFind (fun s -> normalized.StartsWith(s, StringComparison.Ordinal)) - |> Option.map (fun start -> document.Substring(start.Length + 1)) - |> Option.defaultValue document + sources[Option.ofObj document] let ``class`` = XElement( diff --git a/AltCover.Engine/Instrument.fs b/AltCover.Engine/Instrument.fs index c2171319..a0729394 100644 --- a/AltCover.Engine/Instrument.fs +++ b/AltCover.Engine/Instrument.fs @@ -364,14 +364,14 @@ module internal Instrument = updateStrongNaming definition pair - definition.MainModule.GetTypes() - |> Seq.iter (fun t -> - if - t.IsPublic - && (not - <| t.FullName.StartsWith("AltCover", StringComparison.Ordinal)) - then - t.IsPublic <- false) + //definition.MainModule.GetTypes() + //|> Seq.iter (fun t -> + // if + // t.IsPublic + // && (not + // <| t.FullName.StartsWith("AltCover", StringComparison.Ordinal)) + // then + // t.IsPublic <- false) injectInstrumentation definition diff --git a/AltCover.Engine/Visitor.fs b/AltCover.Engine/Visitor.fs index ce11fddf..4ee0dd00 100644 --- a/AltCover.Engine/Visitor.fs +++ b/AltCover.Engine/Visitor.fs @@ -1193,19 +1193,22 @@ module internal Visitor = null | _ -> seq + let getSequencePoint (dbg: IDictionary) (i: Instruction) = + dbg.TryGetValue(i.Offset) |> snd + let internal findEffectiveSequencePoint genuine - (dbg: MethodDebugInformation) + (dbg: Dictionary) (instructions: Instruction seq) = instructions |> Seq.map (fun i -> - let seq = dbg.GetSequencePoint i + let seq = getSequencePoint dbg i fakeSequencePoint genuine seq i) |> Seq.tryFind isSequencePoint let internal findSequencePoint - (dbg: MethodDebugInformation) + (dbg: Dictionary) (instructions: Instruction seq) = findEffectiveSequencePoint Genuine dbg instructions @@ -1272,30 +1275,36 @@ module internal Visitor = findEffectiveSequencePoint FakeAtReturn dbg range [] - let rec internal lastOfSequencePoint (dbg: MethodDebugInformation) (i: Instruction) = + let rec internal lastOfSequencePoint + (dbg: IDictionary) + (i: Instruction) + = let n = i.Next if n |> isNull - || n |> dbg.GetSequencePoint |> isSequencePoint + || n |> (getSequencePoint dbg) |> isSequencePoint then i else lastOfSequencePoint dbg n [] - let rec internal firstOfSequencePoint (dbg: MethodDebugInformation) (i: Instruction) = + let rec internal firstOfSequencePoint + (dbg: IDictionary) + (i: Instruction) + = let p = i.Previous if p |> isNull // generated code e.g Fody won't have sequence point values - || (i |> dbg.GetSequencePoint).IsNotNull + || (i |> getSequencePoint dbg).IsNotNull then i else firstOfSequencePoint dbg p - let internal getJumps (dbg: MethodDebugInformation) (i: Instruction) = + let internal getJumps (dbg: Dictionary) (i: Instruction) = let terminal = lastOfSequencePoint dbg i let next = i.Next @@ -1393,7 +1402,13 @@ module internal Visitor = |> Seq.collect id |> Seq.sortBy _.Key // important! instrumentation assumes we work in the order we started with - let private extractBranchPoints dbg rawInstructions interesting vc = + let private extractBranchPoints + (v0t: TypeReference option) + (dbg: Dictionary) + rawInstructions + interesting + vc + = let makeDefault i = if CoverageParameters.coalesceBranches.Value then -1 @@ -1409,15 +1424,13 @@ module internal Visitor = // possibly add MoveNext filtering let generated (i: Instruction) = let before = firstOfSequencePoint dbg i // This line to suppress - let sp = dbg.GetSequencePoint before + let sp = getSequencePoint dbg before before.OpCode = OpCodes.Ldloc_0 && sp.IsNotNull && sp.IsHidden - && (let v0t = - dbg.Method.Body.Variables.[0].VariableType - - v0t.MetadataType = MetadataType.Int32) // state machines do this + && (v0t.IsSome + && v0t.Value.MetadataType = MetadataType.Int32) // state machines do this [ rawInstructions |> Seq.cast ] |> Seq.filter (fun _ -> dbg.IsNotNull) @@ -1471,8 +1484,11 @@ module internal Visitor = |> Seq.map BranchPoint |> Seq.toList - let internal validateInstruction (dbg: MethodDebugInformation) (x: Instruction) = - let s = dbg.GetSequencePoint x + let internal validateInstruction + (dbg: IDictionary) + (x: Instruction) + = + let (_, s) = dbg.TryGetValue x.Offset s.IsNotNull && (s.IsHidden |> not) let internal trivial = @@ -1485,14 +1501,21 @@ module internal Visitor = OpCodes.Nop ] ) - let internal isNonTrivialSeqPnt (dbg: MethodDebugInformation) (x: Instruction) = + let internal isNonTrivialSeqPnt + (dbg: Dictionary) + (x: Instruction) + = if CoverageParameters.trivia.Value then - let rest = + let rest = // rest of the sequence point Seq.unfold (fun (i: Instruction) -> if i |> isNull - || i |> dbg.GetSequencePoint |> isNull |> not + || i.Offset + |> dbg.TryGetValue + |> snd + |> isNull + |> not then None else @@ -1509,21 +1532,28 @@ module internal Visitor = true let private visitMethod (m: MethodEntry) = - let rawInstructions = - m.Method.Body.Instructions + let body = m.Method.Body - let dbg = m.Method.DebugInformation + let rawInstructions = body.Instructions + + let splut = Dictionary() + + do + let dbg = m.Method.DebugInformation + + if dbg.IsNotNull && dbg.HasSequencePoints then + dbg.SequencePoints + |> Seq.iter (fun s -> splut.Add(s.Offset, s)) + + // build more look-up tables let instructions = [ rawInstructions |> Seq.cast ] - |> Seq.filter (fun _ -> dbg.IsNotNull) + |> Seq.filter (fun _ -> splut.Any()) |> Seq.concat |> Seq.filter (fun (x: Instruction) -> - if dbg.HasSequencePoints then - validateInstruction dbg x - && isNonTrivialSeqPnt dbg x - else - false) + validateInstruction splut x + && isNonTrivialSeqPnt splut x) |> Seq.toList let number = instructions.Length @@ -1551,7 +1581,8 @@ module internal Visitor = MethodPoint { Instruction = i SeqPnt = - dbg.GetSequencePoint(i) + splut.TryGetValue(i.Offset) + |> snd |> Option.ofObj |> Option.filter (fun _ -> CoverageParameters.methodPoint.Value) |> Option.map SeqPnt.Build @@ -1561,7 +1592,7 @@ module internal Visitor = else instructions.OrderByDescending(fun (x: Instruction) -> x.Offset) |> Seq.mapi (fun i x -> - let s = dbg.GetSequencePoint(x) + let s = splut.TryGetValue(x.Offset) |> snd MethodPoint { Instruction = x @@ -1580,10 +1611,18 @@ module internal Visitor = let bp = if includeBranches () then let spnt = - instructions |> Seq.head |> dbg.GetSequencePoint + (instructions |> Seq.head).Offset + |> splut.TryGetValue + |> snd let branches = wanted interesting spnt - extractBranchPoints dbg rawInstructions branches m.DefaultVisitCount + + let v0t = + body.Variables + |> Seq.tryHead + |> Option.map _.VariableType + + extractBranchPoints v0t splut rawInstructions branches m.DefaultVisitCount else [] @@ -1688,24 +1727,24 @@ module internal Visitor = "InstantiateArgumentExceptionCorrectlyRule", Scope = "member", // MethodDefinition Target = - "AltCover.Visitor/I/start@1257::Invoke(Microsoft.FSharp.Core.FSharpFunc`2,Microsoft.FSharp.Collections.FSharpList`1)", + "AltCover.Visitor/I/start@1260::Invoke(Microsoft.FSharp.Core.FSharpFunc`2,Microsoft.FSharp.Collections.FSharpList`1)", Justification = "Inlined library code")>] [,Microsoft.FSharp.Collections.FSharpList`1)", + "AltCover.Visitor/I/finish@1263::Invoke(Microsoft.FSharp.Core.FSharpFunc`2,Microsoft.FSharp.Collections.FSharpList`1)", Justification = "Inlined library code")>] [] [] () \ No newline at end of file diff --git a/AltCover.Recorder.Tests/Recorder.Tests.fs b/AltCover.Recorder.Tests/Recorder.Tests.fs index 78d49a1e..767f685f 100644 --- a/AltCover.Recorder.Tests/Recorder.Tests.fs +++ b/AltCover.Recorder.Tests/Recorder.Tests.fs @@ -85,9 +85,7 @@ module AltCoverTests = [] let ShouldFailXmlDataForNativeJson () = Assert.Throws(fun () -> - ReportFormat.NativeJson - |> Counter.I.XmlByFormat - |> ignore) + ignore (Counter.I.XmlByFormat ReportFormat.NativeJson)) |> ignore [] @@ -1936,6 +1934,7 @@ module AltCoverTests = let ZipFlushLeavesExpectedTracesWhenDiverted () = let saved = Console.Out let here = Directory.GetCurrentDirectory() + let mutable complete = false let where = Assembly.GetExecutingAssembly().Location @@ -2021,17 +2020,21 @@ module AltCoverTests = "2" "1" ] ) + + complete <- true finally AltCoverCoreTests.maybeDeleteFile reportFile Console.SetOut saved Directory.SetCurrentDirectory(here) AltCoverCoreTests.maybeIOException (fun () -> Directory.Delete(unique)) + Assert.That(complete, Is.True, "incomplete") #endif [] let ZipFlushLeavesExpectedTracesWhenBroken () = let saved = Console.Out let here = Directory.GetCurrentDirectory() + let mutable complete = false let where = Assembly.GetExecutingAssembly().Location @@ -2096,6 +2099,7 @@ module AltCoverTests = let after = XmlDocument() after.Load worker' Assert.That(after.OuterXml, Is.EqualTo "") + complete <- true finally AltCoverCoreTests.maybeDeleteFile reportFile AltCoverCoreTests.maybeDeleteFile outputFile @@ -2103,11 +2107,13 @@ module AltCoverTests = Console.SetOut saved Directory.SetCurrentDirectory(here) AltCoverCoreTests.maybeIOException (fun () -> Directory.Delete(unique)) + Assert.That(complete, Is.True, "incomplete") [] let ZipFlushLeavesExpectedTracesWhenBrokenInPlace () = let saved = Console.Out let here = Directory.GetCurrentDirectory() + let mutable complete = false let where = Assembly.GetExecutingAssembly().Location @@ -2161,16 +2167,19 @@ module AltCoverTests = Assert.That(reportFile |> File.Exists |> not) let zipInfo = FileInfo(zipFile) Assert.That(zipInfo.Length, Is.EqualTo 0) + complete <- true finally AltCoverCoreTests.maybeDeleteFile zipFile Console.SetOut saved Directory.SetCurrentDirectory(here) AltCoverCoreTests.maybeIOException (fun () -> Directory.Delete(unique)) + Assert.That(complete, Is.True, "incomplete") #if !NET20 [] let ZipFlushLeavesExpectedTraces () = getMyMethodName "=>" + let mutable complete = false lock Instance.I.visits (fun () -> Instance.I.isRunner <- false @@ -2280,6 +2289,8 @@ module AltCoverTests = "2" "1" ] ) + + complete <- true finally Instance.I.Trace <- save AltCoverCoreTests.maybeDeleteFile Instance.ReportFilePath @@ -2287,7 +2298,8 @@ module AltCoverTests = AltCoverCoreTests.VisitsClear() Console.SetOut saved Directory.SetCurrentDirectory(here) - AltCoverCoreTests.maybeIOException (fun () -> Directory.Delete(unique)))) + AltCoverCoreTests.maybeIOException (fun () -> Directory.Delete(unique)) + Assert.That(complete, Is.True, "incomplete"))) getMyMethodName "<=" #endif diff --git a/AltCover.Tests/Runner.Tests.fs b/AltCover.Tests/Runner.Tests.fs index 70541acd..6f468df1 100644 --- a/AltCover.Tests/Runner.Tests.fs +++ b/AltCover.Tests/Runner.Tests.fs @@ -6258,6 +6258,8 @@ module AltCoverRunnerTests = let sep = String([| Path.DirectorySeparatorChar |]) + let s = [ "some dummy value" ] |> List.toSeq + let cases = [ ([ "a" ], "a") ([ "a/b/"; "a/b/c" ], "a/b" + sep) @@ -6265,16 +6267,13 @@ module AltCoverRunnerTests = ([ "c:\\b\\x\\y"; "c:\\b\\c\\d" ], "c:\\b" + sep) ] |> List.map (fun (inputs, expect) -> (inputs - |> Seq.map (fun x -> (x, Cobertura.I.splitPath x))), + |> List.map (fun x -> ((x, s), Cobertura.I.splitPath x))), expect) Assert.Multiple(fun () -> cases |> Seq.iter (fun (case, expect) -> - test - <@ Cobertura.I.extractSource case = expect - - @>)) + test <@ case |> Cobertura.I.extractSource |> fst = expect @>)) [] let DegenerateCasesShouldNotGenerateCobertura () = diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 57404968..80fd0f12 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -3,10 +3,14 @@ A. Start with the Quick Start guide : https://github.com/SteveGilham/altcover/wiki/QuickStart-Guide and read the FAQ : https://github.com/SteveGilham/altcover/wiki/FAQ -# (Habu series release 31) +# (Habu series release 32) +* [PERFORMANCE] Issue #227 - removing the slow-down observed the new (at 8.8.165) file name processing for Cobertura +* [PERFORMANCE] removing a surprising hot-spot in branch coverage instrumentation that was taking 60% of the whole instrumentation time + +# 8.8.165 (Habu series release 31) * [ADVISORY] the Fake.build related assemblies (in the `altcover.api` and `altcover.fake` packages), and the Avalonia 0.10-based visualizer, rely on components with known vulnerabilities. The Fake.build project appears nigh-moribund so has not released an update, whereas Avalonia 11 completely rewrites all the earlier APIs and has not documented anything to assist in the rewrite of the application. * [BUGFIX] Issue #197 - correctly split file paths in the Cobertura output -* [NET9 preparation] Recode the recorder into C# as compiler/build target changes in F#9 make maintaing net2.0 compatibility in F# too much bother. +* [NET9 preparation] Recode the recorder into C# as compiler/build target changes in F#9 make maintaining net2.0 compatibility in F# too much bother. # 8.8.74 (Habu series release 30) * [BUGFIX] Issue #222 - distinguish methods differing only in number of generic parameters (JSON and cobertura in particular, but with small changes for all all output formats)