Skip to content

Commit

Permalink
internal/core/adt: cache evaluation results for dynamic fields
Browse files Browse the repository at this point in the history
Also cache string index lookup. This is fast, but requires
an RWLock, which can add up.

Issue #572

Change-Id: I782cf52f50aa022b069b98de9d0496775c40e30e
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8562
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Feb 5, 2021
1 parent e6a2fb0 commit 2ca6c5e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 26 deletions.
27 changes: 21 additions & 6 deletions internal/core/adt/closed.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,18 +310,27 @@ func Accept(ctx *OpContext, n *Vertex, f Feature) (found, required bool) {
ctx.generation++
ctx.todo = nil

// TODO(perf): more aggressively determine whether a struct is open or
// closed: open structs do not have to be checked, yet they can particularly
// be the ones with performance isssues, for instanced as a result of
// embedded for comprehensions.
for _, s := range n.Structs {
if !s.useForAccept() {
continue
}
markCounts(ctx, s.CloseInfo)
}

str := ""
if f.IsString() {
str = f.StringValue(ctx)
}

for _, s := range n.Structs {
if !s.useForAccept() {
continue
}
if verifyArc(ctx, s, f) {
if verifyArc(ctx, s, f, str) {
// Beware: don't add to below expression: this relies on the
// side effects of markUp.
ok := markUp(ctx, s.closeInfo, 0)
Expand Down Expand Up @@ -435,7 +444,7 @@ func getScratch(ctx *OpContext, s *closeInfo) *closeStats {
return x
}

func verifyArc(ctx *OpContext, s *StructInfo, f Feature) bool {
func verifyArc(ctx *OpContext, s *StructInfo, f Feature, label string) bool {
isRegular := f.IsRegular()

o := s.StructLit
Expand All @@ -455,10 +464,16 @@ func verifyArc(ctx *OpContext, s *StructInfo, f Feature) bool {
return false
}

for _, b := range o.Dynamic {
m := dynamicMatcher{b.Key}
if m.Match(ctx, env, f) {
return true
if f.IsString() {
for _, b := range o.Dynamic {
v := env.evalCached(ctx, b.Key)
s, ok := v.(*String)
if !ok {
continue
}
if label == s.Str {
return true
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/core/adt/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ type StructInfo struct {
Embedding bool
}

// TODO(perf): this could be much more aggressive for eliminating structs that
// are immaterial for closing.
func (s *StructInfo) useForAccept() bool {
if c := s.closeInfo; c != nil {
return !c.noCheck
Expand Down
20 changes: 0 additions & 20 deletions internal/core/adt/optional.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,26 +125,6 @@ func (m typeMatcher) Match(c *OpContext, env *Environment, f Feature) bool {
return false
}

type dynamicMatcher struct {
expr Expr
}

func (m dynamicMatcher) Match(c *OpContext, env *Environment, f Feature) bool {
if !f.IsRegular() || !f.IsString() {
return false
}
v, ok := c.Evaluate(env, m.expr)
if !ok {
return false
}
s, ok := v.(*String)
if !ok {
return false
}
label := f.StringValue(c)
return label == s.Str
}

type patternMatcher Conjunct

func (m patternMatcher) Match(c *OpContext, env *Environment, f Feature) bool {
Expand Down

0 comments on commit 2ca6c5e

Please sign in to comment.