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
This commit is contained in:
Colin Cross 2020-01-29 12:58:03 -08:00
parent 38e095ae8a
commit 2ce594e446
7 changed files with 118 additions and 70 deletions

View file

@ -96,15 +96,15 @@ type Context struct {
// set during PrepareBuildActions // set during PrepareBuildActions
pkgNames map[*packageContext]string pkgNames map[*packageContext]string
liveGlobals *liveTracker liveGlobals *liveTracker
globalVariables map[Variable]*ninjaString globalVariables map[Variable]ninjaString
globalPools map[Pool]*poolDef globalPools map[Pool]*poolDef
globalRules map[Rule]*ruleDef globalRules map[Rule]*ruleDef
// set during PrepareBuildActions // set during PrepareBuildActions
ninjaBuildDir *ninjaString // The builddir special Ninja variable ninjaBuildDir ninjaString // The builddir special Ninja variable
requiredNinjaMajor int // For the ninja_required_version variable requiredNinjaMajor int // For the ninja_required_version variable
requiredNinjaMinor int // For the ninja_required_version variable requiredNinjaMinor int // For the ninja_required_version variable
requiredNinjaMicro int // For the ninja_required_version variable requiredNinjaMicro int // For the ninja_required_version variable
subninjas []string 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 { if c.ninjaBuildDir == nil {
c.ninjaBuildDir = value c.ninjaBuildDir = value
} }
@ -2854,7 +2854,7 @@ func (c *Context) makeUniquePackageNames(
} }
func (c *Context) checkForVariableReferenceCycles( 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 visited := make(map[Variable]bool) // variables that were already checked
checking := make(map[Variable]bool) // variables actively being checked checking := make(map[Variable]bool) // variables actively being checked
@ -2867,7 +2867,7 @@ func (c *Context) checkForVariableReferenceCycles(
defer delete(checking, v) defer delete(checking, v)
value := variables[v] value := variables[v]
for _, dep := range value.variables { for _, dep := range value.Variables() {
if checking[dep] { if checking[dep] {
// This is a cycle. // This is a cycle.
return []Variable{dep, v} return []Variable{dep, v}
@ -3352,7 +3352,7 @@ func (c *Context) writeGlobalVariables(nw *ninjaWriter) error {
// First visit variables on which this variable depends. // First visit variables on which this variable depends.
value := c.globalVariables[v] value := c.globalVariables[v]
for _, dep := range value.variables { for _, dep := range value.Variables() {
if !visited[dep] { if !visited[dep] {
err := walk(dep) err := walk(dep)
if err != nil { if err != nil {

View file

@ -24,7 +24,7 @@ type liveTracker struct {
sync.Mutex sync.Mutex
config interface{} // Used to evaluate variable, rule, and pool values. config interface{} // Used to evaluate variable, rule, and pool values.
variables map[Variable]*ninjaString variables map[Variable]ninjaString
pools map[Pool]*poolDef pools map[Pool]*poolDef
rules map[Rule]*ruleDef rules map[Rule]*ruleDef
} }
@ -32,7 +32,7 @@ type liveTracker struct {
func newLiveTracker(config interface{}) *liveTracker { func newLiveTracker(config interface{}) *liveTracker {
return &liveTracker{ return &liveTracker{
config: config, config: config,
variables: make(map[Variable]*ninjaString), variables: make(map[Variable]ninjaString),
pools: make(map[Pool]*poolDef), pools: make(map[Pool]*poolDef),
rules: make(map[Rule]*ruleDef), rules: make(map[Rule]*ruleDef),
} }
@ -170,7 +170,7 @@ func (l *liveTracker) addVariable(v Variable) error {
return nil return nil
} }
func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error { func (l *liveTracker) addNinjaStringListDeps(list []ninjaString) error {
for _, str := range list { for _, str := range list {
err := l.addNinjaStringDeps(str) err := l.addNinjaStringDeps(str)
if err != nil { if err != nil {
@ -180,8 +180,8 @@ func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error {
return nil return nil
} }
func (l *liveTracker) addNinjaStringDeps(str *ninjaString) error { func (l *liveTracker) addNinjaStringDeps(str ninjaString) error {
for _, v := range str.variables { for _, v := range str.Variables() {
err := l.addVariable(v) err := l.addVariable(v)
if err != nil { if err != nil {
return err return err

View file

@ -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 // A ruleDef describes a rule definition. It does not include the name of the
// rule. // rule.
type ruleDef struct { type ruleDef struct {
CommandDeps []*ninjaString CommandDeps []ninjaString
CommandOrderOnly []*ninjaString CommandOrderOnly []ninjaString
Comment string Comment string
Pool Pool Pool Pool
Variables map[string]*ninjaString Variables map[string]ninjaString
} }
func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, func parseRuleParams(scope scope, params *RuleParams) (*ruleDef,
@ -141,7 +141,7 @@ func parseRuleParams(scope scope, params *RuleParams) (*ruleDef,
r := &ruleDef{ r := &ruleDef{
Comment: params.Comment, Comment: params.Comment,
Pool: params.Pool, Pool: params.Pool,
Variables: make(map[string]*ninjaString), Variables: make(map[string]ninjaString),
} }
if params.Command == "" { if params.Command == "" {
@ -252,13 +252,13 @@ type buildDef struct {
Comment string Comment string
Rule Rule Rule Rule
RuleDef *ruleDef RuleDef *ruleDef
Outputs []*ninjaString Outputs []ninjaString
ImplicitOutputs []*ninjaString ImplicitOutputs []ninjaString
Inputs []*ninjaString Inputs []ninjaString
Implicits []*ninjaString Implicits []ninjaString
OrderOnly []*ninjaString OrderOnly []ninjaString
Args map[Variable]*ninjaString Args map[Variable]ninjaString
Variables map[string]*ninjaString Variables map[string]ninjaString
Optional bool Optional bool
} }
@ -273,9 +273,9 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef,
Rule: rule, Rule: rule,
} }
setVariable := func(name string, value *ninjaString) { setVariable := func(name string, value ninjaString) {
if b.Variables == nil { if b.Variables == nil {
b.Variables = make(map[string]*ninjaString) b.Variables = make(map[string]ninjaString)
} }
b.Variables[name] = value b.Variables[name] = value
} }
@ -339,7 +339,7 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef,
argNameScope := rule.scope() argNameScope := rule.scope()
if len(params.Args) > 0 { if len(params.Args) > 0 {
b.Args = make(map[Variable]*ninjaString) b.Args = make(map[Variable]ninjaString)
for name, value := range params.Args { for name, value := range params.Args {
if !rule.isArg(name) { if !rule.isArg(name) {
return nil, fmt.Errorf("unknown argument %q", 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() return nw.BlankLine()
} }
func valueList(list []*ninjaString, pkgNames map[*packageContext]string, func valueList(list []ninjaString, pkgNames map[*packageContext]string,
escaper *strings.Replacer) []string { escaper *strings.Replacer) []string {
result := make([]string, len(list)) result := make([]string, len(list))
@ -429,7 +429,7 @@ func valueList(list []*ninjaString, pkgNames map[*packageContext]string,
return result return result
} }
func writeVariables(nw *ninjaWriter, variables map[string]*ninjaString, func writeVariables(nw *ninjaWriter, variables map[string]ninjaString,
pkgNames map[*packageContext]string) error { pkgNames map[*packageContext]string) error {
var keys []string var keys []string
for k := range variables { for k := range variables {

View file

@ -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 strings []string
variables []Variable variables []Variable
} }
type literalNinjaString string
type scope interface { type scope interface {
LookupVariable(name string) (Variable, error) LookupVariable(name string) (Variable, error)
IsRuleVisible(rule Rule) bool IsRuleVisible(rule Rule) bool
IsPoolVisible(pool Pool) bool IsPoolVisible(pool Pool) bool
} }
func simpleNinjaString(str string) *ninjaString { func simpleNinjaString(str string) ninjaString {
return &ninjaString{ return literalNinjaString(str)
strings: []string{str},
}
} }
type parseState struct { type parseState struct {
@ -57,7 +64,7 @@ type parseState struct {
pendingStr string pendingStr string
stringStart int stringStart int
varStart int varStart int
result *ninjaString result *varNinjaString
} }
func (ps *parseState) pushVariable(v Variable) { 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 $<something> // parseNinjaString parses an unescaped ninja string (i.e. all $<something>
// occurrences are expected to be variables or $$) and returns a list of the // occurrences are expected to be variables or $$) and returns a list of the
// variable names that the string references. // 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 // naively pre-allocate slices by counting $ signs
n := strings.Count(str, "$") 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), strings: make([]string, 0, n+1),
variables: make([]Variable, 0, n), 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) { error) {
if len(strs) == 0 { if len(strs) == 0 {
return nil, nil return nil, nil
} }
result := make([]*ninjaString, len(strs)) result := make([]ninjaString, len(strs))
for i, str := range strs { for i, str := range strs {
ninjaStr, err := parseNinjaString(scope, str) ninjaStr, err := parseNinjaString(scope, str)
if err != nil { if err != nil {
@ -270,11 +283,11 @@ func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
return result, nil 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) 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 { escaper *strings.Replacer) string {
if len(n.strings) == 1 { if len(n.strings) == 1 {
@ -293,7 +306,7 @@ func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
return str.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] str := n.strings[0]
for i, v := range n.variables { for i, v := range n.variables {
variable, ok := variables[v] variable, ok := variables[v]
@ -309,6 +322,27 @@ func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error)
return str, nil 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 { func validateNinjaName(name string) error {
for i, r := range name { for i, r := range name {
valid := (r >= 'a' && r <= 'z') || valid := (r >= 'a' && r <= 'z') ||

View file

@ -22,10 +22,11 @@ import (
) )
var ninjaParseTestCases = []struct { var ninjaParseTestCases = []struct {
input string input string
vars []string vars []string
strs []string strs []string
err string literal bool
err string
}{ }{
{ {
input: "abc def $ghi jkl", input: "abc def $ghi jkl",
@ -56,6 +57,7 @@ var ninjaParseTestCases = []struct {
input: "foo $$ bar", input: "foo $$ bar",
vars: nil, vars: nil,
strs: []string{"foo $$ bar"}, strs: []string{"foo $$ bar"},
// this is technically a literal, but not recognized as such due to the $$
}, },
{ {
input: "$foo${bar}", input: "$foo${bar}",
@ -68,16 +70,22 @@ var ninjaParseTestCases = []struct {
strs: []string{"", "$$"}, strs: []string{"", "$$"},
}, },
{ {
input: "foo bar", input: "foo bar",
vars: nil, vars: nil,
strs: []string{"foo bar"}, strs: []string{"foo bar"},
literal: true,
}, },
{ {
input: " foo ", input: " foo ",
vars: nil, vars: nil,
strs: []string{"$ foo "}, strs: []string{"$ foo "},
literal: true,
}, },
{ {
input: " $foo ",
vars: []string{"foo"},
strs: []string{"$ ", " "},
}, {
input: "foo $ bar", input: "foo $ bar",
err: "invalid character after '$' at byte offset 5", err: "invalid character after '$' at byte offset 5",
}, },
@ -114,19 +122,25 @@ func TestParseNinjaString(t *testing.T) {
expectedVars = append(expectedVars, v) 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) output, err := parseNinjaString(scope, testCase.input)
if err == nil { if err == nil {
if !reflect.DeepEqual(output.variables, expectedVars) { if !reflect.DeepEqual(output, expected) {
t.Errorf("incorrect variable list:") t.Errorf("incorrect ninja string:")
t.Errorf(" input: %q", testCase.input) t.Errorf(" input: %q", testCase.input)
t.Errorf(" expected: %#v", expectedVars) t.Errorf(" expected: %#v", expected)
t.Errorf(" got: %#v", output.variables) t.Errorf(" got: %#v", output)
}
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)
} }
} }
var errStr string var errStr string
@ -156,7 +170,7 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) {
} }
expect := []Variable{ImpVar} expect := []Variable{ImpVar}
if !reflect.DeepEqual(output.variables, expect) { if !reflect.DeepEqual(output.(*varNinjaString).variables, expect) {
t.Errorf("incorrect output:") t.Errorf("incorrect output:")
t.Errorf(" input: %q", input) t.Errorf(" input: %q", input)
t.Errorf(" expected: %#v", expect) t.Errorf(" expected: %#v", expect)

View file

@ -292,7 +292,7 @@ func (v *staticVariable) fullName(pkgNames map[*packageContext]string) string {
return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_ 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_) ninjaStr, err := parseNinjaString(v.pctx.scope, v.value_)
if err != nil { if err != nil {
err = fmt.Errorf("error parsing variable %s value: %s", v, err) 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_ 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) value, err := v.value_(config)
if err != nil { if err != nil {
return nil, err return nil, err
@ -452,7 +452,7 @@ func (v *argVariable) fullName(pkgNames map[*packageContext]string) string {
return v.name_ return v.name_
} }
func (v *argVariable) value(config interface{}) (*ninjaString, error) { func (v *argVariable) value(config interface{}) (ninjaString, error) {
return nil, errVariableIsArg return nil, errVariableIsArg
} }

View file

@ -28,7 +28,7 @@ type Variable interface {
packageContext() *packageContext packageContext() *packageContext
name() string // "foo" name() string // "foo"
fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.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 String() string
} }
@ -351,7 +351,7 @@ func (s *localScope) AddLocalRule(name string, params *RuleParams,
type localVariable struct { type localVariable struct {
namePrefix string namePrefix string
name_ string name_ string
value_ *ninjaString value_ ninjaString
} }
func (l *localVariable) packageContext() *packageContext { func (l *localVariable) packageContext() *packageContext {
@ -366,7 +366,7 @@ func (l *localVariable) fullName(pkgNames map[*packageContext]string) string {
return l.namePrefix + l.name_ return l.namePrefix + l.name_
} }
func (l *localVariable) value(interface{}) (*ninjaString, error) { func (l *localVariable) value(interface{}) (ninjaString, error) {
return l.value_, nil return l.value_, nil
} }