Skip to content

Commit

Permalink
internal/core/adt: weave OpContext to more operations
Browse files Browse the repository at this point in the history
This solves a pain during debugging, as may be evident from the
removed code. It also allows for better stats collection and per-op
debugging settings.

Note that this makes the signature of incDependent identical to
that of decDependent.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I9426b74eec7963222fdec41716605e17a5429f2a
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193604
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mpvl committed Apr 23, 2024
1 parent 2cc7b65 commit 7ada2dd
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 75 deletions.
6 changes: 3 additions & 3 deletions internal/core/adt/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,15 @@ func equalDeref(a, b *Vertex) bool {

// rootCloseContext creates a closeContext for this Vertex or returns the
// existing one.
func (v *Vertex) rootCloseContext() *closeContext {
func (v *Vertex) rootCloseContext(ctx *OpContext) *closeContext {
if v.cc == nil {
v.cc = &closeContext{
group: (*ConjunctGroup)(&v.Conjuncts),
parent: nil,
src: v,
parentConjuncts: v,
}
v.cc.incDependent(ROOT, nil) // matched in REF(decrement:nodeDone)
v.cc.incDependent(ctx, ROOT, nil) // matched in REF(decrement:nodeDone)
}
return v.cc
}
Expand Down Expand Up @@ -996,7 +996,7 @@ func (v *Vertex) MatchAndInsert(ctx *OpContext, arc *Vertex) {
for _, pc := range pcs.Pairs {
if matchPattern(ctx, pc.Pattern, arc.Label) {
for _, c := range pc.Constraint.Conjuncts {
root := arc.rootCloseContext()
root := arc.rootCloseContext(ctx)
root.insertConjunct(ctx, root, c, c.CloseInfo, ArcMember, true, false)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/comprehension.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func (n *nodeContext) insertComprehension(
n.assertInitialized()
_, c.arcCC = n.insertArcCC(f.Label, ArcPending, conjunct, conjunct.CloseInfo, false)
c.cc = ci.cc
ci.cc.incDependent(COMP, c.arcCC)
ci.cc.incDependent(n.ctx, COMP, c.arcCC)
} else {
n.insertFieldUnchecked(f.Label, ArcPending, conjunct)
}
Expand Down
26 changes: 13 additions & 13 deletions internal/core/adt/conjunct.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
// conjunct represents an embedding or definition, we need to create a
// new closeContext to represent this.
if id.cc == nil {
id.cc = n.node.rootCloseContext()
id.cc = n.node.rootCloseContext(n.ctx)
}
if id.cc == cc {
panic("inconsistent state: same closeContext")
Expand All @@ -82,10 +82,10 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
t |= closeEmbed
}
if t != 0 {
id, _ = id.spawnCloseContext(t)
id, _ = id.spawnCloseContext(n.ctx, t)
}
if !id.cc.done {
id.cc.incDependent(DEFER, nil)
id.cc.incDependent(n.ctx, DEFER, nil)
defer id.cc.decDependent(n.ctx, DEFER, nil)
}

Expand Down Expand Up @@ -274,7 +274,7 @@ loop2:
// for files.
// TODO: set this as a flag in StructLit so as to not have to
// do the somewhat dangerous cast here.
ci, _ = ci.spawnCloseContext(0)
ci, _ = ci.spawnCloseContext(n.ctx, 0)
}
// Note: adding a count is not needed here, as there will be an
// embed spawn below.
Expand All @@ -300,8 +300,8 @@ loop2:
n.insertArc(x.Label, ArcMember, lc, ci, true)

case *Comprehension:
ci, cc := ci.spawnCloseContext(closeEmbed)
cc.incDependent(DEFER, nil)
ci, cc := ci.spawnCloseContext(n.ctx, closeEmbed)
cc.incDependent(n.ctx, DEFER, nil)
defer cc.decDependent(n.ctx, DEFER, nil)
n.insertComprehension(childEnv, x, ci)
hasEmbed = true
Expand All @@ -325,10 +325,10 @@ loop2:

case Expr:
// TODO: perhaps special case scalar Values to avoid creating embedding.
ci, cc := ci.spawnCloseContext(closeEmbed)
ci, cc := ci.spawnCloseContext(n.ctx, closeEmbed)

// TODO: do we need to increment here?
cc.incDependent(DEFER, nil) // decrement deferred below
cc.incDependent(n.ctx, DEFER, nil) // decrement deferred below
defer cc.decDependent(n.ctx, DEFER, nil)

ec := MakeConjunct(childEnv, x, ci)
Expand Down Expand Up @@ -394,10 +394,10 @@ func (n *nodeContext) scheduleVertexConjuncts(c Conjunct, arc *Vertex, closeInfo

if IsDef(c.Expr()) {
// TODO: or should we always insert the wrapper (for errors)?
ci, dc := closeInfo.spawnCloseContext(closeDef)
ci, dc := closeInfo.spawnCloseContext(n.ctx, closeDef)
closeInfo = ci

dc.incDependent(DEFER, nil) // decrement deferred below
dc.incDependent(n.ctx, DEFER, nil) // decrement deferred below
defer dc.decDependent(n.ctx, DEFER, nil)
}

Expand Down Expand Up @@ -446,7 +446,7 @@ func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) []receiver {

// TODO: dedup: only add if t does not already exist. First check if this
// is even possible by adding a panic.
root := n.node.rootCloseContext()
root := n.node.rootCloseContext(n.ctx)
if root.isDecremented {
return old
}
Expand All @@ -459,7 +459,7 @@ func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) []receiver {

cc := c.cc

if root.linkNotify(v, cc, c.CycleInfo) {
if root.linkNotify(n.ctx, v, cc, c.CycleInfo) {
n.notify = append(n.notify, receiver{v, cc})
}

Expand All @@ -485,7 +485,7 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
id.IsClosed = true
if ctx.isDevVersion() {
var cc *closeContext
id, cc = id.spawnCloseContext(0)
id, cc = id.spawnCloseContext(n.ctx, 0)
cc.isClosedOnce = true
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/core/adt/cycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ outer:
// evaluator.
v := Conjunct{env, x, ci}
if n.ctx.isDevVersion() {
n.node.cc.incDependent(DEFER, nil)
n.node.cc.incDependent(n.ctx, DEFER, nil)
}
n.cyclicConjuncts = append(n.cyclicConjuncts, cyclicConjunct{v, arc})
return ci, true
Expand Down Expand Up @@ -565,7 +565,7 @@ func (n *nodeContext) updateCyclicStatus(c CloseInfo) {
for _, c := range n.cyclicConjuncts {
if n.ctx.isDevVersion() {
ci := c.c.CloseInfo
ci.cc = n.node.rootCloseContext()
ci.cc = n.node.rootCloseContext(n.ctx)
n.scheduleVertexConjuncts(c.c, c.arc, ci)
n.node.cc.decDependent(n.ctx, DEFER, nil)
} else {
Expand Down
24 changes: 5 additions & 19 deletions internal/core/adt/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ type ccDep struct {
taskID int
}

func (c *closeContext) addDependent(kind depKind, dependant *closeContext) *ccDep {
func (c *closeContext) addDependent(ctx *OpContext, kind depKind, dependant *closeContext) *ccDep {
if !DebugDeps {
return nil
}
Expand All @@ -249,17 +249,7 @@ func (c *closeContext) addDependent(kind depKind, dependant *closeContext) *ccDe
}

if Verbosity > 1 {
var state *nodeContext
if c.src != nil && c.src.state != nil {
state = c.src.state
} else if dependant != nil && dependant.src != nil && dependant.src.state != nil {
state = dependant.src.state
}
if state != nil {
state.Logf("INC(%s, %d) %v; %p (parent: %p) <= %p\n", kind, c.conjunctCount, c.Label(), c, c.parent, dependant)
} else {
log.Printf("INC(%s) %v %p parent: %p %d\n", kind, c.Label(), c, c.parent, c.conjunctCount)
}
ctx.Logf(ctx.vertex, "INC(%s) %v %p parent: %p %d\n", kind, c.Label(), c, c.parent, c.conjunctCount)
}

dep := &ccDep{kind: kind, dependency: dependant}
Expand All @@ -269,7 +259,7 @@ func (c *closeContext) addDependent(kind depKind, dependant *closeContext) *ccDe
}

// matchDecrement checks that this decrement matches a previous increment.
func (c *closeContext) matchDecrement(v *Vertex, kind depKind, dependant *closeContext) {
func (c *closeContext) matchDecrement(ctx *OpContext, v *Vertex, kind depKind, dependant *closeContext) {
if !DebugDeps {
return
}
Expand All @@ -279,11 +269,7 @@ func (c *closeContext) matchDecrement(v *Vertex, kind depKind, dependant *closeC
}

if Verbosity > 1 {
if v.state != nil {
v.state.Logf("DEC(%s) %v %p %d\n", kind, c.Label(), c, c.conjunctCount)
} else {
log.Printf("DEC(%s) %v %p %d\n", kind, c.Label(), c, c.conjunctCount)
}
ctx.Logf(ctx.vertex, "DEC(%s) %v %p %d\n", kind, c.Label(), c, c.conjunctCount)
}

for _, d := range c.dependencies {
Expand Down Expand Up @@ -420,7 +406,7 @@ func CreateMermaidGraph(ctx *OpContext, v *Vertex, all bool) (graph string, hasE
//
// Each closeContext has the following info: ptr(cc); cc.count
func (m *mermaidContext) vertex(v *Vertex) *mermaidVertex {
root := v.rootCloseContext()
root := v.rootCloseContext(m.ctx)

vc := m.roots[root]
if vc != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/disjunct2.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (n *nodeContext) scheduleDisjunction(d envDisjunct) {
// as the result will either be an error, a single disjunct, in which
// case mergeVertex will override the original value, or multiple disjuncts,
// in which case the original is set to the disjunct itself.
ccHole.incDisjunct(DISJUNCT)
ccHole.incDisjunct(n.ctx, DISJUNCT)

n.disjunctions = append(n.disjunctions, d)

Expand Down
8 changes: 4 additions & 4 deletions internal/core/adt/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewFieldTester(r Runtime) *FieldTester {
return &FieldTester{
OpContext: ctx,
n: n,
cc: v.rootCloseContext(),
cc: v.rootCloseContext(ctx),
Root: v,
}
}
Expand All @@ -64,7 +64,7 @@ type declaration func(cc *closeContext)

// Run simulates a CUE evaluation of the given declarations.
func (x *FieldTester) Run(sub ...declaration) {
x.cc.incDependent(TEST, nil)
x.cc.incDependent(x.n.ctx, TEST, nil)
for i, s := range sub {
// We want to have i around for debugging purposes. Use i to avoid
// compiler error.
Expand All @@ -91,9 +91,9 @@ func (x *FieldTester) Def(sub ...declaration) declaration {
func (x *FieldTester) spawn(t closeNodeType, sub ...declaration) declaration {
return func(cc *closeContext) {
ci := CloseInfo{cc: cc}
ci, dc := ci.spawnCloseContext(t)
ci, dc := ci.spawnCloseContext(x.n.ctx, t)

dc.incDependent(TEST, nil)
dc.incDependent(x.n.ctx, TEST, nil)
for _, sfn := range sub {
sfn(dc)
}
Expand Down

0 comments on commit 7ada2dd

Please sign in to comment.