Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Commit

Permalink
internal/core/eval: minor performance improvements for disjunctions
Browse files Browse the repository at this point in the history 
This also remove some duplicates.

This makes Issue #758 a bit more pallatable, although
the core issue remains.

Change-Id: I0d7c60c96cc7ebf65380c677626215f586d7a7e9
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8801
Reviewed-by: CUE cueckoo <[email protected]>
Reviewed-by: Marcel van Lohuizen <[email protected]>
  • Loading branch information
@mpvl
mpvl committed Feb 17, 2021
1 parent 77e6b51 commit 2c86835
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 39 deletions.
16 changes: 0 additions & 16 deletions cue/testdata/disjunctions/embed.txtar
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,6 @@ nested: {
}, (#struct){
c: (string){ string }
e: (string){ string }
}, (#struct){
d: (string){ string }
}, (#struct){
d: (string){ string }
}, (#struct){
d: (string){ string }
e: (string){ string }
Expand All @@ -183,12 +179,6 @@ nested: {
a: (string){ string }
c: (string){ string }
e: (string){ string }
}, (#struct){
a: (string){ string }
d: (string){ string }
}, (#struct){
a: (string){ string }
d: (string){ string }
}, (#struct){
a: (string){ string }
d: (string){ string }
Expand All @@ -212,12 +202,6 @@ nested: {
b: (string){ string }
c: (string){ string }
e: (string){ string }
}, (#struct){
b: (string){ string }
d: (string){ string }
}, (#struct){
b: (string){ string }
d: (string){ string }
}, (#struct){
b: (string){ string }
d: (string){ string }
Expand Down
2 changes: 1 addition & 1 deletion cue/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,7 @@ func (v Value) Equals(other Value) bool {
if v.v == nil || other.v == nil {
return false
}
return adt.Equal(v.ctx().opCtx, v.v, other.v, false)
return adt.Equal(v.ctx().opCtx, v.v, other.v, adt.CheckStructural)
}

// Format prints a debug version of a value.
Expand Down
13 changes: 9 additions & 4 deletions internal/core/adt/disjunct.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (n *nodeContext) expandDisjuncts(
state VertexStatus,
parent *nodeContext,
parentMode defaultMode, // default mode of this disjunct
recursive bool) {
recursive, last bool) {

n.ctx.stats.DisjunctCount++

Expand Down Expand Up @@ -210,6 +210,7 @@ func (n *nodeContext) expandDisjuncts(
n.disjuncts = n.buffer[:0]
n.buffer = a[:0]

last := i+1 == len(n.disjunctions)
skipNonMonotonicChecks := i+1 < len(n.disjunctions)
if skipNonMonotonicChecks {
n.ctx.inDisjunct++
Expand All @@ -228,7 +229,7 @@ func (n *nodeContext) expandDisjuncts(

newMode := mode(d.hasDefaults, v.Default)

cn.expandDisjuncts(state, n, newMode, true)
cn.expandDisjuncts(state, n, newMode, true, last)
}

case d.value != nil:
Expand All @@ -241,7 +242,7 @@ func (n *nodeContext) expandDisjuncts(

newMode := mode(d.hasDefaults, i < d.value.NumDefaults)

cn.expandDisjuncts(state, n, newMode, true)
cn.expandDisjuncts(state, n, newMode, true, last)
}
}
}
Expand Down Expand Up @@ -365,7 +366,11 @@ func (n *nodeContext) expandDisjuncts(
outer:
for _, d := range n.disjuncts {
for k, v := range p.disjuncts {
if Equal(n.ctx, &v.result, &d.result, true) {
flags := CheckStructural
if last {
flags |= IgnoreOptional
}
if Equal(n.ctx, &v.result, &d.result, flags) {
m := maybeDefault
for _, u := range d.usedDefault {
m = combineDefault(m, u.nestedMode)
Expand Down
50 changes: 34 additions & 16 deletions internal/core/adt/equality.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,30 @@

package adt

func Equal(ctx *OpContext, v, w Value, optional bool) bool {
type Flag uint16

const (
// IgnoreOptional allows optional information to be ignored. This only
// applies when CheckStructural is given.
IgnoreOptional Flag = 1 << iota

// CheckStructural indicates that closedness information should be
// considered for equality. Equal may return false even when values are
// equal.
CheckStructural Flag = 1 << iota
)

func Equal(ctx *OpContext, v, w Value, flags Flag) bool {
if x, ok := v.(*Vertex); ok {
return equalVertex(ctx, x, w, optional)
return equalVertex(ctx, x, w, flags)
}
if y, ok := w.(*Vertex); ok {
return equalVertex(ctx, y, v, optional)
return equalVertex(ctx, y, v, flags)
}
return equalTerminal(ctx, v, w, optional)
return equalTerminal(ctx, v, w, flags)
}

func equalVertex(ctx *OpContext, x *Vertex, v Value, opt bool) bool {
func equalVertex(ctx *OpContext, x *Vertex, v Value, flags Flag) bool {
y, ok := v.(*Vertex)
if !ok {
return false
Expand All @@ -47,15 +60,15 @@ func equalVertex(ctx *OpContext, x *Vertex, v Value, opt bool) bool {
if x.IsClosed(ctx) != y.IsClosed(ctx) {
return false
}
if opt && !equalClosed(ctx, x, y) {
if flags != 0 && !equalClosed(ctx, x, y, flags) {
return false
}

loop1:
for _, a := range x.Arcs {
for _, b := range y.Arcs {
if a.Label == b.Label {
if !Equal(ctx, a, b, opt) {
if !Equal(ctx, a, b, flags) {
return false
}
continue loop1
Expand All @@ -81,7 +94,7 @@ loop1:
return true // both are struct or list.
}

return equalTerminal(ctx, v, w, opt)
return equalTerminal(ctx, v, w, flags)
}

// equalClosed tests if x and y have the same set of close information.
Expand All @@ -93,16 +106,21 @@ loop1:
//
// For all these refinements it would be necessary to have well-working
// structure sharing so as to not repeatedly recompute optional arcs.
func equalClosed(ctx *OpContext, x, y *Vertex) bool {
return verifyStructs(x, y) && verifyStructs(y, x)
func equalClosed(ctx *OpContext, x, y *Vertex, flags Flag) bool {
return verifyStructs(x, y, flags) && verifyStructs(y, x, flags)
}

func verifyStructs(x, y *Vertex) bool {
func verifyStructs(x, y *Vertex, flags Flag) bool {
outer:
for _, s := range x.Structs {
if s.closeInfo == nil || s.closeInfo.span|DefinitionSpan == 0 {
if (flags&IgnoreOptional != 0) && !s.StructLit.HasOptional() {
continue
}
if s.closeInfo == nil || s.closeInfo.span&DefinitionSpan == 0 {
if !s.StructLit.HasOptional() {
continue
}
}
for _, t := range y.Structs {
if s.StructLit == t.StructLit {
continue outer
Expand All @@ -113,7 +131,7 @@ outer:
return true
}

func equalTerminal(ctx *OpContext, v, w Value, opt bool) bool {
func equalTerminal(ctx *OpContext, v, w Value, flags Flag) bool {
if v == w {
return true
}
Expand All @@ -130,7 +148,7 @@ func equalTerminal(ctx *OpContext, v, w Value, opt bool) bool {

case *BoundValue:
if y, ok := w.(*BoundValue); ok {
return x.Op == y.Op && Equal(ctx, x.Value, y.Value, opt)
return x.Op == y.Op && Equal(ctx, x.Value, y.Value, flags)
}

case *BasicType:
Expand All @@ -145,7 +163,7 @@ func equalTerminal(ctx *OpContext, v, w Value, opt bool) bool {
}
// always ordered the same
for i, xe := range x.Values {
if !Equal(ctx, xe, y.Values[i], opt) {
if !Equal(ctx, xe, y.Values[i], flags) {
return false
}
}
Expand All @@ -159,7 +177,7 @@ func equalTerminal(ctx *OpContext, v, w Value, opt bool) bool {
return false
}
for i, xe := range x.Values {
if !Equal(ctx, xe, y.Values[i], opt) {
if !Equal(ctx, xe, y.Values[i], flags) {
return false
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/eval.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func (c *OpContext) Unify(v *Vertex, state VertexStatus) {
if len(n.disjunctions) > 0 && disState != Finalized {
disState = Finalized
}
n.expandDisjuncts(disState, n, maybeDefault, false)
n.expandDisjuncts(disState, n, maybeDefault, false, true)

n.finalizeDisjuncts()

Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/simplify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func SimplifyValidator(ctx *OpContext, v, w Validator) Validator {
return nil
}
for i, a := range x.Args {
if !Equal(ctx, a, y.Args[i], false) {
if !Equal(ctx, a, y.Args[i], CheckStructural) {
return nil
}
}
Expand Down

0 comments on commit 2c86835

Please sign in to comment.