From 2ce594e44637be0d5ad4d4f939911272fe4680aa Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 29 Jan 2020 12:58:03 -0800 Subject: [PATCH] Make ninjaString an interface There are 8935901 *ninjaString objects generated in an AOSP aosp_blueline-userdebug build, and 7865180 of those are a literal string with no ninja variables. Each of those *ninjaString objects takes a minimum of 48 bytes for 2 slices, plus 8 bytes for the pointer to the ninjaString. For the literal string case, one of those slices has a single element, (costing another 16 bytes for the backing array), and the other slice is empty, for a total of 72 bytes. Replace *ninjaString with a ninjaString interface. This increases the size of the reference from 8 bytes to 16 bytes, but using a type alias of a string for the literal string implementation uses only 16 bytes, saving 40 bytes per literal string or 314 MB. Test: ninja_strings_test Change-Id: Ic5fe16ed1f2a244fe6a8ccdf762919634d825cbe --- context.go | 18 ++++++------- live_tracker.go | 10 ++++---- ninja_defs.go | 32 +++++++++++------------ ninja_strings.go | 60 +++++++++++++++++++++++++++++++++---------- ninja_strings_test.go | 56 +++++++++++++++++++++++++--------------- package_ctx.go | 6 ++--- scope.go | 6 ++--- 7 files changed, 118 insertions(+), 70 deletions(-) diff --git a/context.go b/context.go index 9a1e6cf..1ad1588 100644 --- a/context.go +++ b/context.go @@ -96,15 +96,15 @@ type Context struct { // set during PrepareBuildActions pkgNames map[*packageContext]string liveGlobals *liveTracker - globalVariables map[Variable]*ninjaString + globalVariables map[Variable]ninjaString globalPools map[Pool]*poolDef globalRules map[Rule]*ruleDef // set during PrepareBuildActions - ninjaBuildDir *ninjaString // The builddir special Ninja variable - requiredNinjaMajor int // For the ninja_required_version variable - requiredNinjaMinor int // For the ninja_required_version variable - requiredNinjaMicro int // For the ninja_required_version variable + ninjaBuildDir ninjaString // The builddir special Ninja variable + requiredNinjaMajor int // For the ninja_required_version variable + requiredNinjaMinor int // For the ninja_required_version variable + requiredNinjaMicro int // For the ninja_required_version variable subninjas []string @@ -2788,7 +2788,7 @@ func (c *Context) requireNinjaVersion(major, minor, micro int) { } } -func (c *Context) setNinjaBuildDir(value *ninjaString) { +func (c *Context) setNinjaBuildDir(value ninjaString) { if c.ninjaBuildDir == nil { c.ninjaBuildDir = value } @@ -2854,7 +2854,7 @@ func (c *Context) makeUniquePackageNames( } func (c *Context) checkForVariableReferenceCycles( - variables map[Variable]*ninjaString, pkgNames map[*packageContext]string) { + variables map[Variable]ninjaString, pkgNames map[*packageContext]string) { visited := make(map[Variable]bool) // variables that were already checked checking := make(map[Variable]bool) // variables actively being checked @@ -2867,7 +2867,7 @@ func (c *Context) checkForVariableReferenceCycles( defer delete(checking, v) value := variables[v] - for _, dep := range value.variables { + for _, dep := range value.Variables() { if checking[dep] { // This is a cycle. return []Variable{dep, v} @@ -3352,7 +3352,7 @@ func (c *Context) writeGlobalVariables(nw *ninjaWriter) error { // First visit variables on which this variable depends. value := c.globalVariables[v] - for _, dep := range value.variables { + for _, dep := range value.Variables() { if !visited[dep] { err := walk(dep) if err != nil { diff --git a/live_tracker.go b/live_tracker.go index 5e13a87..40e1930 100644 --- a/live_tracker.go +++ b/live_tracker.go @@ -24,7 +24,7 @@ type liveTracker struct { sync.Mutex config interface{} // Used to evaluate variable, rule, and pool values. - variables map[Variable]*ninjaString + variables map[Variable]ninjaString pools map[Pool]*poolDef rules map[Rule]*ruleDef } @@ -32,7 +32,7 @@ type liveTracker struct { func newLiveTracker(config interface{}) *liveTracker { return &liveTracker{ config: config, - variables: make(map[Variable]*ninjaString), + variables: make(map[Variable]ninjaString), pools: make(map[Pool]*poolDef), rules: make(map[Rule]*ruleDef), } @@ -170,7 +170,7 @@ func (l *liveTracker) addVariable(v Variable) error { return nil } -func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error { +func (l *liveTracker) addNinjaStringListDeps(list []ninjaString) error { for _, str := range list { err := l.addNinjaStringDeps(str) if err != nil { @@ -180,8 +180,8 @@ func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error { return nil } -func (l *liveTracker) addNinjaStringDeps(str *ninjaString) error { - for _, v := range str.variables { +func (l *liveTracker) addNinjaStringDeps(str ninjaString) error { + for _, v := range str.Variables() { err := l.addVariable(v) if err != nil { return err diff --git a/ninja_defs.go b/ninja_defs.go index 61846fe..c5d0e4b 100644 --- a/ninja_defs.go +++ b/ninja_defs.go @@ -128,11 +128,11 @@ func (p *poolDef) WriteTo(nw *ninjaWriter, name string) error { // A ruleDef describes a rule definition. It does not include the name of the // rule. type ruleDef struct { - CommandDeps []*ninjaString - CommandOrderOnly []*ninjaString + CommandDeps []ninjaString + CommandOrderOnly []ninjaString Comment string Pool Pool - Variables map[string]*ninjaString + Variables map[string]ninjaString } func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, @@ -141,7 +141,7 @@ func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, r := &ruleDef{ Comment: params.Comment, Pool: params.Pool, - Variables: make(map[string]*ninjaString), + Variables: make(map[string]ninjaString), } if params.Command == "" { @@ -252,13 +252,13 @@ type buildDef struct { Comment string Rule Rule RuleDef *ruleDef - Outputs []*ninjaString - ImplicitOutputs []*ninjaString - Inputs []*ninjaString - Implicits []*ninjaString - OrderOnly []*ninjaString - Args map[Variable]*ninjaString - Variables map[string]*ninjaString + Outputs []ninjaString + ImplicitOutputs []ninjaString + Inputs []ninjaString + Implicits []ninjaString + OrderOnly []ninjaString + Args map[Variable]ninjaString + Variables map[string]ninjaString Optional bool } @@ -273,9 +273,9 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef, Rule: rule, } - setVariable := func(name string, value *ninjaString) { + setVariable := func(name string, value ninjaString) { if b.Variables == nil { - b.Variables = make(map[string]*ninjaString) + b.Variables = make(map[string]ninjaString) } b.Variables[name] = value } @@ -339,7 +339,7 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef, argNameScope := rule.scope() if len(params.Args) > 0 { - b.Args = make(map[Variable]*ninjaString) + b.Args = make(map[Variable]ninjaString) for name, value := range params.Args { if !rule.isArg(name) { return nil, fmt.Errorf("unknown argument %q", name) @@ -419,7 +419,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string) return nw.BlankLine() } -func valueList(list []*ninjaString, pkgNames map[*packageContext]string, +func valueList(list []ninjaString, pkgNames map[*packageContext]string, escaper *strings.Replacer) []string { result := make([]string, len(list)) @@ -429,7 +429,7 @@ func valueList(list []*ninjaString, pkgNames map[*packageContext]string, return result } -func writeVariables(nw *ninjaWriter, variables map[string]*ninjaString, +func writeVariables(nw *ninjaWriter, variables map[string]ninjaString, pkgNames map[*packageContext]string) error { var keys []string for k := range variables { diff --git a/ninja_strings.go b/ninja_strings.go index 5b8767d..190cae9 100644 --- a/ninja_strings.go +++ b/ninja_strings.go @@ -34,21 +34,28 @@ var ( ":", "$:") ) -type ninjaString struct { +type ninjaString interface { + Value(pkgNames map[*packageContext]string) string + ValueWithEscaper(pkgNames map[*packageContext]string, escaper *strings.Replacer) string + Eval(variables map[Variable]ninjaString) (string, error) + Variables() []Variable +} + +type varNinjaString struct { strings []string variables []Variable } +type literalNinjaString string + type scope interface { LookupVariable(name string) (Variable, error) IsRuleVisible(rule Rule) bool IsPoolVisible(pool Pool) bool } -func simpleNinjaString(str string) *ninjaString { - return &ninjaString{ - strings: []string{str}, - } +func simpleNinjaString(str string) ninjaString { + return literalNinjaString(str) } type parseState struct { @@ -57,7 +64,7 @@ type parseState struct { pendingStr string stringStart int varStart int - result *ninjaString + result *varNinjaString } func (ps *parseState) pushVariable(v Variable) { @@ -84,10 +91,16 @@ type stateFunc func(*parseState, int, rune) (stateFunc, error) // parseNinjaString parses an unescaped ninja string (i.e. all $ // occurrences are expected to be variables or $$) and returns a list of the // variable names that the string references. -func parseNinjaString(scope scope, str string) (*ninjaString, error) { +func parseNinjaString(scope scope, str string) (ninjaString, error) { // naively pre-allocate slices by counting $ signs n := strings.Count(str, "$") - result := &ninjaString{ + if n == 0 { + if strings.HasPrefix(str, " ") { + str = "$" + str + } + return literalNinjaString(str), nil + } + result := &varNinjaString{ strings: make([]string, 0, n+1), variables: make([]Variable, 0, n), } @@ -253,13 +266,13 @@ func parseBracketsState(state *parseState, i int, r rune) (stateFunc, error) { } } -func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString, +func parseNinjaStrings(scope scope, strs []string) ([]ninjaString, error) { if len(strs) == 0 { return nil, nil } - result := make([]*ninjaString, len(strs)) + result := make([]ninjaString, len(strs)) for i, str := range strs { ninjaStr, err := parseNinjaString(scope, str) if err != nil { @@ -270,11 +283,11 @@ func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString, return result, nil } -func (n *ninjaString) Value(pkgNames map[*packageContext]string) string { +func (n varNinjaString) Value(pkgNames map[*packageContext]string) string { return n.ValueWithEscaper(pkgNames, defaultEscaper) } -func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string, +func (n varNinjaString) ValueWithEscaper(pkgNames map[*packageContext]string, escaper *strings.Replacer) string { if len(n.strings) == 1 { @@ -293,7 +306,7 @@ func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string, return str.String() } -func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error) { +func (n varNinjaString) Eval(variables map[Variable]ninjaString) (string, error) { str := n.strings[0] for i, v := range n.variables { variable, ok := variables[v] @@ -309,6 +322,27 @@ func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error) return str, nil } +func (n varNinjaString) Variables() []Variable { + return n.variables +} + +func (l literalNinjaString) Value(pkgNames map[*packageContext]string) string { + return l.ValueWithEscaper(pkgNames, defaultEscaper) +} + +func (l literalNinjaString) ValueWithEscaper(pkgNames map[*packageContext]string, + escaper *strings.Replacer) string { + return escaper.Replace(string(l)) +} + +func (l literalNinjaString) Eval(variables map[Variable]ninjaString) (string, error) { + return string(l), nil +} + +func (l literalNinjaString) Variables() []Variable { + return nil +} + func validateNinjaName(name string) error { for i, r := range name { valid := (r >= 'a' && r <= 'z') || diff --git a/ninja_strings_test.go b/ninja_strings_test.go index 0e0de64..c1e05f7 100644 --- a/ninja_strings_test.go +++ b/ninja_strings_test.go @@ -22,10 +22,11 @@ import ( ) var ninjaParseTestCases = []struct { - input string - vars []string - strs []string - err string + input string + vars []string + strs []string + literal bool + err string }{ { input: "abc def $ghi jkl", @@ -56,6 +57,7 @@ var ninjaParseTestCases = []struct { input: "foo $$ bar", vars: nil, strs: []string{"foo $$ bar"}, + // this is technically a literal, but not recognized as such due to the $$ }, { input: "$foo${bar}", @@ -68,16 +70,22 @@ var ninjaParseTestCases = []struct { strs: []string{"", "$$"}, }, { - input: "foo bar", - vars: nil, - strs: []string{"foo bar"}, + input: "foo bar", + vars: nil, + strs: []string{"foo bar"}, + literal: true, }, { - input: " foo ", - vars: nil, - strs: []string{"$ foo "}, + input: " foo ", + vars: nil, + strs: []string{"$ foo "}, + literal: true, }, { + input: " $foo ", + vars: []string{"foo"}, + strs: []string{"$ ", " "}, + }, { input: "foo $ bar", err: "invalid character after '$' at byte offset 5", }, @@ -114,19 +122,25 @@ func TestParseNinjaString(t *testing.T) { expectedVars = append(expectedVars, v) } + var expected ninjaString + if len(testCase.strs) > 0 { + if testCase.literal { + expected = literalNinjaString(testCase.strs[0]) + } else { + expected = &varNinjaString{ + strings: testCase.strs, + variables: expectedVars, + } + } + } + output, err := parseNinjaString(scope, testCase.input) if err == nil { - if !reflect.DeepEqual(output.variables, expectedVars) { - t.Errorf("incorrect variable list:") + if !reflect.DeepEqual(output, expected) { + t.Errorf("incorrect ninja string:") t.Errorf(" input: %q", testCase.input) - t.Errorf(" expected: %#v", expectedVars) - t.Errorf(" got: %#v", output.variables) - } - if !reflect.DeepEqual(output.strings, testCase.strs) { - t.Errorf("incorrect string list:") - t.Errorf(" input: %q", testCase.input) - t.Errorf(" expected: %#v", testCase.strs) - t.Errorf(" got: %#v", output.strings) + t.Errorf(" expected: %#v", expected) + t.Errorf(" got: %#v", output) } } var errStr string @@ -156,7 +170,7 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) { } expect := []Variable{ImpVar} - if !reflect.DeepEqual(output.variables, expect) { + if !reflect.DeepEqual(output.(*varNinjaString).variables, expect) { t.Errorf("incorrect output:") t.Errorf(" input: %q", input) t.Errorf(" expected: %#v", expect) diff --git a/package_ctx.go b/package_ctx.go index c55152a..088239e 100644 --- a/package_ctx.go +++ b/package_ctx.go @@ -292,7 +292,7 @@ func (v *staticVariable) fullName(pkgNames map[*packageContext]string) string { return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_ } -func (v *staticVariable) value(interface{}) (*ninjaString, error) { +func (v *staticVariable) value(interface{}) (ninjaString, error) { ninjaStr, err := parseNinjaString(v.pctx.scope, v.value_) if err != nil { err = fmt.Errorf("error parsing variable %s value: %s", v, err) @@ -392,7 +392,7 @@ func (v *variableFunc) fullName(pkgNames map[*packageContext]string) string { return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_ } -func (v *variableFunc) value(config interface{}) (*ninjaString, error) { +func (v *variableFunc) value(config interface{}) (ninjaString, error) { value, err := v.value_(config) if err != nil { return nil, err @@ -452,7 +452,7 @@ func (v *argVariable) fullName(pkgNames map[*packageContext]string) string { return v.name_ } -func (v *argVariable) value(config interface{}) (*ninjaString, error) { +func (v *argVariable) value(config interface{}) (ninjaString, error) { return nil, errVariableIsArg } diff --git a/scope.go b/scope.go index 84db0cf..0a520d9 100644 --- a/scope.go +++ b/scope.go @@ -28,7 +28,7 @@ type Variable interface { packageContext() *packageContext name() string // "foo" fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.foo" - value(config interface{}) (*ninjaString, error) + value(config interface{}) (ninjaString, error) String() string } @@ -351,7 +351,7 @@ func (s *localScope) AddLocalRule(name string, params *RuleParams, type localVariable struct { namePrefix string name_ string - value_ *ninjaString + value_ ninjaString } func (l *localVariable) packageContext() *packageContext { @@ -366,7 +366,7 @@ func (l *localVariable) fullName(pkgNames map[*packageContext]string) string { return l.namePrefix + l.name_ } -func (l *localVariable) value(interface{}) (*ninjaString, error) { +func (l *localVariable) value(interface{}) (ninjaString, error) { return l.value_, nil }