Skip to content

Commit f1ea2ff

Browse files
committedDec 9, 2024·
internal/core/adt: patch performance issue related to error reporting
In V3, there are multiple points an error may be added to the results. If an error ends up being added to more than one path, it may duplicate, causing errors to grow exponentially. The real solution would be to only have single points of entry. But for now we just dedup entries in the low-level append function. Fixes #3517 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I43f4531027f1e71d7e4610de3cd424b8a2dfda57 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1205368 TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]> Reviewed-by: Matthew Sackman <[email protected]>
1 parent 6c86b41 commit f1ea2ff

File tree

5 files changed

+784
-8
lines changed

5 files changed

+784
-8
lines changed
 

‎cue/errors/errors.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,19 @@ func appendToList(a list, err Error) list {
309309
case nil:
310310
return a
311311
case list:
312-
if a == nil {
312+
if len(a) == 0 {
313313
return x
314314
}
315-
return append(a, x...)
315+
for _, e := range x {
316+
a = appendToList(a, e)
317+
}
318+
return a
316319
default:
320+
for _, e := range a {
321+
if e == err {
322+
return a
323+
}
324+
}
317325
return append(a, err)
318326
}
319327
}

‎cue/testdata/benchmarks/issue3517.txtar

+767
Large diffs are not rendered by default.

‎internal/core/adt/sched.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -729,8 +729,9 @@ func runTask(t *task, mode runMode) {
729729
} else {
730730
t.state = taskFAILED
731731
}
732-
t.node.addBottom(t.err) // TODO: replace with something more principled.
733-
732+
// TODO: do not add both context and task errors. Do something more
733+
// principled.
734+
t.node.addBottom(t.err)
734735
if t.id.cc != nil {
735736
t.id.cc.decDependent(ctx, TASK, nil)
736737
}

‎internal/core/export/testdata/main/let.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ diff old new
915915
- name: "two"
916916
- }
917917
- }]
918-
- files: _|_ // invalid interpolation: cycle error (and 3 more errors)
918+
- files: _|_ // invalid interpolation: cycle error (and 1 more errors)
919919
+ comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: comprehension.filepath: undefined field: name) (and 1 more errors)
920920
+ complete: {
921921
+ x: "a foo z"
@@ -1173,7 +1173,7 @@ Improved error messages.
11731173
name: "two"
11741174
}
11751175
}]
1176-
files: _|_ // invalid interpolation: cycle error (and 3 more errors)
1176+
files: _|_ // invalid interpolation: cycle error (and 1 more errors)
11771177
scoped: {
11781178
direct: {
11791179
a: 1

‎internal/core/export/testdata/main/simplify.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ diff old new
113113
s: strings.MinRunes(4) & strings.MaxRunes(7)
114114
additional: {
115115
env: _
116-
- confs: _|_ // additional.confs: incomplete bool: _ (and 1 more errors)
116+
- confs: _|_ // incomplete bool: _
117117
+ confs: _|_ // additional.confs: incomplete bool: _
118118
}
119119
}
@@ -153,7 +153,7 @@ diff old new
153153
s: strings.MinRunes(4) & strings.MaxRunes(7)
154154
additional: {
155155
env: _
156-
confs: _|_ // additional.confs: incomplete bool: _ (and 1 more errors)
156+
confs: _|_ // incomplete bool: _
157157
}
158158
}
159159
== All

0 commit comments

Comments
 (0)
Please sign in to comment.