internal/core/adt: more pruning in partial disjunction check

In a partial conjunction check, equality for uncompleted
tasks need to be verified to be equal. In many cases
tasks will have completed before a disjunction is checked.
This is especially true for tasks that result from a
reference like `string`.

Previously, the implementation ignored the scheduler's
taskPos field, thereby considering any task to be
not completed, causing many disambiguation
opportunities to be missed. The new implementation
fast tracks the common case when both disjuncts
have all tasks completed. Furthermore, it only
compares tasks that are not done yet.

This adds one nodeCounter issue, which is related
to the embedded identifier. This will need to be
addressed elsewhere.

Fixes #3610

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ib3a5830d415e9afd74e8c96b6a0b97383c53218c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1205400
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: mpvl

Diff for: cue/testdata/benchmarks/disjunctelim.txtar

@@ -25,47 +25,61 @@ list: [1] | [2] | [3] | [4] // 151 conjuncts; 68  disjuncts (+33; +16)
 list: [1] | [2] | [3] | [4] // 184 conjuncts; 84  disjuncts (+33; +16)
 list: [1] | [2] | [3] | [4] // 217 conjuncts; 100 disjuncts (+33; +16)
 
+issue3610: {
+	// Ensure disjuncts are disambiguated. The string field here is a reference
+	// and causes a task to be added. Ensure that, for instance, disjuncts
+	// >"b" and (>"b" & string) get equated when at least the task has
+	// completed.
+	#a
+	#a: <"a" | >"b" | string
+	#a: <"a" | >"b" | string // conjuncts 127  disjuncts 120
+	#a: <"a" | >"b" | string // conjuncts 219  disjuncts 210 (+92; +90)
+	#a: <"a" | >"b" | string // conjuncts 311  disjuncts 300 (+92; +90)
+	#a: <"a" | >"b" | string // conjuncts 403  disjuncts 390 (+92; +90)
+	#a: <"a" | >"b" | string // conjuncts 495  disjuncts 480 (+92; +90)
+}
+
 -- out/evalalpha/stats --
-Leaks:  182
-Freed:  90
-Reused: 90
-Allocs: 182
+Leaks:  572
+Freed:  230
+Reused: 230
+Allocs: 572
 Retain: 0
 
-Unifications: 20
-Conjuncts:    327
-Disjuncts:    138
+Unifications: 22
+Conjuncts:    825
+Disjuncts:    618
 -- diff/-out/evalalpha/stats<==>+out/eval/stats --
 diff old new
 --- old
 +++ new
 @@ -1,9 +1,9 @@
 -Leaks:  0
--Freed:  269
--Reused: 258
+-Freed:  367
+-Reused: 356
 -Allocs: 11
-+Leaks:  182
-+Freed:  90
-+Reused: 90
-+Allocs: 182
++Leaks:  572
++Freed:  230
++Reused: 230
++Allocs: 572
  Retain: 0
 
--Unifications: 131
--Conjuncts:    755
--Disjuncts:    269
-+Unifications: 20
-+Conjuncts:    327
-+Disjuncts:    138
+-Unifications: 133
+-Conjuncts:    866
+-Disjuncts:    367
++Unifications: 22
++Conjuncts:    825
++Disjuncts:    618
 -- out/eval/stats --
 Leaks:  0
-Freed:  269
-Reused: 258
+Freed:  367
+Reused: 356
 Allocs: 11
 Retain: 0
 
-Unifications: 131
-Conjuncts:    755
-Disjuncts:    269
+Unifications: 133
+Conjuncts:    866
+Disjuncts:    367
 -- out/eval --
 (struct){
   pat: (struct){ |((struct){
@@ -81,6 +95,16 @@ Disjuncts:    269
     }, (#list){
       0: (int){ 4 }
     }) }
+  issue3610: (string){ |((string){
+      <"a"
+      #a: (string){ |((string){ <"a" }, (string){ >"b" }, (string){ string }) }
+    }, (string){
+      >"b"
+      #a: (string){ |((string){ <"a" }, (string){ >"b" }, (string){ string }) }
+    }, (string){
+      string
+      #a: (string){ |((string){ <"a" }, (string){ >"b" }, (string){ string }) }
+    }) }
 }
 -- out/compile --
 --- in.cue
@@ -198,4 +222,13 @@ Disjuncts:    269
   ]|[
     4,
   ])
+  issue3610: {
+    〈0;#a〉
+    #a: (<"a"|>"b"|string)
+    #a: (<"a"|>"b"|string)
+    #a: (<"a"|>"b"|string)
+    #a: (<"a"|>"b"|string)
+    #a: (<"a"|>"b"|string)
+    #a: (<"a"|>"b"|string)
+  }
 }

Diff for: cue/testdata/eval/conjuncts.txtar

@@ -69,15 +69,15 @@ issue2355: {
 }
 
 -- out/evalalpha/stats --
-Leaks:  192
+Leaks:  188
 Freed:  24
 Reused: 24
-Allocs: 192
+Allocs: 188
 Retain: 0
 
 Unifications: 43
-Conjuncts:    352
-Disjuncts:    126
+Conjuncts:    350
+Disjuncts:    124
 -- out/evalalpha --
 Errors:
 param: conflicting values "foo" and [{}] (mismatched types string and list):
@@ -168,18 +168,18 @@ diff old new
 -Reused: 59
 -Allocs: 18
 -Retain: 22
-+Leaks:  192
++Leaks:  188
 +Freed:  24
 +Reused: 24
-+Allocs: 192
++Allocs: 188
 +Retain: 0
 
 -Unifications: 45
 -Conjuncts:    135
 -Disjuncts:    86
 +Unifications: 43
-+Conjuncts:    352
-+Disjuncts:    126
++Conjuncts:    350
++Disjuncts:    124
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new
 --- old

Diff for: internal/core/adt/disjunct2.go

@@ -573,10 +573,12 @@ outer:
 					}
 				}
 			}
-			if len(xn.tasks) != len(x.tasks) {
-				continue
+			if len(xn.tasks) != xn.taskPos || len(x.tasks) != x.taskPos {
+				if len(xn.tasks) != len(x.tasks) {
+					continue
+				}
 			}
-			for i, t := range xn.tasks {
+			for i, t := range xn.tasks[xn.taskPos:] {
 				s := x.tasks[i]
 				if s.x != t.x || s.id.cc != t.id.cc {
 					continue outer
@@ -735,8 +737,10 @@ func isEqualNodeValue(x, y *nodeContext) bool {
 	if len(x.checks) != len(y.checks) {
 		return false
 	}
-	if len(x.tasks) != len(y.tasks) {
-		return false
+	if len(x.tasks) != x.taskPos || len(y.tasks) != y.taskPos {
+		if len(x.tasks) != len(y.tasks) {
+			return false
+		}
 	}
 
 	if !isEqualValue(x.ctx, x.lowerBound, y.lowerBound) {
@@ -754,7 +758,7 @@ func isEqualNodeValue(x, y *nodeContext) bool {
 		}
 	}
 
-	for i, t := range x.tasks {
+	for i, t := range x.tasks[x.taskPos:] {
 		s := y.tasks[i]
 		if s.x != t.x {
 			return false

Diff for: internal/core/adt/eval_test.go

@@ -77,6 +77,7 @@ var needFix = map[string]string{
 // TODO: These counters should all go to zero.
 var skipDebugDepErrors = map[string]int{
 	"benchmarks/issue1684":     16,
+	"benchmarks/disjunctelim":  1,
 	"builtins/default":         1,
 	"compile/scope":            1,
 	"comprehensions/pushdown":  3,