Snap for 11413053 from 51d8322241127212a78acc3637377f7ce394446d to build-tools-release

Change-Id: Ib162b1a9459edab04b4fb4fffbc6101bcd6b5bde

7 files changed, 276 insertions, 122 deletions

diff --git a/bootstrap/command.go b/bootstrap/command.go
index d7dcc27..580907c 100644
--- a/bootstrap/command.go
+++ b/bootstrap/command.go
@@ -40,6 +40,9 @@ type Args struct {
 	Cpuprofile string
 	Memprofile string
 	TraceFile  string
+
+	// Debug data json file
+	ModuleDebugFile string
 }
 
 // RegisterGoModuleTypes adds module types to build tools written in golang
@@ -131,6 +134,10 @@ func RunBlueprint(args Args, stopBefore StopBefore, ctx *blueprint.Context, conf
 		ninjaDeps = append(ninjaDeps, buildActionsDeps...)
 	}
 
+	if args.ModuleDebugFile != "" {
+		ctx.GenerateModuleDebugInfo(args.ModuleDebugFile)
+	}
+
 	if stopBefore == StopBeforeWriteNinja {
 		return ninjaDeps, nil
 	}
diff --git a/context.go b/context.go
index 4130700..d682e55 100644
--- a/context.go
+++ b/context.go
@@ -18,11 +18,10 @@ import (
 	"bytes"
 	"cmp"
 	"context"
-	"crypto/sha256"
-	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"hash/fnv"
 	"io"
 	"io/ioutil"
 	"os"
@@ -37,6 +36,7 @@ import (
 	"sync/atomic"
 	"text/scanner"
 	"text/template"
+	"unsafe"
 
 	"github.com/google/blueprint/metrics"
 	"github.com/google/blueprint/parser"
@@ -441,7 +441,10 @@ func (vm variationMap) subsetOf(other variationMap) bool {
 }
 
 func (vm variationMap) equal(other variationMap) bool {
-	return reflect.DeepEqual(vm, other)
+	if len(vm) != len(other) {
+		return false
+	}
+	return vm.subsetOf(other)
 }
 
 type singletonInfo struct {
@@ -3820,6 +3823,25 @@ func (c *Context) visitAllModuleVariants(module *moduleInfo,
 	}
 }
 
+func (c *Context) visitAllModuleInfos(visit func(*moduleInfo)) {
+	var module *moduleInfo
+
+	defer func() {
+		if r := recover(); r != nil {
+			panic(newPanicErrorf(r, "VisitAllModules(%s) for %s",
+				funcName(visit), module))
+		}
+	}()
+
+	for _, moduleGroup := range c.sortedModuleGroups() {
+		for _, moduleOrAlias := range moduleGroup.modules {
+			if module = moduleOrAlias.module(); module != nil {
+				visit(module)
+			}
+		}
+	}
+}
+
 func (c *Context) requireNinjaVersion(major, minor, micro int) {
 	if major != 1 {
 		panic("ninja version with major version != 1 not supported")
@@ -4710,60 +4732,73 @@ func (c *Context) SetBeforePrepareBuildActionsHook(hookFn func() error) {
 // to be extracted as a phony output
 type phonyCandidate struct {
 	sync.Once
-	phony *buildDef // the phony buildDef that wraps the set
-	first *buildDef // the first buildDef that uses this set
+	phony            *buildDef      // the phony buildDef that wraps the set
+	first            *buildDef      // the first buildDef that uses this set
+	orderOnlyStrings []string       // the original OrderOnlyStrings of the first buildDef that uses this set
+	orderOnly        []*ninjaString // the original OrderOnly of the first buildDef that uses this set
 }
 
 // keyForPhonyCandidate gives a unique identifier for a set of deps.
 // If any of the deps use a variable, we return an empty string to signal
 // that this set of deps is ineligible for extraction.
-func keyForPhonyCandidate(deps []*ninjaString, stringDeps []string) string {
-	hasher := sha256.New()
+func keyForPhonyCandidate(deps []*ninjaString, stringDeps []string) uint64 {
+	hasher := fnv.New64a()
+	write := func(s string) {
+		// The hasher doesn't retain or modify the input slice, so pass the string data directly to avoid
+		// an extra allocation and copy.
+		_, err := hasher.Write(unsafe.Slice(unsafe.StringData(s), len(s)))
+		if err != nil {
+			panic(fmt.Errorf("write failed: %w", err))
+		}
+	}
 	for _, d := range deps {
 		if len(d.Variables()) != 0 {
-			return ""
+			return 0
 		}
-		io.WriteString(hasher, d.Value(nil))
+		write(d.Value(nil))
 	}
 	for _, d := range stringDeps {
-		io.WriteString(hasher, d)
+		write(d)
 	}
-	return base64.RawURLEncoding.EncodeToString(hasher.Sum(nil))
+	return hasher.Sum64()
 }
 
 // scanBuildDef is called for every known buildDef `b` that has a non-empty `b.OrderOnly`.
 // If `b.OrderOnly` is not present in `candidates`, it gets stored.
 // But if `b.OrderOnly` already exists in `candidates`, then `b.OrderOnly`
 // (and phonyCandidate#first.OrderOnly) will be replaced with phonyCandidate#phony.Outputs
-func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.Uint32, b *buildDef) {
-	defer wg.Done()
+func scanBuildDef(candidates *sync.Map, b *buildDef) {
 	key := keyForPhonyCandidate(b.OrderOnly, b.OrderOnlyStrings)
-	if key == "" {
+	if key == 0 {
 		return
 	}
 	if v, loaded := candidates.LoadOrStore(key, &phonyCandidate{
-		first: b,
+		first:            b,
+		orderOnly:        b.OrderOnly,
+		orderOnlyStrings: b.OrderOnlyStrings,
 	}); loaded {
 		m := v.(*phonyCandidate)
-		m.Do(func() {
-			// this is the second occurrence and hence it makes sense to
-			// extract it as a phony output
-			phonyCount.Add(1)
-			m.phony = &buildDef{
-				Rule:          Phony,
-				OutputStrings: []string{"dedup-" + key},
-				Inputs:        m.first.OrderOnly, //we could also use b.OrderOnly
-				InputStrings:  m.first.OrderOnlyStrings,
-				Optional:      true,
-			}
-			// the previously recorded build-def, which first had these deps as its
-			// order-only deps, should now use this phony output instead
-			m.first.OrderOnlyStrings = m.phony.OutputStrings
-			m.first.OrderOnly = nil
-			m.first = nil
-		})
-		b.OrderOnlyStrings = m.phony.OutputStrings
-		b.OrderOnly = nil
+		if slices.EqualFunc(m.orderOnly, b.OrderOnly, ninjaStringsEqual) &&
+			slices.Equal(m.orderOnlyStrings, b.OrderOnlyStrings) {
+			m.Do(func() {
+				// this is the second occurrence and hence it makes sense to
+				// extract it as a phony output
+				m.phony = &buildDef{
+					Rule:          Phony,
+					OutputStrings: []string{fmt.Sprintf("dedup-%x", key)},
+					Inputs:        m.first.OrderOnly, //we could also use b.OrderOnly
+					InputStrings:  m.first.OrderOnlyStrings,
+					Optional:      true,
+				}
+				// the previously recorded build-def, which first had these deps as its
+				// order-only deps, should now use this phony output instead
+				m.first.OrderOnlyStrings = m.phony.OutputStrings
+				m.first.OrderOnly = nil
+				m.first = nil
+			})
+			b.OrderOnlyStrings = m.phony.OutputStrings
+			b.OrderOnly = nil
+		}
 	}
 }
 
@@ -4771,25 +4806,23 @@ func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.U
 // buildDef instances in the provided moduleInfo instances. Each such
 // common set forms a new buildDef representing a phony output that then becomes
 // the sole order-only dependency of those buildDef instances
-func (c *Context) deduplicateOrderOnlyDeps(infos []*moduleInfo) *localBuildActions {
+func (c *Context) deduplicateOrderOnlyDeps(modules []*moduleInfo) *localBuildActions {
 	c.BeginEvent("deduplicate_order_only_deps")
 	defer c.EndEvent("deduplicate_order_only_deps")
 
 	candidates := sync.Map{} //used as map[key]*candidate
-	phonyCount := atomic.Uint32{}
-	wg := sync.WaitGroup{}
-	for _, info := range infos {
-		for _, b := range info.actionDefs.buildDefs {
-			if len(b.OrderOnly) > 0 || len(b.OrderOnlyStrings) > 0 {
-				wg.Add(1)
-				go scanBuildDef(&wg, &candidates, &phonyCount, b)
+	parallelVisit(modules, unorderedVisitorImpl{}, parallelVisitLimit,
+		func(m *moduleInfo, pause chan<- pauseSpec) bool {
+			for _, b := range m.actionDefs.buildDefs {
+				if len(b.OrderOnly) > 0 || len(b.OrderOnlyStrings) > 0 {
+					scanBuildDef(&candidates, b)
+				}
 			}
-		}
-	}
-	wg.Wait()
+			return false
+		})
 
 	// now collect all created phonys to return
-	phonys := make([]*buildDef, 0, phonyCount.Load())
+	var phonys []*buildDef
 	candidates.Range(func(_ any, v any) bool {
 		candidate := v.(*phonyCandidate)
 		if candidate.phony != nil {
@@ -4920,6 +4953,126 @@ func funcName(f interface{}) string {
 	return runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name()
 }
 
+// json representation of a dependency
+type depJson struct {
+	Name    string `json:"name"`
+	Variant string `json:"variant"`
+	TagType string `json:"tag_type"`
+}
+
+// json representation of a provider
+type providerJson struct {
+	Type  string `json:"type"`
+	Debug string `json:"debug"` // from GetDebugString on the provider data
+}
+
+// interface for getting debug info from various data.
+// TODO: Consider having this return a json object instead
+type Debuggable interface {
+	GetDebugString() string
+}
+
+// Get the debug json for a single module. Returns thae data as
+// flattened json text for easy concatenation by GenerateModuleDebugInfo.
+func getModuleDebugJson(module *moduleInfo) []byte {
+	info := struct {
+		Name       string         `json:"name"`
+		SourceFile string         `json:"source_file"`
+		SourceLine int            `json:"source_line"`
+		Type       string         `json:"type"`
+		Variant    string         `json:"variant"`
+		Deps       []depJson      `json:"deps"`
+		Providers  []providerJson `json:"providers"`
+		Debug      string         `json:"debug"` // from GetDebugString on the module
+	}{
+		Name:       module.logicModule.Name(),
+		SourceFile: module.pos.Filename,
+		SourceLine: module.pos.Line,
+		Type:       module.typeName,
+		Variant:    module.variant.name,
+		Deps: func() []depJson {
+			result := make([]depJson, len(module.directDeps))
+			for i, dep := range module.directDeps {
+				result[i] = depJson{
+					Name:    dep.module.logicModule.Name(),
+					Variant: dep.module.variant.name,
+				}
+				t := reflect.TypeOf(dep.tag)
+				if t != nil {
+					result[i].TagType = t.PkgPath() + "." + t.Name()
+				}
+			}
+			return result
+		}(),
+		Providers: func() []providerJson {
+			result := make([]providerJson, 0, len(module.providers))
+			for _, p := range module.providers {
+				pj := providerJson{}
+				include := false
+
+				t := reflect.TypeOf(p)
+				if t != nil {
+					pj.Type = t.PkgPath() + "." + t.Name()
+					include = true
+				}
+
+				if dbg, ok := p.(Debuggable); ok {
+					pj.Debug = dbg.GetDebugString()
+					if pj.Debug != "" {
+						include = true
+					}
+				}
+				if include {
+					result = append(result, pj)
+				}
+			}
+			return result
+		}(),
+		Debug: func() string {
+			if dbg, ok := module.logicModule.(Debuggable); ok {
+				return dbg.GetDebugString()
+			} else {
+				return ""
+			}
+		}(),
+	}
+	buf, _ := json.Marshal(info)
+	return buf
+}
+
+// Generate out/soong/soong-debug-info.json Called if GENERATE_SOONG_DEBUG=true.
+func (this *Context) GenerateModuleDebugInfo(filename string) {
+	err := os.MkdirAll(filepath.Dir(filename), 0777)
+	if err != nil {
+		// We expect this to be writable
+		panic(fmt.Sprintf("couldn't create directory for soong module debug file %s: %s", filepath.Dir(filename), err))
+	}
+
+	f, err := os.Create(filename)
+	if err != nil {
+		// We expect this to be writable
+		panic(fmt.Sprintf("couldn't create soong module debug file %s: %s", filename, err))
+	}
+	defer f.Close()
+
+	needComma := false
+	f.WriteString("{\n\"modules\": [\n")
+
+	// TODO: Optimize this (parallel execution, etc) if it gets slow.
+	this.visitAllModuleInfos(func(module *moduleInfo) {
+		if needComma {
+			f.WriteString(",\n")
+		} else {
+			needComma = true
+		}
+
+		moduleData := getModuleDebugJson(module)
+		f.Write(moduleData)
+	})
+
+	f.WriteString("\n]\n}")
+}
+
 var fileHeaderTemplate = `******************************************************************************
 ***            This file is generated and should not be edited             ***
 ******************************************************************************
diff --git a/context_test.go b/context_test.go
index 521d163..b946497 100644
--- a/context_test.go
+++ b/context_test.go
@@ -18,8 +18,10 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"hash/fnv"
 	"path/filepath"
 	"reflect"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -1174,17 +1176,22 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 		expectedPhonys []*buildDef
 		conversions    map[string][]string
 	}
+	fnvHash := func(s string) string {
+		hash := fnv.New64a()
+		hash.Write([]byte(s))
+		return strconv.FormatUint(hash.Sum64(), 16)
+	}
 	testCases := []testcase{{
 		modules: []*moduleInfo{
 			m(b("A", nil, []string{"d"})),
 			m(b("B", nil, []string{"d"})),
 		},
 		expectedPhonys: []*buildDef{
-			b("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ", []string{"d"}, nil),
+			b("dedup-"+fnvHash("d"), []string{"d"}, nil),
 		},
 		conversions: map[string][]string{
-			"A": []string{"dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"},
-			"B": []string{"dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"},
+			"A": []string{"dedup-" + fnvHash("d")},
+			"B": []string{"dedup-" + fnvHash("d")},
 		},
 	}, {
 		modules: []*moduleInfo{
@@ -1197,11 +1204,11 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 			m(b("B", nil, []string{"b"})),
 			m(b("C", nil, []string{"a"})),
 		},
-		expectedPhonys: []*buildDef{b("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs", []string{"a"}, nil)},
+		expectedPhonys: []*buildDef{b("dedup-"+fnvHash("a"), []string{"a"}, nil)},
 		conversions: map[string][]string{
-			"A": []string{"dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"},
+			"A": []string{"dedup-" + fnvHash("a")},
 			"B": []string{"b"},
-			"C": []string{"dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"},
+			"C": []string{"dedup-" + fnvHash("a")},
 		},
 	}, {
 		modules: []*moduleInfo{
@@ -1211,13 +1218,13 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 				b("D", nil, []string{"a", "c"})),
 		},
 		expectedPhonys: []*buildDef{
-			b("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM", []string{"a", "b"}, nil),
-			b("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E", []string{"a", "c"}, nil)},
+			b("dedup-"+fnvHash("ab"), []string{"a", "b"}, nil),
+			b("dedup-"+fnvHash("ac"), []string{"a", "c"}, nil)},
 		conversions: map[string][]string{
-			"A": []string{"dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"},
-			"B": []string{"dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"},
-			"C": []string{"dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"},
-			"D": []string{"dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"},
+			"A": []string{"dedup-" + fnvHash("ab")},
+			"B": []string{"dedup-" + fnvHash("ab")},
+			"C": []string{"dedup-" + fnvHash("ac")},
+			"D": []string{"dedup-" + fnvHash("ac")},
 		},
 	}}
 	for index, tc := range testCases {
diff --git a/ninja_strings.go b/ninja_strings.go
index 8576eae..c351b93 100644
--- a/ninja_strings.go
+++ b/ninja_strings.go
@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"slices"
 	"strings"
 )
 
@@ -477,3 +478,10 @@ func validateArgNames(argNames []string) error {
 
 	return nil
 }
+
+func ninjaStringsEqual(a, b *ninjaString) bool {
+	return a.str == b.str &&
+		(a.variables == nil) == (b.variables == nil) &&
+		(a.variables == nil ||
+			slices.Equal(*a.variables, *b.variables))
+}
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 @@ package proptools
 import (
 	"fmt"
 	"reflect"
+	"strings"
 )
 
 // AppendProperties appends the values of properties in the property struct src to the property
@@ -157,29 +158,19 @@ const (
 	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 @@ func extendProperties(dst interface{}, src interface{}, filter ExtendPropertyFil
 
 	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 @@ func extendMatchingProperties(dst []interface{}, src interface{}, filter ExtendP
 		}
 	}
 
-	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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 			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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 							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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 			// 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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 			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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 				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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 			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 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 
 		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 @@ func appendPropertiesTestCases() []appendPropertyTestCase {
 			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 @@ func appendPropertiesTestCases() []appendPropertyTestCase {
 			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 @@ func appendPropertiesTestCases() []appendPropertyTestCase {
 			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 @@ func appendPropertiesTestCases() []appendPropertyTestCase {
 			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 @@ func TestExtendProperties(t *testing.T) {
 			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 @@ func TestExtendMatchingProperties(t *testing.T) {
 			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
diff --git a/proptools/hash_provider.go b/proptools/hash_provider.go
index b52a10e..6205bc7 100644
--- a/proptools/hash_provider.go
+++ b/proptools/hash_provider.go
@@ -19,7 +19,6 @@ import (
 	"encoding/binary"
 	"fmt"
 	"hash/maphash"
-	"io"
 	"math"
 	"reflect"
 	"sort"
@@ -44,7 +43,7 @@ func HashProvider(provider interface{}) (uint64, error) {
 	return hasher.Sum64(), err
 }
 
-func hashProviderInternal(hasher io.Writer, v reflect.Value, ptrs map[uintptr]bool) error {
+func hashProviderInternal(hasher *maphash.Hash, v reflect.Value, ptrs map[uintptr]bool) error {
 	var int64Array [8]byte
 	int64Buf := int64Array[:]
 	binary.LittleEndian.PutUint64(int64Buf, uint64(v.Kind()))
@@ -129,7 +128,7 @@ func hashProviderInternal(hasher io.Writer, v reflect.Value, ptrs map[uintptr]bo
 			}
 		}
 	case reflect.String:
-		hasher.Write([]byte(v.String()))
+		hasher.WriteString(v.String())
 	case reflect.Bool:
 		if v.Bool() {
 			int64Buf[0] = 1