Skip to content

Commit b4212a3

Browse files
committed
internal/core/adt: rewrite replaceIDs to scale better
https://github.com/Caascad/unity-tests is an archived CUE project part of the Unity test suite, and I noticed that while it is pretty fast on evalv2: > CUE_EXPERIMENT=evalv3=0 /bin/time cue vet -c=false 11.35user 0.28system 0:03.83elapsed 303%CPU (0avgtext+0avgdata 752148maxresident)k it is much slower on evalv3, and it's due to 33s spent on closedness: > /bin/time cue vet -c=false 37.59user 0.71system 0:34.27elapsed 111%CPU (0avgtext+0avgdata 2690860maxresident)k > CUE_DEBUG=opendef /bin/time cue vet -c=false 3.96user 0.34system 0:01.46elapsed 294%CPU (0avgtext+0avgdata 1437052maxresident)k Rewrite the algorithm to replace recursion with a breadth-first search, and building up a map upfront so that we can do lookups without needing to do linear searches inside loops. This results in the 33s spent in closedness being cut by two thirds to about 11s: > /bin/time cue vet -c=false 16.72user 0.69system 0:12.66elapsed 137%CPU (0avgtext+0avgdata 2950188maxresident)k Note that two test outputs are reordered due to swapping DFS for BFS. Also note that the CloseIDElems changes are due to the old algorithm incrementing the counter even when an element is marked via deleteID. The new algorithm skips those elements upfront, so the counter is not incremented for them. This algorithm refactor was done with assistance from Gemini 2.5 Pro, which had to be nudged a few times to avoid test failures. I tweaked it quite a lot to ensure that memory reuse is preserved, to avoid unnecessary steps such as sorting the results, and to ensure that the "embed" logic is kept exactly as it was before. The new algorithm also uses more maps and slices, so we also expand OpContext from one to three fields to hold memory so that it can be reused between calls to replaceIDs. This helps reduce the memory usage on the project above with the new algorithm from ~10GiB to ~5GiB, avoiding a regression in the memory usage of closedness. Updates #3881. Signed-off-by: Daniel Martí <[email protected]> Change-Id: I714c26af22967ff89c9f0f242e027608d596f8fb Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1217153 Reviewed-by: Marcel van Lohuizen <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]>
1 parent 361384c commit b4212a3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+174
-162
lines changed

cue/testdata/benchmarks/issue1684.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Unifications: 5541
7272
Conjuncts: 25871
7373
Disjuncts: 6534
7474

75-
CloseIDElems: 85837
75+
CloseIDElems: 74207
7676
NumCloseIDs: 4115
7777
-- out/evalalpha --
7878
(struct){
@@ -159,7 +159,7 @@ diff old new
159159
+Conjuncts: 25871
160160
+Disjuncts: 6534
161161
+
162-
+CloseIDElems: 85837
162+
+CloseIDElems: 74207
163163
+NumCloseIDs: 4115
164164
-- diff/-out/evalalpha<==>+out/eval --
165165
diff old new

cue/testdata/benchmarks/issue2176.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Unifications: 377
6868
Conjuncts: 1333
6969
Disjuncts: 8
7070

71-
CloseIDElems: 20071
71+
CloseIDElems: 20070
7272
NumCloseIDs: 535
7373
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
7474
diff old new
@@ -97,7 +97,7 @@ diff old new
9797
+Conjuncts: 1333
9898
+Disjuncts: 8
9999
+
100-
+CloseIDElems: 20071
100+
+CloseIDElems: 20070
101101
+NumCloseIDs: 535
102102
-- out/eval/stats --
103103
Leaks: 90

cue/testdata/benchmarks/issue3514.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ Unifications: 69
4646
Conjuncts: 7053
4747
Disjuncts: 2386
4848

49-
CloseIDElems: 92
49+
CloseIDElems: 77
5050
NumCloseIDs: 5733
5151
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
5252
diff old new
@@ -72,7 +72,7 @@ diff old new
7272
+Conjuncts: 7053
7373
+Disjuncts: 2386
7474
+
75-
+CloseIDElems: 92
75+
+CloseIDElems: 77
7676
+NumCloseIDs: 5733
7777
-- out/eval/stats --
7878
Leaks: 27

cue/testdata/benchmarks/issue3633.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Unifications: 667
1010
Conjuncts: 2680
1111
Disjuncts: 1955
1212

13-
CloseIDElems: 7234
13+
CloseIDElems: 6838
1414
NumCloseIDs: 53
1515
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
1616
diff old new
@@ -34,7 +34,7 @@ diff old new
3434
+Conjuncts: 2680
3535
+Disjuncts: 1955
3636
+
37-
+CloseIDElems: 7234
37+
+CloseIDElems: 6838
3838
+NumCloseIDs: 53
3939
-- out/eval/stats --
4040
Leaks: 0

cue/testdata/benchmarks/listdedup.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ Unifications: 40
4343
Conjuncts: 98
4444
Disjuncts: 24
4545

46-
CloseIDElems: 1206
46+
CloseIDElems: 1092
4747
NumCloseIDs: 22
4848
-- out/evalalpha --
4949
(struct){
@@ -486,7 +486,7 @@ diff old new
486486
+Conjuncts: 98
487487
+Disjuncts: 24
488488
+
489-
+CloseIDElems: 1206
489+
+CloseIDElems: 1092
490490
+NumCloseIDs: 22
491491
-- out/eval/stats --
492492
Leaks: 0

cue/testdata/benchmarks/typocheck.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ Unifications: 703
3131
Conjuncts: 1305
3232
Disjuncts: 0
3333

34-
CloseIDElems: 45090
34+
CloseIDElems: 43902
3535
NumCloseIDs: 212
3636
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
3737
diff old new
@@ -60,7 +60,7 @@ diff old new
6060
+Conjuncts: 1305
6161
+Disjuncts: 0
6262
+
63-
+CloseIDElems: 45090
63+
+CloseIDElems: 43902
6464
+NumCloseIDs: 212
6565
-- out/eval/stats --
6666
Leaks: 0

cue/testdata/builtins/closed.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ Unifications: 230
102102
Conjuncts: 360
103103
Disjuncts: 12
104104

105-
CloseIDElems: 2194
105+
CloseIDElems: 1773
106106
NumCloseIDs: 106
107107
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
108108
diff old new
@@ -128,7 +128,7 @@ diff old new
128128
+Conjuncts: 360
129129
+Disjuncts: 12
130130
+
131-
+CloseIDElems: 2194
131+
+CloseIDElems: 1773
132132
+NumCloseIDs: 106
133133
-- out/eval/stats --
134134
Leaks: 61

cue/testdata/builtins/matchn.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ Unifications: 931
356356
Conjuncts: 1511
357357
Disjuncts: 16
358358

359-
CloseIDElems: 1371
359+
CloseIDElems: 1170
360360
NumCloseIDs: 483
361361
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
362362
diff old new
@@ -382,7 +382,7 @@ diff old new
382382
+Conjuncts: 1511
383383
+Disjuncts: 16
384384
+
385-
+CloseIDElems: 1371
385+
+CloseIDElems: 1170
386386
+NumCloseIDs: 483
387387
-- out/eval/stats --
388388
Leaks: 52

cue/testdata/compile/scope.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ Unifications: 22
8888
Conjuncts: 39
8989
Disjuncts: 2
9090

91-
CloseIDElems: 120
91+
CloseIDElems: 116
9292
NumCloseIDs: 13
9393
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
9494
diff old new
@@ -114,7 +114,7 @@ diff old new
114114
+Conjuncts: 39
115115
+Disjuncts: 2
116116
+
117-
+CloseIDElems: 120
117+
+CloseIDElems: 116
118118
+NumCloseIDs: 13
119119
-- out/eval/stats --
120120
Leaks: 2

cue/testdata/comprehensions/issue3929.txtar

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ Unifications: 97
9797
Conjuncts: 266
9898
Disjuncts: 53
9999

100-
CloseIDElems: 1972
100+
CloseIDElems: 1903
101101
NumCloseIDs: 101
102102
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
103103
diff old new
@@ -126,7 +126,7 @@ diff old new
126126
+Conjuncts: 266
127127
+Disjuncts: 53
128128
+
129-
+CloseIDElems: 1972
129+
+CloseIDElems: 1903
130130
+NumCloseIDs: 101
131131
-- out/eval/stats --
132132
Leaks: 3

0 commit comments

Comments
 (0)