Optimize (Extend|Append|Prepend)[Matching]Properties
The property squashing functions were performing 19% of all allocations
and allocating 3.43 GB, most of which was used to track the name of the
property, but the name of the property is only used to print errors.
Keep the elements of the name in a preallocated slice instead, and only
compute the name when an error occurs.
Calling reflect.Value.Interface() was also expensive, and only passed
to filter and order functions, none of which use the values. Modify
the signature of the filter and order functions to remove the interfaces
and the property name.
Test: extend_test.go
Change-Id: I517f89daf251bb43f7cfefa6f1e83951c0e271b7
diff --git a/proptools/extend.go b/proptools/extend.go
index 4e2f498..63ff1d7 100644
--- a/proptools/extend.go
+++ b/proptools/extend.go
@@ -17,6 +17,7 @@
import (
"fmt"
"reflect"
+ "strings"
)
// AppendProperties appends the values of properties in the property struct src to the property
@@ -157,29 +158,19 @@
Replace
)
-type ExtendPropertyFilterFunc func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (bool, error)
+type ExtendPropertyFilterFunc func(dstField, srcField reflect.StructField) (bool, error)
-type ExtendPropertyOrderFunc func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (Order, error)
+type ExtendPropertyOrderFunc func(dstField, srcField reflect.StructField) (Order, error)
-func OrderAppend(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (Order, error) {
+func OrderAppend(dstField, srcField reflect.StructField) (Order, error) {
return Append, nil
}
-func OrderPrepend(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (Order, error) {
+func OrderPrepend(dstField, srcField reflect.StructField) (Order, error) {
return Prepend, nil
}
-func OrderReplace(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (Order, error) {
+func OrderReplace(dstField, srcField reflect.StructField) (Order, error) {
return Replace, nil
}
@@ -221,7 +212,7 @@
dstValues := []reflect.Value{dstValue}
- return extendPropertiesRecursive(dstValues, srcValue, "", filter, true, order)
+ return extendPropertiesRecursive(dstValues, srcValue, make([]string, 0, 8), filter, true, order)
}
func extendMatchingProperties(dst []interface{}, src interface{}, filter ExtendPropertyFilterFunc,
@@ -244,22 +235,30 @@
}
}
- return extendPropertiesRecursive(dstValues, srcValue, "", filter, false, order)
+ return extendPropertiesRecursive(dstValues, srcValue, make([]string, 0, 8), filter, false, order)
}
func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value,
- prefix string, filter ExtendPropertyFilterFunc, sameTypes bool,
+ prefix []string, filter ExtendPropertyFilterFunc, sameTypes bool,
orderFunc ExtendPropertyOrderFunc) error {
dstValuesCopied := false
+ propertyName := func(field reflect.StructField) string {
+ names := make([]string, 0, len(prefix)+1)
+ for _, s := range prefix {
+ names = append(names, PropertyNameForField(s))
+ }
+ names = append(names, PropertyNameForField(field.Name))
+ return strings.Join(names, ".")
+ }
+
srcType := srcValue.Type()
for i, srcField := range typeFields(srcType) {
if ShouldSkipProperty(srcField) {
continue
}
- propertyName := prefix + PropertyNameForField(srcField.Name)
srcFieldValue := srcValue.Field(i)
// Step into source interfaces
@@ -271,7 +270,7 @@
srcFieldValue = srcFieldValue.Elem()
if srcFieldValue.Kind() != reflect.Ptr {
- return extendPropertyErrorf(propertyName, "interface not a pointer")
+ return extendPropertyErrorf(propertyName(srcField), "interface not a pointer")
}
}
@@ -311,8 +310,8 @@
embeddedDstValue = embeddedDstValue.Elem()
}
if !isStruct(embeddedDstValue.Type()) {
- return extendPropertyErrorf(propertyName, "%s is not a struct (%s)",
- prefix+field.Name, embeddedDstValue.Type())
+ return extendPropertyErrorf(propertyName(srcField), "%s is not a struct (%s)",
+ propertyName(field), embeddedDstValue.Type())
}
// The destination struct contains an embedded struct, add it to the list
// of destinations to consider. Make a copy of dstValues if necessary
@@ -337,13 +336,13 @@
// Step into destination interfaces
if dstFieldValue.Kind() == reflect.Interface {
if dstFieldValue.IsNil() {
- return extendPropertyErrorf(propertyName, "nilitude mismatch")
+ return extendPropertyErrorf(propertyName(srcField), "nilitude mismatch")
}
dstFieldValue = dstFieldValue.Elem()
if dstFieldValue.Kind() != reflect.Ptr {
- return extendPropertyErrorf(propertyName, "interface not a pointer")
+ return extendPropertyErrorf(propertyName(srcField), "interface not a pointer")
}
}
@@ -360,7 +359,7 @@
switch srcFieldValue.Kind() {
case reflect.Struct:
if sameTypes && dstFieldValue.Type() != srcFieldValue.Type() {
- return extendPropertyErrorf(propertyName, "mismatched types %s and %s",
+ return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s",
dstFieldValue.Type(), srcFieldValue.Type())
}
@@ -369,34 +368,30 @@
continue
case reflect.Bool, reflect.String, reflect.Slice, reflect.Map:
if srcFieldValue.Type() != dstFieldValue.Type() {
- return extendPropertyErrorf(propertyName, "mismatched types %s and %s",
+ return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s",
dstFieldValue.Type(), srcFieldValue.Type())
}
case reflect.Ptr:
if srcFieldValue.Type() != dstFieldValue.Type() {
- return extendPropertyErrorf(propertyName, "mismatched types %s and %s",
+ return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s",
dstFieldValue.Type(), srcFieldValue.Type())
}
switch ptrKind := srcFieldValue.Type().Elem().Kind(); ptrKind {
case reflect.Bool, reflect.Int64, reflect.String, reflect.Struct:
// Nothing
default:
- return extendPropertyErrorf(propertyName, "pointer is a %s", ptrKind)
+ return extendPropertyErrorf(propertyName(srcField), "pointer is a %s", ptrKind)
}
default:
- return extendPropertyErrorf(propertyName, "unsupported kind %s",
+ return extendPropertyErrorf(propertyName(srcField), "unsupported kind %s",
srcFieldValue.Kind())
}
- dstFieldInterface := dstFieldValue.Interface()
- srcFieldInterface := srcFieldValue.Interface()
-
if filter != nil {
- b, err := filter(propertyName, dstField, srcField,
- dstFieldInterface, srcFieldInterface)
+ b, err := filter(dstField, srcField)
if err != nil {
return &ExtendPropertyError{
- Property: propertyName,
+ Property: propertyName(srcField),
Err: err,
}
}
@@ -408,11 +403,10 @@
order := Append
if orderFunc != nil {
var err error
- order, err = orderFunc(propertyName, dstField, srcField,
- dstFieldInterface, srcFieldInterface)
+ order, err = orderFunc(dstField, srcField)
if err != nil {
return &ExtendPropertyError{
- Property: propertyName,
+ Property: propertyName(srcField),
Err: err,
}
}
@@ -423,12 +417,12 @@
if len(recurse) > 0 {
err := extendPropertiesRecursive(recurse, srcFieldValue,
- propertyName+".", filter, sameTypes, orderFunc)
+ append(prefix, srcField.Name), filter, sameTypes, orderFunc)
if err != nil {
return err
}
} else if !found {
- return extendPropertyErrorf(propertyName, "failed to find property to extend")
+ return extendPropertyErrorf(propertyName(srcField), "failed to find property to extend")
}
}
diff --git a/proptools/extend_test.go b/proptools/extend_test.go
index d2dac72..e562776 100644
--- a/proptools/extend_test.go
+++ b/proptools/extend_test.go
@@ -1168,9 +1168,7 @@
out: &struct{ S string }{
S: "string1string2",
},
- filter: func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (bool, error) {
+ filter: func(dstField, srcField reflect.StructField) (bool, error) {
return true, nil
},
},
@@ -1185,9 +1183,7 @@
out: &struct{ S string }{
S: "string1",
},
- filter: func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (bool, error) {
+ filter: func(dstField, srcField reflect.StructField) (bool, error) {
return false, nil
},
},
@@ -1202,12 +1198,8 @@
out: &struct{ S string }{
S: "string1string2",
},
- filter: func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (bool, error) {
- return property == "s" &&
- dstField.Name == "S" && srcField.Name == "S" &&
- dstValue.(string) == "string1" && srcValue.(string) == "string2", nil
+ filter: func(dstField, srcField reflect.StructField) (bool, error) {
+ return dstField.Name == "S" && srcField.Name == "S", nil
},
},
{
@@ -1257,9 +1249,7 @@
out: &struct{ S string }{
S: "string1",
},
- filter: func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (bool, error) {
+ filter: func(dstField, srcField reflect.StructField) (bool, error) {
return true, fmt.Errorf("filter error")
},
err: extendPropertyErrorf("s", "filter error"),
@@ -1300,9 +1290,7 @@
var err error
var testType string
- order := func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (Order, error) {
+ order := func(dstField, srcField reflect.StructField) (Order, error) {
switch testCase.order {
case Append:
return Append, nil
@@ -1764,9 +1752,7 @@
var err error
var testType string
- order := func(property string,
- dstField, srcField reflect.StructField,
- dstValue, srcValue interface{}) (Order, error) {
+ order := func(dstField, srcField reflect.StructField) (Order, error) {
switch testCase.order {
case Append:
return Append, nil
