Skip to content

Commit c25ced6

Browse files
timleavitttimleavitt
committed
#3 Performance fixes
Fixes #3, plus some other performance/robustness fixes (brings in line with our internal development on this tool)
1 parent b8dcc08 commit c25ced6

File tree

13 files changed

+344
-141
lines changed

13 files changed

+344
-141
lines changed

‎cls/TestCoverage/Data/CodeUnit.cls

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Property MethodMap As array Of %Integer;
3232

3333
/// For classes, map of line numbers in code to associated method names
3434
/// For routines, map of labels to associated line numbers
35-
Property LineToMethodMap As array Of %String [ Private ];
35+
Property LineToMethodMap As array Of %Dictionary.CacheIdentifier [ Private ];
3636

3737
/// Set to true if this class/routine is generated
3838
Property Generated As %Boolean [ InitialExpression = 0 ];
@@ -160,8 +160,13 @@ ClassMethod GetCurrentByName(pInternalName As %String, pSourceNamespace As %Stri
160160
}
161161
}
162162

163-
$$$ThrowOnError(pCodeUnit.%Save())
164-
163+
Set tSC = pCodeUnit.%Save()
164+
If $$$ISERR(tSC) && $System.Status.Equals(tSC,$$$ERRORCODE($$$IDKeyNotUnique)) {
165+
// Some other process beat us to it.
166+
Set tSC = $$$OK
167+
Set pCodeUnit = ..%OpenId(pCodeUnit.Hash,,.tSC)
168+
Quit
169+
}
165170
// For non-class (e.g., .MAC/.INT) code, it's possible that something else generated it,
166171
// so update the mappings between generated and the thing that generated it.
167172
If (tType '= "CLS") {

‎cls/TestCoverage/Data/CodeUnitMap.cls

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/// This class maintains the mapping between .INT/.MAC/.CLS and therefore is critical for
2+
/// interpreting the .INT-level coverage data that the line-by-line monitor collects.
13
Class TestCoverage.Data.CodeUnitMap Extends %Persistent
24
{
35

@@ -23,33 +25,99 @@ ForeignKey ToCodeUnitFK(ToHash) References TestCoverage.Data.CodeUnit(Hash) [ On
2325

2426
ClassMethod Create(pFromHash As %String, pFromLine As %Integer, pToHash As %String, pToLineStart As %Integer, pToLineEnd As %Integer) As %Status
2527
{
28+
#def1arg DefaultStorageNode(%node) ##expression($$$comMemberKeyGet("TestCoverage.Data.CodeUnitMap", $$$cCLASSstorage, "Default", %node))
29+
#def1arg CodeUnitMasterMap(%arg) $$$DefaultStorageNode($$$cSDEFdatalocation)(%arg)
30+
#def1arg CodeUnitReverseMap(%arg) $$$DefaultStorageNode($$$cSDEFindexlocation)("Reverse",%arg)
31+
2632
Set tSC = $$$OK
2733
Try {
28-
&sql(insert or update %NOLOCK %NOCHECK into TestCoverage_Data.CodeUnitMap
29-
(FromHash, FromLine, ToHash, ToLine)
30-
select :pFromHash, :pFromLine, :pToHash, Counter
31-
from TestCoverage.Sequence(:pToLineStart,:pToLineEnd))
32-
If (SQLCODE < 0) {
33-
Throw ##class(%Exception.SQL).CreateFromSQLCODE(SQLCODE,%msg)
34+
For counter=pToLineStart:1:pToLineEnd {
35+
// Uses direct global references for performance boost; this is one of the most performance-critical sections.
36+
If '$Data($$$CodeUnitMasterMap(pFromHash,pFromLine,pToHash,counter)) {
37+
&sql(insert %NOLOCK %NOCHECK into TestCoverage_Data.CodeUnitMap
38+
(FromHash, FromLine, ToHash, ToLine)
39+
select :pFromHash, :pFromLine, :pToHash, :counter)
40+
If (SQLCODE < 0) {
41+
Throw ##class(%Exception.SQL).CreateFromSQLCODE(SQLCODE,%msg)
42+
}
43+
}
3444
}
3545

3646
// Insert/update transitive data (e.g., .INT -> .MAC (generator) -> .CLS)
47+
// Original implementation:
48+
/*
49+
// Leg 1: Lines that map to the "from" line also map to the "to" line
50+
// Leg 2: The "from" line also maps to lines that the "to" line maps to
3751
&sql(
38-
/* Lines that map to the "from" line also map to the "to" line */
3952
insert or update %NOLOCK %NOCHECK into TestCoverage_Data.CodeUnitMap
4053
(FromHash, FromLine, ToHash, ToLine)
4154
select FromHash, FromLine, :pToHash, Counter
4255
from TestCoverage.Sequence(:pToLineStart,:pToLineEnd),TestCoverage_Data.CodeUnitMap
4356
where ToHash = :pFromHash and ToLine = :pFromLine
4457
union
45-
/* The "from" line also maps to lines that the "to" line maps to */
4658
select :pFromHash, :pFromLine, ToHash, ToLine
4759
from TestCoverage.Sequence(:pToLineStart,:pToLineEnd)
4860
join TestCoverage_Data.CodeUnitMap
4961
on FromHash = :pToHash and FromLine = Counter)
5062
If (SQLCODE < 0) {
5163
Throw ##class(%Exception.SQL).CreateFromSQLCODE(SQLCODE,%msg)
5264
}
65+
*/
66+
67+
// This introduced some unacceptable performance overhead, and has been rewritten with direct global references.
68+
// This reduces overall overhead of code capture for test coverage measurement by roughly 40%.
69+
70+
// Leg 1: Lines that map to the "from" line also map to the "to" line
71+
Set fromHash = ""
72+
For {
73+
Set fromHash = $Order($$$CodeUnitReverseMap(pFromHash,pFromLine,fromHash))
74+
If (fromHash = "") {
75+
Quit
76+
}
77+
Set fromLine = ""
78+
For {
79+
Set fromLine = $Order($$$CodeUnitReverseMap(pFromHash,pFromLine,fromHash,fromLine))
80+
If (fromLine = "") {
81+
Quit
82+
}
83+
For counter=pToLineStart:1:pToLineEnd {
84+
If '$Data($$$CodeUnitMasterMap(fromHash,fromLine,pToHash,counter)) {
85+
&sql(insert %NOLOCK %NOCHECK into TestCoverage_Data.CodeUnitMap
86+
(FromHash, FromLine, ToHash, ToLine)
87+
select :fromHash, :fromLine, :pToHash, :counter)
88+
If (SQLCODE < 0) {
89+
Throw ##class(%Exception.SQL).CreateFromSQLCODE(SQLCODE,%msg)
90+
}
91+
}
92+
}
93+
}
94+
}
95+
96+
For counter=pToLineStart:1:pToLineEnd {
97+
// Leg 2: The "from" line also maps to lines that the "to" line maps to
98+
Set toHash = ""
99+
For {
100+
Set toHash = $Order($$$CodeUnitMasterMap(pToHash,counter,toHash))
101+
If (toHash = "") {
102+
Quit
103+
}
104+
Set toLine = ""
105+
For {
106+
Set toLine = $Order($$$CodeUnitMasterMap(pToHash,counter,toHash,toLine))
107+
If (toLine = "") {
108+
Quit
109+
}
110+
If '$Data($$$CodeUnitMasterMap(pFromHash,pFromLine,toHash,toLine)) {
111+
&sql(insert %NOLOCK %NOCHECK into TestCoverage_Data.CodeUnitMap
112+
(FromHash, FromLine, ToHash, ToLine)
113+
select :pFromHash, :pFromLine, :toHash, :toLine)
114+
If (SQLCODE < 0) {
115+
Throw ##class(%Exception.SQL).CreateFromSQLCODE(SQLCODE,%msg)
116+
}
117+
}
118+
}
119+
}
120+
}
53121
} Catch e {
54122
Set tSC = e.AsStatus()
55123
}

‎cls/TestCoverage/Data/Run.cls

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ ClassMethod MapRunCoverage(pRunIndex As %Integer) As %Status
8585
Set tMetric = tRun.Metrics.GetAt(i)
8686
Set tSQLStatement = "INSERT OR UPDATE %NOLOCK %NOCHECK INTO TestCoverage_Data.""Coverage_"_tMetric_""" "_
8787
"(Coverage,element_key,"""_tMetric_""") "_
88-
"SELECT target.ID,map.ToLine,NVL(oldMetric."""_tMetric_""",0) + metric."""_tMetric_""" "_
88+
"SELECT target.ID,map.ToLine,NVL(oldMetric."""_tMetric_""",0) + SUM(metric."""_tMetric_""") "_
8989
"FROM TestCoverage_Data.Coverage source "_
9090
"JOIN TestCoverage_Data.CodeUnitMap map "_
9191
" ON source.Hash = map.FromHash "_
@@ -101,7 +101,8 @@ ClassMethod MapRunCoverage(pRunIndex As %Integer) As %Status
101101
" AND oldMetric.element_key = map.ToLine "_
102102
"WHERE source.Run = ? "_
103103
" AND source.Ignore = 0"_
104-
" AND source.Calculated = 0"
104+
" AND source.Calculated = 0"_
105+
"GROUP BY target.ID,map.ToLine"
105106

106107
#dim tResult As %SQL.StatementResult
107108
Set tResult = ##class(%SQL.Statement).%ExecDirect(,tSQLStatement,pRunIndex)

‎cls/TestCoverage/Manager.cls

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ Property ProcessIDs As %List [ Internal, Private ];
5252

5353
Property Run As TestCoverage.Data.Run;
5454

55-
/// Known coverage targets (already snapshotted)
55+
/// Known coverage targets (already snapshotted). <br />
56+
/// Value at subscript is set to 1 if there are executable lines of code in the target, 0 if not.
5657
Property KnownCoverageTargets [ MultiDimensional, Private ];
5758

5859
/// Cache of (name, type) -> hash
@@ -172,23 +173,33 @@ Method StartCoverageTracking() As %Status [ Private ]
172173
Try {
173174
If (..CoverageTargets '= "") {
174175
Set $Namespace = ..SourceNamespace
175-
176+
177+
Set tRelevantTargets = ""
176178
Set tNewTargets = ""
177179
Set tPointer = 0
178180
While $ListNext(..CoverageTargets,tPointer,tCoverageTarget) {
179-
If '$Data(..KnownCoverageTargets(tCoverageTarget)) {
181+
If '$Data(..KnownCoverageTargets(tCoverageTarget),tIsRelevant)#2 {
180182
Set tNewTargets = tNewTargets_$ListBuild(tCoverageTarget)
183+
} ElseIf tIsRelevant {
184+
Set tRelevantTargets = tRelevantTargets_$ListBuild(tCoverageTarget)
181185
}
182186
}
183187

184188
If (tNewTargets '= "") {
185189
$$$StartTimer("Taking snapshot of code and CLS/MAC/INT mappings")
186-
Do ##class(TestCoverage.Utils).Snapshot(tNewTargets)
190+
Set tSC = ##class(TestCoverage.Utils).Snapshot(tNewTargets, .tNewRelevantTargets)
187191
$$$StopTimer
192+
$$$ThrowOnError(tSC)
188193

189194
Set tPointer = 0
190195
While $ListNext(tNewTargets,tPointer,tNewTarget) {
191-
Set ..KnownCoverageTargets(tNewTarget) = ""
196+
Set ..KnownCoverageTargets(tNewTarget) = 0
197+
}
198+
199+
Set tPointer = 0
200+
While $ListNext(tNewRelevantTargets,tPointer,tRelevantTarget) {
201+
Set ..KnownCoverageTargets(tRelevantTarget) = 1
202+
Set tRelevantTargets = tRelevantTargets_$ListBuild(tRelevantTarget)
192203
}
193204
}
194205

@@ -218,7 +229,7 @@ Method StartCoverageTracking() As %Status [ Private ]
218229
}
219230
}
220231
Set tMetrics = $ListBuild("RtnLine") _ $Select(..Timing:$ListBuild("Time","TotalTime"),1:"")
221-
$$$ThrowOnError(..Monitor.StartWithScope(..CoverageTargets,tMetrics,tProcessIDs))
232+
$$$ThrowOnError(..Monitor.StartWithScope(tRelevantTargets,tMetrics,tProcessIDs))
222233
}
223234
} Catch e {
224235
Set tSC = e.AsStatus()
@@ -282,8 +293,6 @@ Method UpdateCoverageTargetsForTestDirectory(pDirectory As %String) As %Status [
282293
}
283294

284295
Set tObjectCodeList = ..GetObjectCodeForSourceNames(tCoverageTargetList)
285-
// Check the available memory before trying to capture coverage, so the user can remediate without waiting a really long time
286-
$$$ThrowOnError(##class(TestCoverage.Utils.LineByLineMonitor).CheckAvailableMemory($ListLength(..ProcessIDs),$ListLength(tObjectCodeList)))
287296
Set ..CoverageTargets = tObjectCodeList // Also restarts the monitor if it is running and updates data on covered routines/classes
288297
} Catch e {
289298
Set tSC = e.AsStatus()
@@ -697,19 +706,35 @@ Method PrintURL()
697706
{
698707
Do ##super()
699708

700-
Do ..PrintLine("Use the following URL to view test coverage data:")
701-
Do ..PrintLine(..GetURL(..Run.%Id()))
709+
Set tURL = ..GetURL(..Run.%Id())
710+
If (tURL '= "") {
711+
Do ..PrintLine("Use the following URL to view test coverage data:")
712+
Do ..PrintLine(tURL)
713+
} Else {
714+
Do ..PrintLine("WARNING: No default web application found for namespace '"_$Namespace_"' - test coverage results cannot be viewed.")
715+
}
702716
Quit
703717
}
704718

719+
/// Returns the URL to the aggregate result viewer. <br />
720+
/// <var>pRunID</var> is the test coverage run index.
721+
/// <var>pHost</var> contains the host/protocol to use.
722+
/// <var>pPath</var> contains the rest of the URL after that.
705723
ClassMethod GetURL(pRunID As %String, Output pHost As %String, Output pPath As %String) As %String
706724
{
707-
Set tSC = ##class(%RoutineMgr).GetWebServerPort(.tPort,.tServer,.tURLPrefix)
725+
Set tSC = ##class(%Library.RoutineMgr).GetWebServerPort(.tPort,.tServer,.tURLPrefix)
708726
$$$ThrowOnError(tSC)
709-
Set pHost = $Case(tPort,443:"https",:"http")_"://"_$Get(^%SYS("HealthShare","NetworkHostName"),tServer)
710-
Set pHost = pHost _ $Case(tPort,80:"",:":"_tPort)
727+
Set pHost = $ZConvert($Get(^%SYS("WebServer","Protocol"),$Select(tPort=443:"https",1:"http")),"l")
728+
Set pHost = pHost_"://"_$Get(^%SYS("HealthShare","NetworkHostName"),tServer)
729+
// Ports 80 and 443 are defaults for their respective protocols; in other cases, port needs to be explicit.
730+
Set pHost = pHost _ $Case(tPort,80:"",443:"",:":"_tPort)
731+
Set tDefaultApp = $System.CSP.GetDefaultApp($Namespace)
732+
If (tDefaultApp = "") || (((tDefaultApp = "/csp/sys") || (tDefaultApp [ "/csp/sys/")) && ($Namespace '= "%SYS")) {
733+
// The URL won't be valid, so just return an empty string.
734+
Quit ""
735+
}
711736
Set pPath = $Case(tURLPrefix,"":"",:"/"_tURLPrefix)
712-
Set pPath = pPath _ $$getDefaultApp^%SYS.cspServer2($Namespace)
737+
Set pPath = pPath _ tDefaultApp
713738
Set pPath = pPath _ "/TestCoverage.UI.AggregateResultViewer.cls?Index="_$ZConvert(pRunID,"O","URL")
714739
Quit pHost_pPath
715740
}

‎cls/TestCoverage/Procedures.cls

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/// Contains several helpful stored procedures for use in SQL.
12
Class TestCoverage.Procedures
23
{
34

@@ -86,32 +87,5 @@ ClassMethod ListToBit(pSource As %List) As %Binary [ SqlName = LIST_TO_BIT, SqlP
8687
Quit tResult
8788
}
8889

89-
/// Table-valued function returning a sequence of integers (column name "Counter") going from <var>pStart</var> to <var>pEnd</var> by <var>pIncrement</var>.
90-
Query Sequence(pStart As %Integer, pEnd As %Integer, pIncrement As %Integer = 1) As %Query(ROWSPEC = "Counter:%Integer") [ SqlName = SEQUENCE, SqlProc ]
91-
{
92-
}
93-
94-
ClassMethod SequenceExecute(ByRef qHandle As %Binary, pStart As %Integer, pEnd As %Integer, pIncrement As %Integer = 1) As %Status
95-
{
96-
Set qHandle = pStart
97-
Set qHandle("inc") = pIncrement
98-
Set qHandle("end") = pEnd
99-
Quit $$$OK
100-
}
101-
102-
ClassMethod SequenceClose(ByRef qHandle As %Binary) As %Status [ PlaceAfter = SequenceExecute ]
103-
{
104-
Quit $$$OK
105-
}
106-
107-
ClassMethod SequenceFetch(ByRef qHandle As %Binary, ByRef Row As %List, ByRef AtEnd As %Integer = 0) As %Status [ PlaceAfter = SequenceExecute ]
108-
{
109-
Set Row = $ListBuild(qHandle)
110-
If ($Increment(qHandle,qHandle("inc")) > qHandle("end")) {
111-
Set AtEnd = 1
112-
}
113-
Quit $$$OK
114-
}
115-
11690
}
11791

‎cls/TestCoverage/UI/Application.cls

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,14 @@ pre.coverage {
7070
max-height: 75vh;
7171
overflow-x: auto;
7272
}
73-
pre.coverage span {
73+
pre.coverage > span {
7474
white-space: pre;
7575
tab-size: 4;
7676
display: block;
7777
line-height: 1.5em;
7878
width: 100%;
7979
}
80-
pre.coverage span:before {
80+
pre.coverage > span:before {
8181
counter-increment: line;
8282
content: counter(line);
8383
display: inline-block;
@@ -89,15 +89,15 @@ pre.coverage span:before {
8989
}
9090

9191
/* Classes for display of code coverage */
92-
pre.coverage span.executable:before {
92+
pre.coverage > span.executable:before {
9393
background-color: #f66;
9494
}
9595

96-
pre.coverage span.covered:before {
96+
pre.coverage > span.covered:before {
9797
background-color: #6f6;
9898
}
9999

100-
pre.coverage span.hide:before {
100+
pre.coverage > span.hide:before {
101101
background-color: #fff;
102102
}
103103

0 commit comments

Comments
 (0)