From 92054a49d29bf3fb31366f84e48c3950942a6c36 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 21 Jan 2021 16:49:25 -0800 Subject: [PATCH] Memoize full names of variables, pools and rules Variables, pools and rules each computed their full names every time they were referenced, which required string concatenations. Since every one is guaranteed to be accessed at least twice, once when the definition is written into the ninja file and once for each reference, precompute the full name. For local variables that can be done during initialization, but for global variables add a pass to PrepareBuildActions to compute the name for each live variable using the final package names. Test: ninja_writer_test.go Change-Id: I2264b05e0409e36651db2fb5d463c16c698d4d5e --- context.go | 17 ++++++++ package_ctx.go | 108 ++++++++++++++++++++++++++++++++++++++++++------- scope.go | 51 ++++++++++++++--------- 3 files changed, 142 insertions(+), 34 deletions(-) diff --git a/context.go b/context.go index ef290f6..63a82b4 100644 --- a/context.go +++ b/context.go @@ -2309,6 +2309,8 @@ func (c *Context) PrepareBuildActions(config interface{}) (deps []string, errs [ deps = append(deps, depsPackages...) + c.memoizeFullNames(c.liveGlobals, pkgNames) + // This will panic if it finds a problem since it's a programming error. c.checkForVariableReferenceCycles(c.liveGlobals.variables, pkgNames) @@ -3173,6 +3175,21 @@ func (c *Context) makeUniquePackageNames( return pkgNames, deps } +// memoizeFullNames stores the full name of each live global variable, rule and pool since each is +// guaranteed to be used at least twice, once in the definition and once for each usage, and many +// are used much more than once. +func (c *Context) memoizeFullNames(liveGlobals *liveTracker, pkgNames map[*packageContext]string) { + for v := range liveGlobals.variables { + v.memoizeFullName(pkgNames) + } + for r := range liveGlobals.rules { + r.memoizeFullName(pkgNames) + } + for p := range liveGlobals.pools { + p.memoizeFullName(pkgNames) + } +} + func (c *Context) checkForVariableReferenceCycles( variables map[Variable]ninjaString, pkgNames map[*packageContext]string) { diff --git a/package_ctx.go b/package_ctx.go index 088239e..af78772 100644 --- a/package_ctx.go +++ b/package_ctx.go @@ -250,9 +250,10 @@ func (p *packageContext) ImportAs(as, pkgPath string) { } type staticVariable struct { - pctx *packageContext - name_ string - value_ string + pctx *packageContext + name_ string + value_ string + fullName_ string } // StaticVariable returns a Variable whose value does not depend on any @@ -271,7 +272,11 @@ func (p *packageContext) StaticVariable(name, value string) Variable { panic(err) } - v := &staticVariable{p, name, value} + v := &staticVariable{ + pctx: p, + name_: name, + value_: value, + } err = p.scope.AddVariable(v) if err != nil { panic(err) @@ -289,9 +294,16 @@ func (v *staticVariable) name() string { } func (v *staticVariable) fullName(pkgNames map[*packageContext]string) string { + if v.fullName_ != "" { + return v.fullName_ + } return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_ } +func (v *staticVariable) memoizeFullName(pkgNames map[*packageContext]string) { + v.fullName_ = v.fullName(pkgNames) +} + func (v *staticVariable) value(interface{}) (ninjaString, error) { ninjaStr, err := parseNinjaString(v.pctx.scope, v.value_) if err != nil { @@ -306,9 +318,10 @@ func (v *staticVariable) String() string { } type variableFunc struct { - pctx *packageContext - name_ string - value_ func(interface{}) (string, error) + pctx *packageContext + name_ string + value_ func(interface{}) (string, error) + fullName_ string } // VariableFunc returns a Variable whose value is determined by a function that @@ -332,7 +345,11 @@ func (p *packageContext) VariableFunc(name string, panic(err) } - v := &variableFunc{p, name, f} + v := &variableFunc{ + pctx: p, + name_: name, + value_: f, + } err = p.scope.AddVariable(v) if err != nil { panic(err) @@ -371,7 +388,11 @@ func (p *packageContext) VariableConfigMethod(name string, return resultStr, nil } - v := &variableFunc{p, name, fun} + v := &variableFunc{ + pctx: p, + name_: name, + value_: fun, + } err = p.scope.AddVariable(v) if err != nil { panic(err) @@ -389,9 +410,16 @@ func (v *variableFunc) name() string { } func (v *variableFunc) fullName(pkgNames map[*packageContext]string) string { + if v.fullName_ != "" { + return v.fullName_ + } return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_ } +func (v *variableFunc) memoizeFullName(pkgNames map[*packageContext]string) { + v.fullName_ = v.fullName(pkgNames) +} + func (v *variableFunc) value(config interface{}) (ninjaString, error) { value, err := v.value_(config) if err != nil { @@ -452,6 +480,10 @@ func (v *argVariable) fullName(pkgNames map[*packageContext]string) string { return v.name_ } +func (v *argVariable) memoizeFullName(pkgNames map[*packageContext]string) { + // Nothing to do, full name is known at initialization. +} + func (v *argVariable) value(config interface{}) (ninjaString, error) { return nil, errVariableIsArg } @@ -461,9 +493,10 @@ func (v *argVariable) String() string { } type staticPool struct { - pctx *packageContext - name_ string - params PoolParams + pctx *packageContext + name_ string + params PoolParams + fullName_ string } // StaticPool returns a Pool whose value does not depend on any configuration @@ -483,7 +516,11 @@ func (p *packageContext) StaticPool(name string, params PoolParams) Pool { panic(err) } - pool := &staticPool{p, name, params} + pool := &staticPool{ + pctx: p, + name_: name, + params: params, + } err = p.scope.AddPool(pool) if err != nil { panic(err) @@ -501,9 +538,16 @@ func (p *staticPool) name() string { } func (p *staticPool) fullName(pkgNames map[*packageContext]string) string { + if p.fullName_ != "" { + return p.fullName_ + } return packageNamespacePrefix(pkgNames[p.pctx]) + p.name_ } +func (p *staticPool) memoizeFullName(pkgNames map[*packageContext]string) { + p.fullName_ = p.fullName(pkgNames) +} + func (p *staticPool) def(config interface{}) (*poolDef, error) { def, err := parsePoolParams(p.pctx.scope, &p.params) if err != nil { @@ -520,6 +564,7 @@ type poolFunc struct { pctx *packageContext name_ string paramsFunc func(interface{}) (PoolParams, error) + fullName_ string } // PoolFunc returns a Pool whose value is determined by a function that takes a @@ -542,7 +587,11 @@ func (p *packageContext) PoolFunc(name string, f func(interface{}) (PoolParams, panic(err) } - pool := &poolFunc{p, name, f} + pool := &poolFunc{ + pctx: p, + name_: name, + paramsFunc: f, + } err = p.scope.AddPool(pool) if err != nil { panic(err) @@ -560,9 +609,16 @@ func (p *poolFunc) name() string { } func (p *poolFunc) fullName(pkgNames map[*packageContext]string) string { + if p.fullName_ != "" { + return p.fullName_ + } return packageNamespacePrefix(pkgNames[p.pctx]) + p.name_ } +func (p *poolFunc) memoizeFullName(pkgNames map[*packageContext]string) { + p.fullName_ = p.fullName(pkgNames) +} + func (p *poolFunc) def(config interface{}) (*poolDef, error) { params, err := p.paramsFunc(config) if err != nil { @@ -595,6 +651,10 @@ func (p *builtinPool) fullName(pkgNames map[*packageContext]string) string { return p.name_ } +func (p *builtinPool) memoizeFullName(pkgNames map[*packageContext]string) { + // Nothing to do, full name is known at initialization. +} + func (p *builtinPool) def(config interface{}) (*poolDef, error) { return nil, errPoolIsBuiltin } @@ -616,6 +676,7 @@ type staticRule struct { params RuleParams argNames map[string]bool scope_ *basicScope + fullName_ string sync.Mutex // protects scope_ during lazy creation } @@ -683,9 +744,16 @@ func (r *staticRule) name() string { } func (r *staticRule) fullName(pkgNames map[*packageContext]string) string { + if r.fullName_ != "" { + return r.fullName_ + } return packageNamespacePrefix(pkgNames[r.pctx]) + r.name_ } +func (r *staticRule) memoizeFullName(pkgNames map[*packageContext]string) { + r.fullName_ = r.fullName(pkgNames) +} + func (r *staticRule) def(interface{}) (*ruleDef, error) { def, err := parseRuleParams(r.scope(), &r.params) if err != nil { @@ -721,6 +789,7 @@ type ruleFunc struct { paramsFunc func(interface{}) (RuleParams, error) argNames map[string]bool scope_ *basicScope + fullName_ string sync.Mutex // protects scope_ during lazy creation } @@ -789,9 +858,16 @@ func (r *ruleFunc) name() string { } func (r *ruleFunc) fullName(pkgNames map[*packageContext]string) string { + if r.fullName_ != "" { + return r.fullName_ + } return packageNamespacePrefix(pkgNames[r.pctx]) + r.name_ } +func (r *ruleFunc) memoizeFullName(pkgNames map[*packageContext]string) { + r.fullName_ = r.fullName(pkgNames) +} + func (r *ruleFunc) def(config interface{}) (*ruleDef, error) { params, err := r.paramsFunc(config) if err != nil { @@ -843,6 +919,10 @@ func (r *builtinRule) fullName(pkgNames map[*packageContext]string) string { return r.name_ } +func (r *builtinRule) memoizeFullName(pkgNames map[*packageContext]string) { + // Nothing to do, full name is known at initialization. +} + func (r *builtinRule) def(config interface{}) (*ruleDef, error) { return nil, errRuleIsBuiltin } diff --git a/scope.go b/scope.go index 0a520d9..3f39eb7 100644 --- a/scope.go +++ b/scope.go @@ -28,6 +28,7 @@ type Variable interface { packageContext() *packageContext name() string // "foo" fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.foo" + memoizeFullName(pkgNames map[*packageContext]string) // precompute fullName if desired value(config interface{}) (ninjaString, error) String() string } @@ -38,6 +39,7 @@ type Pool interface { packageContext() *packageContext name() string // "foo" fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.foo" + memoizeFullName(pkgNames map[*packageContext]string) // precompute fullName if desired def(config interface{}) (*poolDef, error) String() string } @@ -48,6 +50,7 @@ type Rule interface { packageContext() *packageContext name() string // "foo" fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.foo" + memoizeFullName(pkgNames map[*packageContext]string) // precompute fullName if desired def(config interface{}) (*ruleDef, error) scope() *basicScope isArg(argName string) bool @@ -294,9 +297,9 @@ func (s *localScope) AddLocalVariable(name, value string) (*localVariable, } v := &localVariable{ - namePrefix: s.namePrefix, - name_: name, - value_: ninjaValue, + fullName_: s.namePrefix + name, + name_: name, + value_: ninjaValue, } err = s.scope.AddVariable(v) @@ -333,11 +336,11 @@ func (s *localScope) AddLocalRule(name string, params *RuleParams, } r := &localRule{ - namePrefix: s.namePrefix, - name_: name, - def_: def, - argNames: argNamesSet, - scope_: ruleScope, + fullName_: s.namePrefix + name, + name_: name, + def_: def, + argNames: argNamesSet, + scope_: ruleScope, } err = s.scope.AddRule(r) @@ -349,9 +352,9 @@ func (s *localScope) AddLocalRule(name string, params *RuleParams, } type localVariable struct { - namePrefix string - name_ string - value_ ninjaString + fullName_ string + name_ string + value_ ninjaString } func (l *localVariable) packageContext() *packageContext { @@ -363,7 +366,11 @@ func (l *localVariable) name() string { } func (l *localVariable) fullName(pkgNames map[*packageContext]string) string { - return l.namePrefix + l.name_ + return l.fullName_ +} + +func (l *localVariable) memoizeFullName(pkgNames map[*packageContext]string) { + // Nothing to do, full name is known at initialization. } func (l *localVariable) value(interface{}) (ninjaString, error) { @@ -371,15 +378,15 @@ func (l *localVariable) value(interface{}) (ninjaString, error) { } func (l *localVariable) String() string { - return ":" + l.namePrefix + l.name_ + return ":" + l.fullName_ } type localRule struct { - namePrefix string - name_ string - def_ *ruleDef - argNames map[string]bool - scope_ *basicScope + fullName_ string + name_ string + def_ *ruleDef + argNames map[string]bool + scope_ *basicScope } func (l *localRule) packageContext() *packageContext { @@ -391,7 +398,11 @@ func (l *localRule) name() string { } func (l *localRule) fullName(pkgNames map[*packageContext]string) string { - return l.namePrefix + l.name_ + return l.fullName_ +} + +func (l *localRule) memoizeFullName(pkgNames map[*packageContext]string) { + // Nothing to do, full name is known at initialization. } func (l *localRule) def(interface{}) (*ruleDef, error) { @@ -407,5 +418,5 @@ func (r *localRule) isArg(argName string) bool { } func (r *localRule) String() string { - return ":" + r.namePrefix + r.name_ + return ":" + r.fullName_ }