Skip to content

Commit

Permalink
internal/core/dep: use a new visitor.marked map when recursing
Browse files Browse the repository at this point in the history
Otherwise the recursing visit call will remember all the marked
nodes from the parent's visit state, causing a significant amount
of repeated work when Config.Dynamic is true without changing the result.

This is because visitor.dynamic uses the presence of leaf conjuncts
in the "marked" map to decide whether to visit a vertex,
so if the marked map is kept when recursing, the nested visitor
ends up visiting way too many vertices and repeating work.

The test case added in the previous commit now runs in about 80ms
rather than in about 7s, and the stats are much lower as well.

Fixes #2559.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I77f9a6c4ec4165eb49c685cc5656ebafce5ed9e6
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198566
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
  • Loading branch information
mvdan committed Jul 30, 2024
1 parent 51bf6f2 commit 34739e9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
3 changes: 3 additions & 0 deletions internal/core/dep/dep.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,16 @@ type Dependency struct {
func (d *Dependency) Recurse() {
savedAll := d.visitor.all
savedTop := d.visitor.top
savedMarked := d.visitor.marked
d.visitor.all = d.visitor.recurse
d.visitor.top = true
d.visitor.marked = nil

d.visitor.visitReusingVisitor(d.Node, false)

d.visitor.all = savedAll
d.visitor.top = savedTop
d.visitor.marked = savedMarked
}

// Import returns the import reference or nil if the reference was within
Expand Down
20 changes: 10 additions & 10 deletions tools/flow/testdata/issue2559.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,24 @@ graph TD
}
-- out/run/t1/stats --
Leaks: 0
Freed: 262168
Reused: 262148
Freed: 25792
Reused: 25772
Allocs: 20
Retain: 352

Unifications: 187362
Conjuncts: 902602
Disjuncts: 262432
Unifications: 18522
Conjuncts: 76898
Disjuncts: 26056
-- out/run/stats/totals --
Leaks: 0
Freed: 262168
Reused: 262148
Freed: 25792
Reused: 25772
Allocs: 20
Retain: 352

Unifications: 187362
Conjuncts: 902602
Disjuncts: 262432
Unifications: 18522
Conjuncts: 76898
Disjuncts: 26056
-- out/run/t2 --
graph TD
t0("root.prepare [Terminated]")
Expand Down

0 comments on commit 34739e9

Please sign in to comment.