Skip to content

Commit

Permalink
internal/core/adt: improve performance for repeated calls to Unify/Fill
Browse files Browse the repository at this point in the history
This is done by skipping SpawnRef for computed vertices.

Repeatedly applying unification caused a new closedInfo to
be inserted at the node's position, resulting in a growing
list to process when calling Accept (called by Unify, which
is called by Fill). This caused processing time to become
quadratic for that node.

There are more gains to be made, but this should be a big
help already.

Issue #899

Change-Id: I96ec77bb4bd09b69b9da023e0fb2b8ed5c297080
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9462
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Apr 21, 2021
1 parent 9e34a41 commit b8ce660
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 5 deletions.
3 changes: 0 additions & 3 deletions cmd/cue/cmd/testdata/script/vet_embed.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ e: 2

-- expect-foo --
field c not allowed:
./foo.yaml:1:2
./foo.yaml:2:2
./schema.cue:1:1
./schema.cue:3:7
Expand All @@ -42,12 +41,10 @@ field d not allowed:
./schema.cue:1:1
./schema.cue:3:7
./schema.cue:7:1
./stream.yaml:1:2
./stream.yaml:2:2
-- expect-stream --
field d not allowed:
./schema.cue:1:1
./schema.cue:3:7
./schema.cue:7:1
./stream.yaml:1:2
./stream.yaml:2:2
1 change: 0 additions & 1 deletion cmd/cue/cmd/testdata/script/vet_expr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ cmp stderr expect-stderr
-- expect-stderr --
translations.hello.lang: incomplete value string
field skip not allowed:
./data.yaml:16:1
./data.yaml:20:1
./vet.cue:1:1
./vet.cue:1:8
Expand Down
10 changes: 9 additions & 1 deletion internal/core/adt/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -1359,7 +1359,15 @@ func (n *nodeContext) addVertexConjuncts(env *Environment, closeInfo CloseInfo,
defer func() { arc.SelfCount-- }()
}

closeInfo = closeInfo.SpawnRef(arc, IsDef(x), x)
// Performance: the following if check filters cases that are not strictly
// necessary for correct functioning. Not updating the closeInfo may cause
// some position information to be lost for top-level positions of merges
// resulting form APIs. These tend to be fairly uninteresting.
// At the same time, this optimization may prevent considerable slowdown
// in case an API does many calls to Unify.
if !inline || arc.IsClosedStruct() || arc.IsClosedList() {
closeInfo = closeInfo.SpawnRef(arc, IsDef(x), x)
}

if arc.status == 0 && !inline {
// This is a rare condition, but can happen in certain
Expand Down
16 changes: 16 additions & 0 deletions internal/core/adt/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (

"github.com/rogpeppe/go-internal/txtar"

"cuelang.org/go/cue"
"cuelang.org/go/cue/cuecontext"
"cuelang.org/go/internal/core/adt"
"cuelang.org/go/internal/core/debug"
"cuelang.org/go/internal/core/eval"
Expand Down Expand Up @@ -135,3 +137,17 @@ module: "example.com"

t.Log(ctx.Stats())
}

func BenchmarkUnifyAPI(b *testing.B) {
for i := 0; i < b.N; i++ {
b.StopTimer()
ctx := cuecontext.New()
v := ctx.CompileString("")
for j := 0; j < 500; j++ {
if j == 400 {
b.StartTimer()
}
v = v.FillPath(cue.ParsePath(fmt.Sprintf("i_%d", i)), i)
}
}
}

0 comments on commit b8ce660

Please sign in to comment.