this repo has no description
internal/core/adt: handle dependencies fully in overlay
After cleaning up the dependency code a bit, it is
a bit clearer how to make a sound copy of all data
structures involved into counter and dependency
handling.
Note the slight differences in handling notifications
versus arcs. The reason for this is that arcs
always point "down" into a graph, whereas
notifications can point in any direction, including
pointing from within the copied graph to one
outside of it.
Also note that externalDeps are created one to
one with arcs and notifications. So it was
relatively simple to reproduce them.
The main effect of this new logic is that
dependency cycle breaking now also works within
disjunctions. This seems to mostly fix bugs in
the jsonschema package.
It also solves a bunch of counter issues, as
expected and improves the performance counters
of one test.
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I9d6b5885119424bb9e5230384eca32ccc3df79eb
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207455
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
+58
-59
+12
-12
cue/testdata/cycle/issue990.txtar
+12
-12
cue/testdata/cycle/issue990.txtar
···
80
80
-- diff/todo/p3 --
81
81
Reordering
82
82
-- out/evalalpha/stats --
83
-
Leaks: 1834
84
-
Freed: 197
85
-
Reused: 197
86
-
Allocs: 1834
83
+
Leaks: 1182
84
+
Freed: 81
85
+
Reused: 81
86
+
Allocs: 1182
87
87
Retain: 0
88
88
89
89
Unifications: 243
90
-
Conjuncts: 4474
91
-
Disjuncts: 396
90
+
Conjuncts: 4186
91
+
Disjuncts: 204
92
92
-- out/evalalpha --
93
93
(struct){
94
94
#AC: (#struct){
···
328
328
-Reused: 3213
329
329
-Allocs: 25
330
330
-Retain: 26
331
-
+Leaks: 1834
332
-
+Freed: 197
333
-
+Reused: 197
334
-
+Allocs: 1834
331
+
+Leaks: 1182
332
+
+Freed: 81
333
+
+Reused: 81
334
+
+Allocs: 1182
335
335
+Retain: 0
336
336
337
337
-Unifications: 2588
338
338
-Conjuncts: 12056
339
339
-Disjuncts: 3258
340
340
+Unifications: 243
341
-
+Conjuncts: 4474
342
-
+Disjuncts: 396
341
+
+Conjuncts: 4186
342
+
+Disjuncts: 204
343
343
-- diff/-out/evalalpha<==>+out/eval --
344
344
diff old new
345
345
--- old
+2
-2
encoding/jsonschema/external_teststats.txt
+2
-2
encoding/jsonschema/external_teststats.txt
···
6
6
7
7
v3:
8
8
schema extract (pass / total): 1054 / 1363 = 77.3%
9
-
tests (pass / total): 3788 / 4803 = 78.9%
10
-
tests on extracted schemas (pass / total): 3788 / 3955 = 95.8%
9
+
tests (pass / total): 3793 / 4803 = 79.0%
10
+
tests on extracted schemas (pass / total): 3793 / 3955 = 95.9%
11
11
12
12
Optional tests
13
13
+1
-4
encoding/jsonschema/testdata/external/tests/draft2019-09/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft2019-09/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft2020-12/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft2020-12/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft4/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft4/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft6/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft6/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft7/ref.json
+1
-4
encoding/jsonschema/testdata/external/tests/draft7/ref.json
+3
-9
internal/core/adt/eval_test.go
+3
-9
internal/core/adt/eval_test.go
···
76
76
// counter errors.
77
77
// TODO: These counters should all go to zero.
78
78
var skipDebugDepErrors = map[string]int{
79
-
"benchmarks/disjunctelim": 1,
80
-
"benchmarks/issue1684": 16,
81
79
"builtins/default": 1,
82
80
"compile/scope": 1,
83
81
"comprehensions/pushdown": 2,
84
82
"cycle/builtins": 2,
85
-
"cycle/comprehension": 3,
83
+
"cycle/comprehension": 1,
86
84
"cycle/disjunction": 4,
87
85
"cycle/evaluate": 1,
88
86
"cycle/issue990": 1,
89
-
"cycle/structural": 13,
90
-
"disjunctions/edge": 1,
87
+
"cycle/structural": 9,
91
88
"disjunctions/errors": 3,
92
89
"disjunctions/elimination": 19,
93
-
"disjunctions/embed": 12,
94
90
"disjunctions/nested": 1,
95
-
"eval/conjuncts": 3,
96
91
"eval/disjunctions": 3,
97
92
"eval/issue545": 1,
98
-
"eval/notify": 22,
93
+
"eval/notify": 3,
99
94
"export/030": 2,
100
-
"fulleval/054_issue312": 2,
101
95
"scalars/embed": 2,
102
96
}
103
97
+36
-16
internal/core/adt/overlay.go
+36
-16
internal/core/adt/overlay.go
···
407
407
if o.parentConjuncts == nil {
408
408
panic("expected parentConjuncts")
409
409
}
410
+
}
411
+
412
+
func (ctx *overlayContext) finishDependencies(x *closeContext) {
413
+
o := x.overlay
410
414
411
415
for _, a := range x.arcs {
412
416
// If an arc does not have an overlay, we should not decrement the
413
417
// dependency counter. We simply remove the dependency in that case.
414
-
if a.dst.overlay == nil {
418
+
if a.dst.overlay == nil || a.root.overlay == nil {
419
+
panic("arcs should always point inwards and thus included in the overlay")
420
+
}
421
+
if a.decremented {
415
422
continue
416
423
}
417
-
if a.root.overlay != nil {
418
-
a.root = a.root.overlay // TODO: is this necessary?
419
-
}
424
+
a.root = a.root.overlay // TODO: is this necessary?
420
425
a.dst = a.dst.overlay
421
426
o.arcs = append(o.arcs, a)
427
+
428
+
root := a.dst.src.cc()
429
+
root.externalDeps = append(root.externalDeps, ccDepRef{
430
+
src: o,
431
+
kind: ARC,
432
+
index: len(o.arcs) - 1,
433
+
})
422
434
}
423
435
424
436
for _, a := range x.notify {
425
-
// If a notification does not have an overlay, we should not decrement the
426
-
// dependency counter. We simply remove the dependency in that case.
437
+
// If a notification does not have an overlay, we should not decrement
438
+
// the dependency counter. We simply remove the dependency in that case.
439
+
// TODO: however, the original closeContext that it point to now will
440
+
// never be "filled". We should insert top in this gat or render it as
441
+
// "defunct", for instance, so that it will not leave an nondecremented
442
+
// counter.
427
443
if a.dst.overlay == nil {
444
+
for c := a.dst; c != nil; c = c.parent {
445
+
c.disjunctCount++
446
+
}
447
+
continue
448
+
}
449
+
if a.decremented {
428
450
continue
429
451
}
430
452
a.dst = a.dst.overlay
431
453
o.notify = append(o.notify, a)
432
-
}
433
454
434
-
// NOTE: copying externalDeps is hard and seems unnecessary, as it needs to
435
-
// be resolved in the base anyway.
436
-
}
437
-
438
-
func (ctx *overlayContext) finishDependencies(x *closeContext) {
439
-
o := x.overlay
455
+
root := a.dst.src.cc()
456
+
root.externalDeps = append(root.externalDeps, ccDepRef{
457
+
src: o,
458
+
kind: NOTIFY,
459
+
index: len(o.notify) - 1,
460
+
})
461
+
}
440
462
441
463
for _, d := range x.dependencies {
442
464
if d.decremented {
···
459
481
}
460
482
461
483
dep := d.dependency
462
-
if dep.overlay != nil {
463
-
dep = dep.overlay
464
-
}
484
+
dep = dep.overlay
465
485
o.dependencies = append(o.dependencies, &ccDep{
466
486
dependency: dep,
467
487
kind: d.kind,