Skip to content

Commit 4cace76

Browse files
isc-kiyerkiyer
isc-kiyer
and
kiyer
authored
BuildDependencyGraph performance enhancements (#975)
* Update CLI commands to deprecate synchronous flag * Remove various LoadDependencies() calls as this should just be handled at install time. Convert default to sync loading. Remove InstallContext as its no longer needed. * Add unit test for paser and only include deprecated if relevant * Remove missed InstallContext refs * Update changelog, remove $$$IsISCWorker references that are no longer valid. Remove command line module name from dependency loads (only needed for initial check when running load/update) * Update module version * remove debug breaks * Small edit to contributing. Add range intersection resolving * Leverage range intersection resolving and re-implement BuildDependencyGraph as iterative + improved caching * Fix null check so comparisons work for "*" * Clean up to remove t variables and reorder method args. Remove optimized method to just compare original against iterative. Have graph comparison in DepGraph(). * Make iterative the only version of BuilDependencyGraph * Changelog update * Add test for matching wildcard. Fix And logic for wildcards * Remove debug code + small fixes * Address review feedback * Fix spacing * Add comment. Remove unused code * Fix typo of % name in classes --------- Co-authored-by: kiyer <[email protected]>
1 parent 8a88d5b commit 4cace76

File tree

10 files changed

+1040
-699
lines changed

10 files changed

+1040
-699
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2020
- #885: Always synchronously load dependencies and let each module do multi-threading as needed
2121
to load using multicompile instead of trying to do own multi-threading of item load which causes
2222
lock contention by bypassing IRIS compiler.
23+
- #481: Improve BuildDependencyGraph performance by doing the following:
24+
- Eliminate recursion and use iteration.
25+
- Remove depth first search and do pure breadth first search.
26+
- Have better caching of results for module searches by collapsing search expressions (reducing expressions that are intersections).
2327

2428
### Removed
2529
- #938 Removed secret flag NewVersion handling in %Publish()

‎CONTRIBUTING.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ Make any necessary changes, compile them (which should be handled by VS Code on
3535
docker exec -it ipm-iris-1 /bin/bash
3636
$ iris session iris
3737
```
38+
OR as a single line:
39+
```bash
40+
docker exec -it ipm-iris-1 /bin/bash -c "iris session iris"
41+
```
3842
If you need to shut down all the containers (either to switch to another branch or to revert back to a clean state) involved in the `docker-compose.yml`, run the following command in the project folder:
3943
```bash
4044
docker compose down --remove-orphans --volumes

‎src/cls/IPM/General/SemanticVersion.cls‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,13 @@
1-
Class %IPM.General.SemanticVersion Extends (%SerialObject, %XML.Adaptor) [ StorageStrategy = "" ]
1+
Class %IPM.General.SemanticVersion Extends (%SerialObject, %IPM.General.SemanticVersion.Properties) [ StorageStrategy = "" ]
22
{
33

44
Parameter DEFAULTGLOBAL = "^IPM.General.SemanticVersion";
55

66
Parameter NAMESPACE As STRING = "http://www.intersystems.com/PackageManager";
77

8-
Property Major As %Integer(MINVAL = 0) [ Required ];
9-
10-
Property Minor As %Integer(MINVAL = 0) [ Required ];
11-
12-
Property Patch As %Integer(MINVAL = 0) [ Required ];
13-
14-
Property Prerelease As %IPM.DataType.RegExString(MAXLEN = 100, REGEX = "([0-9A-Za-z-])+(\.([0-9A-Za-z-])+)*");
15-
168
/// This is an alias for Prerelease. It is used for code readability when SemVerPostRelease is enabled.
179
Property Postrelease As %IPM.DataType.RegExString(MAXLEN = 100, REGEX = "([0-9A-Za-z-])+(\.([0-9A-Za-z-])+)*") [ Calculated, SqlComputeCode = { set {*} = {Prerelease} }, SqlComputed, Transient ];
1810

19-
Property Build As %IPM.DataType.RegExString(MAXLEN = 100, REGEX = "([0-9A-Za-z-])+(\.([0-9A-Za-z-])+)*");
20-
2111
Method PostreleaseGet() As %IPM.DataType.RegExString
2212
{
2313
quit ..Prerelease

‎src/cls/IPM/General/SemanticVersion/Properties.cls‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/// Properties class for Semantic Versioning
2+
Class %IPM.General.SemanticVersion.Properties Extends (%RegisteredObject, %XML.Adaptor)
3+
{
4+
5+
Property Major As %Integer(MINVAL = 0) [ Required ];
6+
7+
Property Minor As %Integer(MINVAL = 0) [ Required ];
8+
9+
Property Patch As %Integer(MINVAL = 0) [ Required ];
10+
11+
Property Prerelease As %IPM.DataType.RegExString(MAXLEN = 100, REGEX = "([0-9A-Za-z-])+(\.([0-9A-Za-z-])+)*");
12+
13+
Property Build As %IPM.DataType.RegExString(MAXLEN = 100, REGEX = "([0-9A-Za-z-])+(\.([0-9A-Za-z-])+)*");
14+
15+
}

‎src/cls/IPM/General/SemanticVersionExpression.cls‎

Lines changed: 121 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -3,66 +3,68 @@ Class %IPM.General.SemanticVersionExpression Extends (%SerialObject, %XML.Adapto
33

44
Parameter DEFAULTGLOBAL = "^IPM.General.SemVerExpression";
55

6-
/// List of semantic version expressions to provide support for ||
7-
Property AnyOf As list Of %IPM.General.SemanticVersionExpression.IComparable(CLASSNAME = 1);
8-
9-
/// Used to combine multiple semantic version expressions with "&&" when doing dependency resolution
10-
Property AllOf As list Of %IPM.General.SemanticVersionExpression.IComparable(CLASSNAME = 1);
6+
/// List of ranges to provide support for ||. Any && would be consolidated into single ranges in the AnyOf list.
7+
Property AnyOf As list Of %IPM.General.SemanticVersionExpression.Range;
118

129
/// String representation of this SemVer expression composed of the AnyOf and AllOf lists
1310
/// Parentheses are not used in parsing but are merely stylistic
1411
Property Expression As %String(MAXLEN = "");
1512

1613
Method %OnNew(rangeExpr As %String = "") As %Status [ Private, ServerOnly = 1 ]
1714
{
18-
set i%Expression = rangeExpr
19-
quit $$$OK
15+
set ..Expression = rangeExpr
16+
quit $$$OK
2017
}
2118

2219
Method ExpressionSet(value As %String) As %Status
2320
{
24-
// An instance of this class can only either have items in AnyOf or AllOf, not both
25-
if ..AllOf.Count() {
26-
quit $$$ERROR($$$GeneralError,$$$FormatText("This instance of SemanticVersionExpression already has an AND operator and cannot be set"))
27-
}
28-
set status = $$$OK
29-
try {
30-
if value["&&" {
31-
set reqs = $listfromstring(value,"&&")
32-
set ptr = 0
33-
while $listnext(reqs,ptr,rangeExpr) {
34-
set rangeExpr = $zstrip(rangeExpr,"<>W")
35-
set rangeExpr = $zstrip(rangeExpr, "*", "()")
36-
set status = ##class(%IPM.General.SemanticVersionExpression.Range).FromString(rangeExpr,.option)
37-
if $$$ISERR(status) {
38-
quit
21+
if (value [ "&&") && (value [ "||") {
22+
$$$ThrowStatus($$$ERROR($$$GeneralError,"Cannot mix '&&' and '||' in a single expression"))
23+
}
24+
if (value [ "&&") {
25+
set reqs = $listfromstring(value,"&&")
26+
set ptr = 0
27+
set andExpr = ""
28+
while $listnext(reqs,ptr,rangeExpr) {
29+
set rangeExpr = $zstrip(rangeExpr,"<>W")
30+
set rangeExpr = $zstrip(rangeExpr,"*","()")
31+
set sc = ##class(%IPM.General.SemanticVersionExpression.Range).FromString(rangeExpr,.option)
32+
$$$ThrowOnError(sc)
33+
if option.ToResolvedString() = "" {
34+
continue
35+
}
36+
if (andExpr = "") {
37+
set andExpr = option
38+
} else {
39+
set merged = ##class(%IPM.General.SemanticVersionExpression.Range).Intersect(andExpr, option)
40+
if (merged = "") {
41+
$$$ThrowStatus($$$ERROR($$$GeneralError,"No overlapping versions in && operation for %1 and %2", andExpr.ToResolvedString(), option.ToResolvedString()))
42+
}
43+
set andExpr = merged
44+
}
3945
}
40-
if option.ToResolvedString() = "" {
41-
continue
46+
if (andExpr.ToResolvedString() '= "") {
47+
do ..AnyOf.Insert(andExpr)
4248
}
43-
do ..AllOf.Insert(option)
44-
}
4549
} else {
46-
set options = $listfromstring(value,"||")
47-
set ptr = 0
48-
while $listnext(options,ptr,rangeExpr) {
49-
set rangeExpr = $zstrip(rangeExpr,"<>W")
50-
set rangeExpr = $zstrip(rangeExpr, "*", "()")
51-
set status = ##class(%IPM.General.SemanticVersionExpression.Range).FromString(rangeExpr,.option)
52-
if $$$ISERR(status) {
53-
quit
54-
}
55-
if option.ToResolvedString() = "" {
56-
continue
50+
set options = $listfromstring(value,"||")
51+
set ptr = 0
52+
while $listnext(options,ptr,rangeExpr) {
53+
set rangeExpr = $zstrip(rangeExpr,"<>W")
54+
set rangeExpr = $zstrip(rangeExpr,"*","()")
55+
set sc = ##class(%IPM.General.SemanticVersionExpression.Range).FromString(rangeExpr,.option)
56+
$$$ThrowOnError(sc)
57+
if option.ToResolvedString() = "" {
58+
continue
59+
}
60+
do ..AnyOf.Insert(option)
5761
}
58-
do ..AnyOf.Insert(option)
59-
}
6062
}
6163
set i%Expression = value
62-
} catch e {
63-
set status = e.AsStatus()
64-
}
65-
quit status
64+
// note: we throw errors and just return $$$Ok at the end because the returned status isn't
65+
// used by the caller. Its still needed though since without it, there will be errors at runtime
66+
// whenever this setter is used.
67+
return $$$OK
6668
}
6769

6870
Method ToString() As %String [ CodeMode = expression ]
@@ -72,110 +74,109 @@ Method ToString() As %String [ CodeMode = expression ]
7274

7375
Method ToResolvedString() As %String
7476
{
75-
set optList = ""
76-
if ..AnyOf.Count() {
77-
for i=1:1:..AnyOf.Count() {
78-
set opt = ..AnyOf.GetAt(i)
79-
set resolvedString = opt.ToResolvedString()
80-
if ..AnyOf.Count() > 1 {
81-
set optList = optList_$listbuild("(" _ resolvedString _ ")")
82-
} else {
83-
set optList = optList_$listbuild(resolvedString)
84-
}
85-
}
86-
quit $listtostring(optList," || ")
87-
}
88-
if ..AllOf.Count() {
89-
for i=1:1:..AllOf.Count() {
90-
set opt = ..AllOf.GetAt(i)
91-
set resolvedString = opt.ToResolvedString()
92-
if ..AllOf.Count() > 1 {
93-
set optList = optList_$listbuild("(" _ resolvedString _ ")")
94-
} else {
95-
set optList = optList_$listbuild(resolvedString)
96-
}
77+
set optList = ""
78+
if ..AnyOf.Count() {
79+
for i=1:1:..AnyOf.Count() {
80+
set opt = ..AnyOf.GetAt(i)
81+
set resolvedString = opt.ToResolvedString()
82+
if ..AnyOf.Count() > 1 {
83+
set optList = optList_$listbuild("(" _ resolvedString _ ")")
84+
} else {
85+
set optList = optList_$listbuild(resolvedString)
86+
}
87+
}
88+
quit $listtostring(optList," || ")
9789
}
98-
quit $listtostring(optList," && ")
99-
}
100-
quit ""
90+
quit ""
10191
}
10292

10393
ClassMethod FromString(
10494
string As %String,
10595
Output expr As %IPM.General.SemanticVersionExpression) As %Status
10696
{
107-
set status = $$$OK
108-
try {
109-
set expr = ..%New(string)
110-
set status = expr.ExpressionSet(string)
111-
} catch e {
112-
set status = e.AsStatus()
113-
}
114-
quit status
97+
set status = $$$OK
98+
try {
99+
set expr = ..%New(string)
100+
} catch e {
101+
set status = e.AsStatus()
102+
}
103+
quit status
115104
}
116105

117106
Method IsSatisfiedBy(version As %IPM.General.SemanticVersion) As %Boolean
118107
{
119-
set satisfied = (..AnyOf.Count() = 0)
120-
for i=1:1:..AnyOf.Count() {
121-
if ..AnyOf.GetAt(i).IsSatisfiedBy(version) {
122-
set satisfied = 1
123-
quit
108+
if (..AnyOf.Count() = 0) {
109+
return 1
124110
}
125-
}
126-
127-
set allSatisfied = 1
128-
for i=1:1:..AllOf.Count() {
129-
if '..AllOf.GetAt(i).IsSatisfiedBy(version) {
130-
set allSatisfied = 0
131-
quit
111+
for i=1:1:..AnyOf.Count() {
112+
if ..AnyOf.GetAt(i).IsSatisfiedBy(version) {
113+
return 1
114+
quit
115+
}
132116
}
133-
}
134-
135-
quit satisfied && allSatisfied
117+
return 0
136118
}
137119

138120
Method And(versionExpression As %IPM.General.SemanticVersionExpression) As %IPM.General.SemanticVersionExpression
139121
{
140-
if versionExpression.Expression '= "" {
141-
set expr1 = ..%ConstructClone()
142-
do ..AnyOf.Clear()
143-
do ..AllOf.Clear()
144-
do ..AllOf.Insert(expr1)
145-
do ..AllOf.Insert(versionExpression)
146-
set i%Expression = ..ToResolvedString()
147-
}
148-
quit $this
122+
if versionExpression.Expression '= "" {
123+
if (..AnyOf.Count() = 0) {
124+
// If current is empty, just copy in the new one
125+
set ..AnyOf = versionExpression.AnyOf
126+
set i%Expression = versionExpression.Expression
127+
return $this
128+
}
129+
if (versionExpression.AnyOf.Count() = 0) {
130+
// If the new one is empty, just return existing
131+
return $this
132+
}
133+
set current = ..%ConstructClone(1)
134+
do ..AnyOf.Clear()
135+
// Need to do an And on all possible combinations of the AnyOf lists
136+
for i = 1:1:current.AnyOf.Count() {
137+
for j = 1:1:versionExpression.AnyOf.Count() {
138+
set expr1 = current.AnyOf.GetAt(i)
139+
set expr2 = versionExpression.AnyOf.GetAt(j)
140+
set merged = ##class(%IPM.General.SemanticVersionExpression.Range).Intersect(expr1,expr2)
141+
if (merged '= "") {
142+
do ..AnyOf.Insert(merged)
143+
}
144+
}
145+
}
146+
set resolvedString = ..ToResolvedString()
147+
if (resolvedString = "") {
148+
set msg = $$$FormatText("No overlapping versions in AND operation for %1 and %2",current.ToResolvedString(),versionExpression.ToResolvedString())
149+
$$$ThrowStatus($$$ERROR($$$GeneralError,msg))
150+
}
151+
set i%Expression = resolvedString
152+
}
153+
return $this
149154
}
150155

151156
Method Or(versionExpression As %IPM.General.SemanticVersionExpression) As %IPM.General.SemanticVersionExpression
152157
{
153-
if versionExpression.Expression '= "" {
154-
set expr1 = ..%ConstructClone()
155-
do ..AnyOf.Clear()
156-
do ..AllOf.Clear()
157-
do ..AnyOf.Insert(expr1)
158-
do ..AnyOf.Insert(versionExpression)
159-
set i%Expression = ..ToResolvedString()
160-
}
158+
if versionExpression.Expression '= "" {
159+
do ..AnyOf.Insert(versionExpression)
160+
set i%Expression = ..ToResolvedString()
161+
}
161162
quit $this
162163
}
163164

164165
Method %OnOpen() As %Status [ Private, ServerOnly = 1 ]
165166
{
166-
// On-the-fly data migration to tack on list-of-serial CLASSNAME=1 with default classname of %IPM.General.SemanticVersionExpression.Range
167-
set key = ""
168-
for {
169-
set key = $order(i%AnyOf(key),1,data)
170-
if key="" {
171-
quit
172-
}
173-
if $listlength(data) = 1 {
174-
set $list(data,2) = "%IPM.General.SemanticVersionExpression.Range"
167+
// On-the-fly data migration to tack on list-of-serial CLASSNAME=1 with default classname of %IPM.General.SemanticVersionExpression.Range
168+
set key = ""
169+
for {
170+
set key = $order(i%AnyOf(key),1,data)
171+
if key="" {
172+
quit
173+
}
174+
if $listlength(data) = 1 {
175+
set $list(data,2) = "%IPM.General.SemanticVersionExpression.Range"
176+
}
177+
set i%AnyOf(key) = data
175178
}
176-
set i%AnyOf(key) = data
177-
}
178-
quit $$$OK
179+
quit $$$OK
179180
}
180181

181182
Storage Default

0 commit comments

Comments
 (0)