internal/core/adt: remove over-aggressive deduplication

This may negatively impact performance, but with the current tests the impact seems manageable. Fixes #1974 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Id04b6d57d77616af875566388cf9692cefd32787 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/545627 Reviewed-by: Roger Peppe <rogpeppe@gmail.com> Unity-Result: CUEcueckoo <cueckoo@cuelang.org> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Paul Jolly <paul@myitcv.io>
cue-lang · Nov 14, 2022 · 74b4288 · 74b4288
1 parent 28088ac
commit 74b4288
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 13 deletions.
diff --git a/cue/testdata/benchmarks/mergeddisjunction.txtar b/cue/testdata/benchmarks/mergeddisjunction.txtar
@@ -42,14 +42,14 @@ b: {
 }
 -- out/eval/stats --
 Leaks:  0
-Freed:  163
-Reused: 157
-Allocs: 6
+Freed:  283
+Reused: 275
+Allocs: 8
 Retain: 0
 
 Unifications: 99
-Conjuncts:    290
-Disjuncts:    163
+Conjuncts:    530
+Disjuncts:    283
 -- out/eval --
 (struct){
   list: (#list){

diff --git a/cue/testdata/builtins/closed.txtar b/cue/testdata/builtins/closed.txtar
@@ -49,7 +49,7 @@ Allocs: 37
 Retain: 27
 
 Unifications: 178
-Conjuncts:    332
+Conjuncts:    354
 Disjuncts:    188
 -- out/eval --
 Errors:

diff --git a/cue/testdata/comprehensions/nested2.txtar b/cue/testdata/comprehensions/nested2.txtar
@@ -80,16 +80,31 @@ indirectlyDoublyNested: {
 	}
 }
 
+// This case used to trigger an over-aggressive deduplication.
+issue1974: {
+	X: {
+		foo: {
+			bar: ""
+			baz: ""
+		}
+	}
+	out: {
+		for k, v in X for vk, vv in v {
+			"\(k)": "\(vk)": vv
+		}
+	}
+}
+
 -- out/eval/stats --
 Leaks:  0
-Freed:  34
-Reused: 30
-Allocs: 4
+Freed:  43
+Reused: 38
+Allocs: 5
 Retain: 5
 
-Unifications: 34
-Conjuncts:    55
-Disjuncts:    34
+Unifications: 43
+Conjuncts:    69
+Disjuncts:    43
 -- out/eval --
 (struct){
   given: (struct){
@@ -138,6 +153,20 @@ Disjuncts:    34
     c: (int){ 1 }
     a: (bool){ true }
   }
+  issue1974: (struct){
+    X: (struct){
+      foo: (struct){
+        bar: (string){ "" }
+        baz: (string){ "" }
+      }
+    }
+    out: (struct){
+      foo: (struct){
+        bar: (string){ "" }
+        baz: (string){ "" }
+      }
+    }
+  }
 }
 -- out/compile --
 --- in.cue
@@ -226,4 +255,19 @@ Disjuncts:    34
       }
     }
   }
+  issue1974: {
+    X: {
+      foo: {
+        bar: ""
+        baz: ""
+      }
+    }
+    out: {
+      for k, v in 〈1;X〉 for vk, vv in 〈0;v〉 {
+        "\(〈2;k〉)": {
+          "\(〈2;vk〉)": 〈2;vv〉
+        }
+      }
+    }
+  }
 }
diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go
@@ -754,7 +754,7 @@ func (v *Vertex) addConjunct(c Conjunct) {
 	}
 	for _, x := range v.Conjuncts {
 		// TODO: disregard certain fields from comparison (e.g. Refs)?
-		if x.CloseInfo.closeInfo == c.CloseInfo.closeInfo && x.x == c.x {
+		if x == c {
 			return
 		}
 	}