Skip to content

Commit

Permalink
internal/core/adt: disable prep for associative lists
Browse files Browse the repository at this point in the history
This preparation is currently causing significant
performance issues. As it is not used, we disable it
for now.

The issue follows from the following:
1) the list type can (currently) only be known after
    the last conjunct is processed.
2) lists are not processed until the list type is known
3) this means that lists are not processed and added
    to the disjunct for any but the last disjunction
4) this means that disjuncts with the same list cannot
     be compared and are not prematurely eliminated.

There are many possible ways to fix this. For instance,
1) we could first evaluate disjuncts, before taking
   their cross product, to determine what the list
   type will be.
2) as we only care about equality, we could anyway
    evaluate lists, and compare lists collected so far
    based on equality. This can especially be fast
    when we have a fast equality check.

For now we just disable the state.

This change exposes another issue: basically, if
evaluated in the right order, a closedContext.Expr
may be non-nil if a struct is not closed. The
resulting panic is just to catch an optimization.
This will be easier to fix once we support the
same semantics for lists as we have for structs.
So for now, we just disable the check.

Note that this causes some error messages to
now mention a "cycle error", which is indicative
of displaying an unevaluated value. We will worry
about that later. It is not material for the result.

Issue #2850

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I11df185bb5d3747e72c3ca2888ee4dd7cce70d47
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1204490
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
  • Loading branch information
mpvl committed Nov 26, 2024
1 parent 5c9b07f commit 00ca1c9
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 162 deletions.
28 changes: 14 additions & 14 deletions cue/testdata/benchmarks/disjunctelim.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ list: [1] | [2] | [3] | [4] // 184 conjuncts; 84 disjuncts (+33; +16)
list: [1] | [2] | [3] | [4] // 217 conjuncts; 100 disjuncts (+33; +16)

-- out/evalalpha/stats --
Leaks: 25997
Freed: 16398
Reused: 16398
Allocs: 25997
Leaks: 182
Freed: 90
Reused: 90
Allocs: 182
Retain: 0

Unifications: 4115
Conjuncts: 71139
Disjuncts: 21882
Unifications: 20
Conjuncts: 327
Disjuncts: 138
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
diff old new
--- old
Expand All @@ -44,18 +44,18 @@ diff old new
-Freed: 269
-Reused: 258
-Allocs: 11
+Leaks: 25997
+Freed: 16398
+Reused: 16398
+Allocs: 25997
+Leaks: 182
+Freed: 90
+Reused: 90
+Allocs: 182
Retain: 0

-Unifications: 131
-Conjuncts: 755
-Disjuncts: 269
+Unifications: 4115
+Conjuncts: 71139
+Disjuncts: 21882
+Unifications: 20
+Conjuncts: 327
+Disjuncts: 138
-- out/eval/stats --
Leaks: 0
Freed: 269
Expand Down
16 changes: 8 additions & 8 deletions cue/testdata/benchmarks/listdedup.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ C: #steps: #Script & {mount: [B]}

#ref: {a: 1} | {b: 2}
-- out/evalalpha/stats --
Leaks: 163
Leaks: 158
Freed: 10
Reused: 10
Allocs: 163
Allocs: 158
Retain: 0

Unifications: 53
Conjuncts: 407
Unifications: 56
Conjuncts: 461
Disjuncts: 40
-- out/evalalpha --
(struct){
Expand Down Expand Up @@ -545,17 +545,17 @@ diff old new
-Reused: 24051
-Allocs: 45
-Retain: 1
+Leaks: 163
+Leaks: 158
+Freed: 10
+Reused: 10
+Allocs: 163
+Allocs: 158
+Retain: 0

-Unifications: 18724
-Conjuncts: 100730
-Disjuncts: 24097
+Unifications: 53
+Conjuncts: 407
+Unifications: 56
+Conjuncts: 461
+Disjuncts: 40
-- out/eval/stats --
Leaks: 0
Expand Down
51 changes: 16 additions & 35 deletions cue/testdata/cycle/comprehension.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -310,14 +310,14 @@ issue2367: {
}

-- out/evalalpha/stats --
Leaks: 775
Leaks: 778
Freed: 63
Reused: 63
Allocs: 775
Allocs: 778
Retain: 0

Unifications: 495
Conjuncts: 3328
Unifications: 498
Conjuncts: 3349
Disjuncts: 196
-- out/evalalpha --
Errors:
Expand Down Expand Up @@ -431,14 +431,15 @@ Result:
}
}
list: (struct){
panels: (_|_){
// [incomplete] selfReferential.list.panels: cannot range over panels (incomplete type _):
// ./in.cue:103:15
panels: (#list){
0: (struct){
id: (int){ 0 }
}
1: (struct){
id: (int){ 1 }
}
2: (struct){
id: (int){ 2 }
}
}
}
Expand Down Expand Up @@ -779,26 +780,7 @@ diff old new
}
}
}
@@ -112,15 +109,14 @@
}
}
list: (struct){
- panels: (#list){
+ panels: (_|_){
+ // [incomplete] selfReferential.list.panels: cannot range over panels (incomplete type _):
+ // ./in.cue:103:15
0: (struct){
- id: (int){ 0 }
}
1: (struct){
- id: (int){ 1 }
}
2: (struct){
- id: (int){ 2 }
}
}
}
@@ -127,9 +123,9 @@
@@ -127,9 +124,9 @@
insertionError: (_|_){
// [eval]
A: (_|_){
Expand All @@ -810,7 +792,7 @@ diff old new
}
}
acrossOr: (struct){
@@ -268,13 +264,17 @@
@@ -268,13 +265,17 @@
reject: (string){ string }
}, (#struct){
resource: (string){ string }
Expand All @@ -835,7 +817,7 @@ diff old new
}) }
}
p2: (struct){
@@ -328,7 +328,8 @@
@@ -328,7 +329,8 @@
}
}
}
Expand All @@ -845,7 +827,7 @@ diff old new
t1: (struct){
p1: (struct){
D: (struct){
@@ -361,35 +362,28 @@
@@ -361,35 +363,28 @@
}
}
}
Expand Down Expand Up @@ -913,17 +895,17 @@ diff old new
-Reused: 1260
-Allocs: 60
-Retain: 145
+Leaks: 775
+Leaks: 778
+Freed: 63
+Reused: 63
+Allocs: 775
+Allocs: 778
+Retain: 0

-Unifications: 832
-Conjuncts: 2525
-Disjuncts: 1404
+Unifications: 495
+Conjuncts: 3328
+Unifications: 498
+Conjuncts: 3349
+Disjuncts: 196
-- out/eval/stats --
Leaks: 50
Expand All @@ -938,7 +920,6 @@ Disjuncts: 1404
-- diff/explanation --
B.a.children: now correctly marked as incomplete
-- diff/todo/p0 --
selfReferential.list.panels: invalid incomplete error: related to "or" builtin
siblingInsertion.t2: errors
-- diff/todo/p2 --
issue1881.#AllOutputs.retry: additional entry.
Expand Down
47 changes: 11 additions & 36 deletions cue/testdata/cycle/issue494.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ Conjuncts: 83
Disjuncts: 45
-- out/evalalpha --
Errors:
f.ben: incompatible list lengths (1 and 2):
./in.cue:20:17
g.ben: incompatible list lengths (1 and 2):
./in.cue:23:17
f.ben: incompatible list lengths (1 and 2)
g.ben: incompatible list lengths (1 and 2)

Result:
(_|_){
Expand Down Expand Up @@ -93,17 +91,15 @@ Result:
f: (_|_){
// [eval]
ben: (_|_){
// [eval] f.ben: incompatible list lengths (1 and 2):
// ./in.cue:20:17
// [eval] f.ben: incompatible list lengths (1 and 2)
0: (struct){
}
}
}
g: (_|_){
// [eval]
ben: (_|_){
// [eval] g.ben: incompatible list lengths (1 and 2):
// ./in.cue:23:17
// [eval] g.ben: incompatible list lengths (1 and 2)
0: (struct){
}
1: (struct){
Expand All @@ -115,18 +111,7 @@ Result:
diff old new
--- old
+++ new
@@ -1,6 +1,8 @@
Errors:
-f.ben: incompatible list lengths (1 and 2)
-g.ben: incompatible list lengths (1 and 2)
+f.ben: incompatible list lengths (1 and 2):
+ ./in.cue:20:17
+g.ben: incompatible list lengths (1 and 2):
+ ./in.cue:23:17

Result:
(_|_){
@@ -41,13 +43,7 @@
@@ -41,13 +41,7 @@
}
}
}
Expand All @@ -141,32 +126,22 @@ diff old new
e: (struct){
ben: (#list){
0: (struct){
@@ -61,12 +57,9 @@
f: (_|_){
// [eval]
@@ -63,10 +57,6 @@
ben: (_|_){
- // [eval] f.ben: incompatible list lengths (1 and 2)
- 0: (struct){
// [eval] f.ben: incompatible list lengths (1 and 2)
0: (struct){
- pos: (int){ 0 }
- }
- 1: (struct){
- pos: (int){ 1 }
+ // [eval] f.ben: incompatible list lengths (1 and 2):
+ // ./in.cue:20:17
+ 0: (struct){
}
}
}
@@ -73,9 +66,9 @@
g: (_|_){
// [eval]
@@ -75,7 +65,6 @@
ben: (_|_){
- // [eval] g.ben: incompatible list lengths (1 and 2)
- 0: (struct){
// [eval] g.ben: incompatible list lengths (1 and 2)
0: (struct){
- pos: (int){ 0 }
+ // [eval] g.ben: incompatible list lengths (1 and 2):
+ // ./in.cue:23:17
+ 0: (struct){
}
1: (struct){
}
Expand Down
Loading

0 comments on commit 00ca1c9

Please sign in to comment.