diff --git a/linter/internal/types/build_graph.go b/linter/internal/types/build_graph.go index f58ecabcd..7ad9c099f 100644 --- a/linter/internal/types/build_graph.go +++ b/linter/internal/types/build_graph.go @@ -192,6 +192,9 @@ func calcTP(node ast.Node, varAt map[ast.Node]*common.Variable, g *typeGraph) ty case *ast.Error: return concreteTP(voidTypeDesc()) case *ast.Index: + // TODO(sbarzowski) It appears we create a separate index node each time. + // Perhaps we could de-duplicate (cache) index placeholders to save processing + // later? switch index := node.Index.(type) { case *ast.LiteralString: return tpIndex(knownObjectIndex(g.getExprPlaceholder(node.Target), index.Value)) diff --git a/linter/internal/types/check.go b/linter/internal/types/check.go index bbb3316fc..21cbbb092 100644 --- a/linter/internal/types/check.go +++ b/linter/internal/types/check.go @@ -140,6 +140,8 @@ func Check(mainNode ast.Node, roots map[string]ast.Node, vars map[string]map[ast g.prepareTypes(mainNode, et) // TODO(sbarzowski) Useful for debugging – expose it in CLI? + // spew.Dump(g.timeStats) + // spew.Dump(g.counters) // t := et[node.node] // fmt.Fprintf(os.Stderr, "%v\n", types.Describe(&t)) diff --git a/linter/internal/types/desc.go b/linter/internal/types/desc.go index 6c411351c..3a422e128 100644 --- a/linter/internal/types/desc.go +++ b/linter/internal/types/desc.go @@ -20,6 +20,17 @@ type arrayDesc struct { elementContains [][]placeholderID } +func (a *arrayDesc) copy() *arrayDesc { + elementContains := make([][]placeholderID, len(a.elementContains)) + for i, placeholders := range a.elementContains { + elementContains[i] = append([]placeholderID{}, placeholders...) + } + return &arrayDesc{ + furtherContain: append([]placeholderID{}, a.furtherContain...), + elementContains: elementContains, + } +} + func (a *arrayDesc) widen(other *arrayDesc) { if other == nil { return @@ -49,6 +60,18 @@ type objectDesc struct { allFieldsKnown bool } +func (o *objectDesc) copy() *objectDesc { + fieldContains := make(map[string][]placeholderID) + for k, v := range o.fieldContains { + fieldContains[k] = append([]placeholderID(nil), v...) + } + return &objectDesc{ + unknownContain: append([]placeholderID(nil), o.unknownContain...), + fieldContains: fieldContains, + allFieldsKnown: o.allFieldsKnown, + } +} + func (o *objectDesc) widen(other *objectDesc) { if other == nil { return @@ -77,6 +100,7 @@ func (o *objectDesc) normalize() { type functionDesc struct { resultContains []placeholderID + // Read-only // TODO(sbarzowski) instead of keeping "real" parameters here, // maybe keep only what we care about in the linter desc // (names and required-or-not). @@ -85,6 +109,15 @@ type functionDesc struct { minArity, maxArity int } +func (f *functionDesc) copy() *functionDesc { + return &functionDesc{ + resultContains: append([]placeholderID{}, f.resultContains...), + params: f.params, // this is read-only, so we can just copy the pointer + minArity: f.minArity, + maxArity: f.maxArity, + } +} + func sameParameters(a, b []ast.Parameter) bool { if a == nil || b == nil { return false @@ -255,7 +288,44 @@ func Describe(t *TypeDesc) string { return strings.Join(parts, " or ") } +// An intuitive notion of the size of type description. For performance tuning. +func (t *TypeDesc) size() int64 { + res := 0 + if t.Bool { + res += 1 + } + if t.Number { + res += 1 + } + if t.String { + res += 1 + } + if t.Null { + res += 1 + } + if t.Function() { + res += len(t.FunctionDesc.resultContains) + res += len(t.FunctionDesc.params) + } + if t.Array() { + res += len(t.ArrayDesc.furtherContain) + for _, elem := range t.ArrayDesc.elementContains { + res += len(elem) + } + } + if t.Object() { + res += len(t.ObjectDesc.unknownContain) + for _, elem := range t.ObjectDesc.fieldContains { + res += len(elem) + } + } + return int64(res) +} + func (t *TypeDesc) widen(b *TypeDesc) { + if t == b { + panic("BUG: widening the type with itself") + } t.Bool = t.Bool || b.Bool t.Number = t.Number || b.Number t.String = t.String || b.String @@ -264,22 +334,24 @@ func (t *TypeDesc) widen(b *TypeDesc) { if t.FunctionDesc != nil { t.FunctionDesc.widen(b.FunctionDesc) } else if t.FunctionDesc == nil && b.FunctionDesc != nil { - copy := *b.FunctionDesc - t.FunctionDesc = © + // copy := *b.FunctionDesc + // t.FunctionDesc = © + t.FunctionDesc = b.FunctionDesc.copy() } if t.ObjectDesc != nil { t.ObjectDesc.widen(b.ObjectDesc) } else if t.ObjectDesc == nil && b.ObjectDesc != nil { - copy := *b.ObjectDesc - t.ObjectDesc = © + // TODO(sbarzowski) Maybe we want copy on write? + // So that it actually aliases underneath (with a bool marker) + // which is resolved when widened. + t.ObjectDesc = b.ObjectDesc.copy() } if t.ArrayDesc != nil { t.ArrayDesc.widen(b.ArrayDesc) } else if t.ArrayDesc == nil && b.ArrayDesc != nil { - copy := *b.ArrayDesc - t.ArrayDesc = © + t.ArrayDesc = b.ArrayDesc.copy() } } diff --git a/linter/internal/types/graph.go b/linter/internal/types/graph.go index 23b057368..87117cdbd 100644 --- a/linter/internal/types/graph.go +++ b/linter/internal/types/graph.go @@ -1,6 +1,10 @@ package types import ( + "fmt" + "io" + "time" + "github.com/google/go-jsonnet/ast" ) @@ -20,6 +24,32 @@ type typeGraph struct { // TODO(sbarzowski) what was this for? importFunc ImportFunc + + // For performance tuning + timeStats timeStats + + counters counters +} + +// Subject to change at any point. For fine-tuning only. +type timeStats struct { + simplifyRef time.Duration + separateElemTypes time.Duration + topoOrder time.Duration + findTypes time.Duration +} + +type counters struct { + sccCount int + containWidenCount int + builtinWidenConcreteCount int + builtinWidenContainedCount int + preNormalizationSumSize int64 + postNormalizationSumSize int64 +} + +func (g *typeGraph) debugStats(w io.Writer) { + fmt.Fprintf(w, "placeholders no: %d\n", len(g._placeholders)) } func (g *typeGraph) placeholder(id placeholderID) *typePlaceholder { @@ -150,10 +180,21 @@ func newTypeGraph(importFunc ImportFunc) *typeGraph { // prepareTypes produces a final type for each expression in the graph. // No further operations on the graph are valid after this is called. func (g *typeGraph) prepareTypes(node ast.Node, typeOf exprTypes) { + tStart := time.Now() g.simplifyReferences() + tSimplify := time.Now() g.separateElementTypes() + tSeparate := time.Now() g.makeTopoOrder() + tTopo := time.Now() g.findTypes() + tTypes := time.Now() + g.timeStats = timeStats{ + simplifyRef: tSimplify.Sub(tStart), + separateElemTypes: tSeparate.Sub(tSimplify), + topoOrder: tTopo.Sub(tSeparate), + findTypes: tTypes.Sub(tTopo), + } for e, p := range g.exprPlaceholder { typeOf[e] = g.upperBound[p] } diff --git a/linter/internal/types/process_graph.go b/linter/internal/types/process_graph.go index 1a5fa6fa0..dcc10bee7 100644 --- a/linter/internal/types/process_graph.go +++ b/linter/internal/types/process_graph.go @@ -1,6 +1,8 @@ package types -import "fmt" +import ( + "fmt" +) type stronglyConnectedComponentID int @@ -288,6 +290,7 @@ func (g *typeGraph) findTypes() { sccID++ } } + g.counters.sccCount = len(stronglyConnectedComponents) for i := len(stronglyConnectedComponents) - 1; i >= 0; i-- { scc := stronglyConnectedComponents[i] @@ -301,11 +304,18 @@ func (g *typeGraph) resolveTypesInSCC(scc []placeholderID) { common := voidTypeDesc() for _, p := range scc { + // Add types from contained SCCs. These are fully calculated + // because we are going in topo order for _, contained := range g.placeholder(p).contains { + // TODO(sbarzowski) Is it possible that we are adding multiple times from the same placeholder? + // Each `p` is normalized, but different placeholders in scc may still contain the same thing. if g.sccOf[contained] != sccID { common.widen(&g.upperBound[contained]) + g.counters.containWidenCount += 1 } } + + // Handle builtins (e.g) builtinOp := g.placeholder(p).builtinOp if builtinOp != nil { concreteArgs := []*TypeDesc{} @@ -318,9 +328,11 @@ func (g *typeGraph) resolveTypesInSCC(scc []placeholderID) { } res := builtinOp.f(concreteArgs, builtinOp.args) common.widen(&res.concrete) + g.counters.builtinWidenConcreteCount += 1 for _, contained := range res.contained { if g.sccOf[contained] != sccID { common.widen(&g.upperBound[contained]) + g.counters.builtinWidenContainedCount += 1 } } } @@ -333,8 +345,12 @@ func (g *typeGraph) resolveTypesInSCC(scc []placeholderID) { } } + g.counters.preNormalizationSumSize += common.size() + common.normalize() + g.counters.postNormalizationSumSize += common.size() + for _, p := range scc { g.upperBound[p] = common } diff --git a/linter/internal/types/stdlib.go b/linter/internal/types/stdlib.go index 9b050db50..47e8739e6 100644 --- a/linter/internal/types/stdlib.go +++ b/linter/internal/types/stdlib.go @@ -9,7 +9,7 @@ func prepareStdlib(g *typeGraph) { arrayOfNumber := anyArrayType stringOrArray := anyType stringOrNumber := anyType - jsonType := anyType // It actually cannot functions anywhere + jsonType := anyType // It actually cannot have functions anywhere required := func(name string) ast.Parameter { return ast.Parameter{Name: ast.Identifier(name)}