internal/core/adt: restrict notification
This restricts the cases where notifications are added,
limiting the overhead of the mechanism.
Issue #661
Change-Id: I4d8929dfb6136ef994e39eb50c1ede39c2f13f60
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8402
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/testdata/cycle/049_self-reference_cycles_conflicts_with_strings.txtar b/cue/testdata/cycle/049_self-reference_cycles_conflicts_with_strings.txtar
index 875d6e0..e764e84 100644
--- a/cue/testdata/cycle/049_self-reference_cycles_conflicts_with_strings.txtar
+++ b/cue/testdata/cycle/049_self-reference_cycles_conflicts_with_strings.txtar
@@ -42,10 +42,6 @@
// ./in.cue:2:5
// ./in.cue:5:7
}
- y: (_|_){
- // [eval] a.x: conflicting values "hey!?" and "hey":
- // ./in.cue:2:5
- // ./in.cue:5:7
- }
+ y: (string){ "hey!" }
}
}
diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
index 1f6049e..394cd49 100644
--- a/internal/core/adt/eval.go
+++ b/internal/core/adt/eval.go
@@ -144,7 +144,14 @@
}
if v.status < Finalized && v.state != nil {
- v.state.addNotify(c.vertex)
+ // TODO: errors are slightly better if we always add addNotify, but
+ // in this case it is less likely to cause a performance penalty.
+ // See https://github.com/cuelang/cue/issues/661. It may be possible to
+ // relax this again once we have proper tests to prevent regressions of
+ // that issue.
+ if !v.state.done() || v.state.errs != nil {
+ v.state.addNotify(c.vertex)
+ }
}
return v
