optimize reflect cache lookups, optimize kind checks

Author: jpbetz

value/listreflect.go

@@ -58,13 +58,15 @@ func (r listReflect) Range() ListRange {
 	rr := lrrPool.Get().(*listReflectRange)
 	rr.list = r.Value
 	rr.i = -1
+	rr.entry = nil
 	return rr
 }
 
 type listReflectRange struct {
-	list reflect.Value
-	vr   *valueReflect
-	i    int
+	list  reflect.Value
+	vr    *valueReflect
+	i     int
+	entry *TypeReflectCacheEntry
 }
 
 func (r *listReflectRange) Next() bool {
@@ -79,7 +81,11 @@ func (r *listReflectRange) Item() (index int, value Value) {
 	if r.i >= r.list.Len() {
 		panic("Item() called on ListRange with no more items")
 	}
-	return r.i, r.vr.mustReuse(r.list.Index(r.i), nil, nil)
+	v := r.list.Index(r.i)
+	if r.entry == nil {
+		r.entry = TypeReflectEntryOf(v.Type())
+	}
+	return r.i, r.vr.mustReuse(v, r.entry, nil, nil)
 }
 
 func (r *listReflectRange) Recycle() {

value/mapreflect.go

@@ -68,21 +68,29 @@ func (r mapReflect) toMapKey(key string) reflect.Value {
 }
 
 func (r mapReflect) Iterate(fn func(string, Value) bool) bool {
+	if r.Value.Len() == 0 {
+		return true
+	}
 	vr := reflectPool.Get().(*valueReflect)
 	defer vr.Recycle()
-	return eachMapEntry(r.Value, func(key reflect.Value, value reflect.Value) bool {
-		return fn(key.String(), vr.mustReuse(value, &r.Value, &key))
+	return eachMapEntry(r.Value, func(e *TypeReflectCacheEntry, key reflect.Value, value reflect.Value) bool {
+		// TODO: Track cache entry
+		return fn(key.String(), vr.mustReuse(value, nil, &r.Value, &key))
 	})
 }
 
-func eachMapEntry(val reflect.Value, fn func(reflect.Value, reflect.Value) bool) bool {
+func eachMapEntry(val reflect.Value, fn func(*TypeReflectCacheEntry, reflect.Value, reflect.Value) bool) bool {
 	iter := val.MapRange()
+	var entry *TypeReflectCacheEntry
 	for iter.Next() {
 		next := iter.Value()
 		if !next.IsValid() {
 			continue
 		}
-		if !fn(iter.Key(), next) {
+		if entry == nil {
+			entry = TypeReflectEntryOf(next.Type())
+		}
+		if !fn(entry, iter.Key(), next) {
 			return false
 		}
 	}
@@ -114,6 +122,7 @@ func (r mapReflect) Equals(m Map) bool {
 		if !ok {
 			return false
 		}
-		return Equals(vr.mustReuse(lhsVal, nil, nil), value)
+		// TODO: Track cache entry
+		return Equals(vr.mustReuse(lhsVal, nil, nil, nil), value)
 	})
 }

value/reflectcache.go

@@ -57,6 +57,9 @@ type FieldCacheEntry struct {
 	// a field in a reflect.Value struct. The field indices in the list form a path used
 	// to traverse through intermediary 'inline' fields.
 	fieldPath [][]int
+
+	fieldType reflect.Type
+	TypeEntry *TypeReflectCacheEntry
 }
 
 // GetFrom returns the field identified by this FieldCacheEntry from the provided struct.
@@ -74,25 +77,51 @@ var unstructuredConvertableType = reflect.TypeOf(new(UnstructuredConverter)).Ele
 var defaultReflectCache = newReflectCache()
 
 // TypeReflectEntryOf returns the TypeReflectCacheEntry of the provided reflect.Type.
-func TypeReflectEntryOf(t reflect.Type) TypeReflectCacheEntry {
-	if record, ok := defaultReflectCache.get(t); ok {
+func TypeReflectEntryOf(t reflect.Type) *TypeReflectCacheEntry {
+	cm := defaultReflectCache.get()
+	if record, ok := cm[t]; ok {
+		return record
+	}
+	updates := reflectCacheMap{}
+	result := typeReflectEntryOf(cm, t, updates)
+	if len(updates) > 0 {
+		defaultReflectCache.update(updates)
+	}
+	return result
+}
+
+// TypeReflectEntryOf returns all updates needed to add provided reflect.Type, and the types its fields transitively
+// depend on, to the cache.
+func typeReflectEntryOf(cm reflectCacheMap, t reflect.Type, updates reflectCacheMap) *TypeReflectCacheEntry {
+	if record, ok := cm[t]; ok {
+		return record
+	}
+	if record, ok := updates[t]; ok {
 		return record
 	}
-	record := TypeReflectCacheEntry{
+	typeEntry := &TypeReflectCacheEntry{
 		isJsonMarshaler:        t.Implements(marshalerType),
 		ptrIsJsonMarshaler:     reflect.PtrTo(t).Implements(marshalerType),
 		isJsonUnmarshaler:      reflect.PtrTo(t).Implements(unmarshalerType),
 		isStringConvertable:    t.Implements(unstructuredConvertableType),
 		ptrIsStringConvertable: reflect.PtrTo(t).Implements(unstructuredConvertableType),
 	}
 	if t.Kind() == reflect.Struct {
-		hints := map[string]*FieldCacheEntry{}
-		buildStructCacheEntry(t, hints, nil)
-		record.structFields = hints
+		fieldEntries := map[string]*FieldCacheEntry{}
+		buildStructCacheEntry(t, fieldEntries, nil)
+		typeEntry.structFields = fieldEntries
 	}
 
-	defaultReflectCache.update(t, record)
-	return record
+	// cyclic type references are allowed, so we must add the typeEntry to the updates map before resolving
+	// the field.typeEntry references, or creating them if they are not already in the cache
+	updates[t] = typeEntry
+
+	for _, field := range typeEntry.structFields {
+		if field.TypeEntry == nil {
+			field.TypeEntry = typeReflectEntryOf(cm, field.fieldType, updates)
+		}
+	}
+	return typeEntry
 }
 
 func buildStructCacheEntry(t reflect.Type, infos map[string]*FieldCacheEntry, fieldPath [][]int) {
@@ -106,7 +135,7 @@ func buildStructCacheEntry(t reflect.Type, infos map[string]*FieldCacheEntry, fi
 			buildStructCacheEntry(field.Type, infos, append(fieldPath, field.Index))
 			continue
 		}
-		info := &FieldCacheEntry{isOmitEmpty: isOmitempty, fieldPath: append(fieldPath, field.Index)}
+		info := &FieldCacheEntry{isOmitEmpty: isOmitempty, fieldPath: append(fieldPath, field.Index), fieldType: t}
 		infos[jsonName] = info
 	}
 }
@@ -275,22 +304,29 @@ func newReflectCache() *typeReflectCache {
 	return cache
 }
 
-type reflectCacheMap map[reflect.Type]TypeReflectCacheEntry
+type reflectCacheMap map[reflect.Type]*TypeReflectCacheEntry
 
-// get returns true and TypeReflectCacheEntry for the given type if the type is in the cache. Otherwise get returns false.
-func (c *typeReflectCache) get(t reflect.Type) (TypeReflectCacheEntry, bool) {
-	entry, ok := c.value.Load().(reflectCacheMap)[t]
-	return entry, ok
+// get returns the reflectCacheMap.
+func (c *typeReflectCache) get() reflectCacheMap {
+	return c.value.Load().(reflectCacheMap)
 }
 
-// update sets the TypeReflectCacheEntry for the given type via a copy-on-write update to the struct cache.
-func (c *typeReflectCache) update(t reflect.Type, m TypeReflectCacheEntry) {
+// update merges the provided updates into the cache.
+func (c *typeReflectCache) update(updates reflectCacheMap) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
 	currentCacheMap := c.value.Load().(reflectCacheMap)
-	if _, ok := currentCacheMap[t]; ok {
-		// Bail if the entry has been set while waiting for lock acquisition.
+
+	hasNewEntries := false
+	for t := range updates {
+		if _, ok := currentCacheMap[t]; !ok {
+			hasNewEntries = true
+			break
+		}
+	}
+	if !hasNewEntries {
+		// Bail if the updates have been set while waiting for lock acquisition.
 		// This is safe since setting entries is idempotent.
 		return
 	}
@@ -299,6 +335,8 @@ func (c *typeReflectCache) update(t reflect.Type, m TypeReflectCacheEntry) {
 	for k, v := range currentCacheMap {
 		newCacheMap[k] = v
 	}
-	newCacheMap[t] = m
+	for t, update := range updates {
+		newCacheMap[t] = update
+	}
 	c.value.Store(newCacheMap)
 }

value/structreflect.go

@@ -27,7 +27,7 @@ type structReflect struct {
 
 func (r structReflect) Length() int {
 	i := 0
-	eachStructField(r.Value, func(s string, value reflect.Value) bool {
+	eachStructField(r.Value, func(_ *TypeReflectCacheEntry, s string, value reflect.Value) bool {
 		i++
 		return true
 	})
@@ -94,19 +94,19 @@ func (r structReflect) update(fieldEntry *FieldCacheEntry, key string, oldVal, n
 func (r structReflect) Iterate(fn func(string, Value) bool) bool {
 	vr := reflectPool.Get().(*valueReflect)
 	defer vr.Recycle()
-	return eachStructField(r.Value, func(s string, value reflect.Value) bool {
-		return fn(s, vr.mustReuse(value, nil, nil))
+	return eachStructField(r.Value, func(e *TypeReflectCacheEntry, s string, value reflect.Value) bool {
+		return fn(s, vr.mustReuse(value, e, nil, nil))
 	})
 }
 
-func eachStructField(structVal reflect.Value, fn func(string, reflect.Value) bool) bool {
+func eachStructField(structVal reflect.Value, fn func(*TypeReflectCacheEntry, string, reflect.Value) bool) bool {
 	for jsonName, fieldCacheEntry := range TypeReflectEntryOf(structVal.Type()).Fields() {
 		fieldVal := fieldCacheEntry.GetFrom(structVal)
 		if fieldCacheEntry.isOmitEmpty && (safeIsNil(fieldVal) || isZero(fieldVal)) {
 			// omit it
 			continue
 		}
-		ok := fn(jsonName, fieldVal)
+		ok := fn(fieldCacheEntry.TypeEntry, jsonName, fieldVal)
 		if !ok {
 			return false
 		}
@@ -146,7 +146,7 @@ func (r structReflect) Equals(m Map) bool {
 			return false
 		}
 		lhsVal := fieldCacheEntry.GetFrom(r.Value)
-		return Equals(vr.mustReuse(lhsVal, nil, nil), value)
+		return Equals(vr.mustReuse(lhsVal, nil, nil, nil), value)
 	})
 }