tequilaOS/platform_build_blueprint
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Optimize NinjaEscapeList to avoid allocating an output slice

Browse source 
NinjaEscapeList is called on every input or output of a rule, and
most of the time does not escape anything.  Optimize it by returning
the input slice when nothing was escaped.  This avoids 1.336 GB of
allocations in my AOSP aosp_cf_x86_64_phone-userdebug build.

Test: TestNinjaEscapeList
Change-Id: I33b9e7b77b33d10401d1ec3546caa6794c567b16
This commit is contained in:
Colin Cross 2023-10-31 16:16:21 -07:00
parent 95bec3331c
commit 42b2e906ef
2 changed files with 139 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
proptools/escape.go
View file

@ -14,26 +14,38 @@
package proptools
import "strings"
import (
"strings"
"unsafe"
)
// NinjaEscapeList takes a slice of strings that may contain characters that are meaningful to ninja
// ($), and escapes each string so they will be passed to bash. It is not necessary on input,
// output, or dependency names, those are handled by ModuleContext.Build. It is generally required
// on strings from properties in Blueprint files that are used as Args to ModuleContext.Build. A
// new slice containing the escaped strings is returned.
// on strings from properties in Blueprint files that are used as Args to ModuleContext.Build. If
// escaping modified any of the strings then a new slice containing the escaped strings is returned,
// otherwise the original slice is returned.
func NinjaEscapeList(slice []string) []string {
slice = append([]string(nil), slice...)
sliceCopied := false
for i, s := range slice {
slice[i] = NinjaEscape(s)
escaped := NinjaEscape(s)
if unsafe.StringData(s) != unsafe.StringData(escaped) {
if !sliceCopied {
// If this was the first string that was modified by escaping then make a copy of the
// input slice to use as the output slice.
slice = append([]string(nil), slice...)
sliceCopied = true
}
slice[i] = escaped
}
}
return slice
}
// NinjaEscapeList takes a string that may contain characters that are meaningful to ninja
// NinjaEscape takes a string that may contain characters that are meaningful to ninja
// ($), and escapes it so it will be passed to bash. It is not necessary on input,
// output, or dependency names, those are handled by ModuleContext.Build. It is generally required
// on strings from properties in Blueprint files that are used as Args to ModuleContext.Build. A
// new slice containing the escaped strings is returned.
// on strings from properties in Blueprint files that are used as Args to ModuleContext.Build.
func NinjaEscape(s string) string {
return ninjaEscaper.Replace(s)
}
@ -43,23 +55,39 @@ var ninjaEscaper = strings.NewReplacer(
// ShellEscapeList takes a slice of strings that may contain characters that are meaningful to bash and
// escapes them if necessary by wrapping them in single quotes, and replacing internal single quotes with
// '\'' (one single quote to end the quoting, a shell-escaped single quote to insert a real single
// quote, and then a single quote to restarting quoting. A new slice containing the escaped strings
// is returned.
// one single quote to end the quoting, a shell-escaped single quote to insert a real single
// quote, and then a single quote to restarting quoting. If escaping modified any of the strings then a
// new slice containing the escaped strings is returned, otherwise the original slice is returned.
func ShellEscapeList(slice []string) []string {
slice = append([]string(nil), slice...)
sliceCopied := false
for i, s := range slice {
slice[i] = ShellEscape(s)
escaped := ShellEscape(s)
if unsafe.StringData(s) != unsafe.StringData(escaped) {
if !sliceCopied {
// If this was the first string that was modified by escaping then make a copy of the
// input slice to use as the output slice.
slice = append([]string(nil), slice...)
sliceCopied = true
}
slice[i] = escaped
}
}
return slice
}
func ShellEscapeListIncludingSpaces(slice []string) []string {
slice = append([]string(nil), slice...)
sliceCopied := false
for i, s := range slice {
slice[i] = ShellEscapeIncludingSpaces(s)
escaped := ShellEscapeIncludingSpaces(s)
if unsafe.StringData(s) != unsafe.StringData(escaped) {
if !sliceCopied {
// If this was the first string that was modified by escaping then make a copy of the
// input slice to use as the output slice.
slice = append([]string(nil), slice...)
sliceCopied = true
}
slice[i] = escaped
}
}
return slice
}
@ -84,7 +112,7 @@ func shellUnsafeChar(r rune) bool {
// ShellEscape takes string that may contain characters that are meaningful to bash and
// escapes it if necessary by wrapping it in single quotes, and replacing internal single quotes with
// '\'' (one single quote to end the quoting, a shell-escaped single quote to insert a real single
// one single quote to end the quoting, a shell-escaped single quote to insert a real single
// quote, and then a single quote to restarting quoting.
func ShellEscape(s string) string {
shellUnsafeCharNotSpace := func(r rune) bool {

93
proptools/escape_test.go
View file

@ -16,7 +16,9 @@ package proptools
import (
"os/exec"
"reflect"
"testing"
"unsafe"
)
type escapeTestCase struct {
@ -167,3 +169,94 @@ func TestExternalShellEscapeIncludingSpaces(t *testing.T) {
}
}
}
func TestNinjaEscapeList(t *testing.T) {
type testCase struct {
name string
in []string
ninjaEscaped []string
shellEscaped []string
ninjaAndShellEscaped []string
sameSlice bool
}
testCases := []testCase{
{
name: "empty",
in: []string{},
sameSlice: true,
},
{
name: "nil",
in: nil,
sameSlice: true,
},
{
name: "no escaping",
in: []string{"abc", "def", "ghi"},
sameSlice: true,
},
{
name: "escape first",
in: []string{`$\abc`, "def", "ghi"},
ninjaEscaped: []string{`$$\abc`, "def", "ghi"},
shellEscaped: []string{`'$\abc'`, "def", "ghi"},
ninjaAndShellEscaped: []string{`'$$\abc'`, "def", "ghi"},
},
{
name: "escape middle",
in: []string{"abc", `$\def`, "ghi"},
ninjaEscaped: []string{"abc", `$$\def`, "ghi"},
shellEscaped: []string{"abc", `'$\def'`, "ghi"},
ninjaAndShellEscaped: []string{"abc", `'$$\def'`, "ghi"},
},
{
name: "escape last",
in: []string{"abc", "def", `$\ghi`},
ninjaEscaped: []string{"abc", "def", `$$\ghi`},
shellEscaped: []string{"abc", "def", `'$\ghi'`},
ninjaAndShellEscaped: []string{"abc", "def", `'$$\ghi'`},
},
}
testFuncs := []struct {
name string
f func([]string) []string
expected func(tt testCase) []string
}{
{name: "NinjaEscapeList", f: NinjaEscapeList, expected: func(tt testCase) []string { return tt.ninjaEscaped }},
{name: "ShellEscapeList", f: ShellEscapeList, expected: func(tt testCase) []string { return tt.shellEscaped }},
{name: "NinjaAndShellEscapeList", f: NinjaAndShellEscapeList, expected: func(tt testCase) []string { return tt.ninjaAndShellEscaped }},
}
for _, tf := range testFuncs {
t.Run(tf.name, func(t *testing.T) {
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
inCopy := append([]string(nil), tt.in...)
got := tf.f(tt.in)
want := tf.expected(tt)
if tt.sameSlice {
want = tt.in
}
if !reflect.DeepEqual(got, want) {
t.Errorf("incorrect output, want %q got %q", want, got)
}
if len(inCopy) != len(tt.in) && (len(tt.in) == 0 || !reflect.DeepEqual(inCopy, tt.in)) {
t.Errorf("input modified, want %#v, got %#v", inCopy, tt.in)
}
if (unsafe.SliceData(tt.in) == unsafe.SliceData(got)) != tt.sameSlice {
if tt.sameSlice {
t.Errorf("expected input and output slices to have the same backing arrays")
} else {
t.Errorf("expected input and output slices to have different backing arrays")
}
}
})
}
})
}
}