Skip to content

Commit

Permalink
internal/core/adt: new cycle algorithm
Browse files Browse the repository at this point in the history
V3 only partially implemented the V2 cycle algorithm.
Rather than implementing the rest, we came up with a
new algorithm that fits better with the depth-first nature
of V3. It is overall considerably simpler than the V2
algorithm.

This CL reflects a "mostly done" state. There are still
improvements to be made (e.g. inline and let fields), but
it is already in better shape that V3 was before. Further
fixes will be done in subsequent CLs.

Note that this algorith is much simpler. It no longer
needs:
- track individual reference points (although it
  still does so for debugging purposes, but it
  does not use it to influence outcome)
- track nodeContext.cyclicReferences
- have a special mechanism to detect and disable
  mutual references
- track depth of optional markers
- track depth of references
- additional finding of evidence of non-cyclic nodes
  in some cases

CHANGES IN TESTS

benchmarks/issue1684:
    more efficient at trimming disjunctions
comprehensions/errors:
    more efficient at trimming disjunctions
cycle/comprehenion:
    changes are strict improvements, even though they
    do not fix the underlying bugs
cycle/constraints:
    - both shorter and deeper cutoffs
cycle/evaluate:
    - reordering, mostly
cycle/inline_non_recursive:
    - FIXES
cycle/inline:
    - Minor improvements
    - Does not yet fix underlying bug
cycle/structural:
    - Some processing is now deeper
    - Some less deep
	- Some P0-level bugs now FIXED that were
	  somehow not listed in diff/todo/*.
eval/v0.7:
	- NEW BUG: notification mechanism bypassed,
	  it seems.
export/030:
    - performance improvements

Issue #2850

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I280fcf9cb333735fcc1774f980bf34e7ed72c9a9
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1201897
Reviewed-by: Matthew Sackman <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
  • Loading branch information
mpvl committed Oct 1, 2024
1 parent a0138fb commit 92330b4
Show file tree
Hide file tree
Showing 19 changed files with 1,124 additions and 800 deletions.
58 changes: 25 additions & 33 deletions cue/testdata/benchmarks/issue1684.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ nestedClosed: passing: {
D: {id: {}} | {[string]: D}
}
-- out/evalalpha/stats --
Leaks: 2533
Freed: 612
Reused: 612
Allocs: 2533
Leaks: 2135
Freed: 658
Reused: 658
Allocs: 2135
Retain: 0

Unifications: 707
Conjuncts: 5373
Disjuncts: 1102
Unifications: 475
Conjuncts: 4781
Disjuncts: 918
-- out/evalalpha --
(struct){
#Secret: (#struct){
Expand Down Expand Up @@ -127,12 +127,6 @@ Disjuncts: 1102
id: (struct){ |((struct){
id: (struct){
}
}, (struct){
id: (struct){ |((struct){
id: (struct){
}
}, (struct){
}) }
}, (struct){
}) }
}, (struct){
Expand All @@ -149,28 +143,26 @@ diff old new
-Freed: 1064333
-Reused: 1064282
-Allocs: 51
+Leaks: 2533
+Freed: 612
+Reused: 612
+Allocs: 2533
+Leaks: 2135
+Freed: 658
+Reused: 658
+Allocs: 2135
Retain: 0

-Unifications: 792123
-Conjuncts: 2480117
-Disjuncts: 1064333
+Unifications: 707
+Conjuncts: 5373
+Disjuncts: 1102
+Unifications: 475
+Conjuncts: 4781
+Disjuncts: 918
-- diff/-out/evalalpha<==>+out/eval --
diff old new
--- old
+++ new
@@ -54,35 +54,11 @@
id: (struct){
}
@@ -56,36 +56,6 @@
}, (struct){
- }) }
- }, (struct){
}) }
}, (struct){
- id: (struct){ |((struct){
- id: (struct){
- }
Expand Down Expand Up @@ -198,14 +190,14 @@ diff old new
- id: (struct){ |((struct){
- id: (struct){
- }
+ id: (struct){ |((struct){
+ id: (struct){
+ }
+ }, (struct){
+ }) }
}, (struct){
}) }
}, (struct){
- }, (struct){
- }) }
- }, (struct){
}) }
}
}
-- diff/explanation --
New algorithm is better at trimming recursive disjunctions.
-- out/eval/stats --
Leaks: 0
Freed: 1064333
Expand Down
22 changes: 11 additions & 11 deletions cue/testdata/benchmarks/listdisj.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,6 @@ Retain: 0
Unifications: 15
Conjuncts: 43
Disjuncts: 4
-- out/evalalpha --
(struct){
#T: (list){
0: (string){ "d" }
}
x: ~(#T)
y: ~(#T)
z: ~(#T)
v: ~(#T)
#X: ~(#T)
}
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
diff old new
--- old
Expand All @@ -96,6 +85,17 @@ diff old new
+Unifications: 15
+Conjuncts: 43
+Disjuncts: 4
-- out/evalalpha --
(struct){
#T: (list){
0: (string){ "d" }
}
x: ~(#T)
y: ~(#T)
z: ~(#T)
v: ~(#T)
#X: ~(#T)
}
-- diff/-out/evalalpha<==>+out/eval --
diff old new
--- old
Expand Down
19 changes: 14 additions & 5 deletions cue/testdata/comprehensions/errors.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ Result:
// [eval]
circularIf: (struct){
#list: (#struct){
tail: ((null|struct)){ |(*(null){ null }, (#struct){
tail: (null){ null }
}) }
tail: (null){ null }
}
}
circularFor: (_|_){
Expand Down Expand Up @@ -116,7 +114,18 @@ diff old new
circularFor.#list: cannot range over tail != null (found bool, want list or struct):
./in.cue:12:12
intField: integer fields not supported:
@@ -35,10 +34,6 @@
@@ -13,9 +12,7 @@
// [eval]
circularIf: (struct){
#list: (#struct){
- tail: ((null|struct)){ |(*(null){ null }, (#struct){
- tail: (null){ null }
- }) }
+ tail: (null){ null }
}
}
circularFor: (_|_){
@@ -35,10 +32,6 @@
intField: (_|_){
// [eval] intField: integer fields not supported:
// ./in.cue:27:4
Expand All @@ -127,7 +136,7 @@ diff old new
}
conflictRangingOverSelf: (_|_){
// [eval]
@@ -49,7 +44,6 @@
@@ -49,7 +42,6 @@
// [eval] conflictRangingOverSelf.x.age: conflicting values int and "age" (mismatched types int and string):
// ./in.cue:36:9
// ./in.cue:40:3
Expand Down
Loading

0 comments on commit 92330b4

Please sign in to comment.