Skip to content

K
k3s
Unverified Commit bd0e4edd authored by Clayton Coleman's avatar Clayton Coleman
Browse files
Options

Add a new conversion path to replace GenericConversionFunc

reflect.Call is very expensive. We currently use a switch block as part of AddGenericConversionFunc to avoid the bulk of top level a->b conversion for our primary types. Instead of having these be handwritten, we should generate them. The pattern for generating them looks like: ``` scheme.AddConversionFunc(&v1.Type{}, &internal.Type{}, func(a, b interface{}, scope conversion.Scope) error { return Convert_v1_Type_to_internal_Type(a.(*v1.Type), b.(*internal.Type), scope) }) ``` which matches AddDefaultObjectFunc (which proved out the approach). The conversion machinery would then do a simple map lookup and invoke the function. This bypasses reflect.Call and in the future allows Golang mid-stack inlining to optimize this code. As a future step we can drop support for the reflection path and simply return a nice error "you must write a generator for your type".
parent a36fd65c
Hide whitespace changes
Inline Side-by-side
Showing with 82 additions and 145 deletions
staging/src/k8s.io/apimachinery/pkg/conversion/converter.go
View file @ bd0e4edd
......@@ -40,7 +40,11 @@ type NameFunc func(t reflect.Type) string
var DefaultNameFunc = func(t reflect.Type) string { return t.Name() }
type GenericConversionFunc func(a, b interface{}, scope Scope) (bool, error)
// ConversionFunc converts the object a into the object b, reusing arrays or objects
// or pointers if necessary. It should return an error if the object cannot be converted
// or if some data is invalid. If you do not wish a and b to share fields or nested
// objects, you must copy a before calling this function.
type ConversionFunc func(a, b interface{}, scope Scope) error
// Converter knows how to convert one type to another.
type Converter struct {
......@@ -161,11 +165,15 @@ type Scope interface {
type FieldMappingFunc func(key string, sourceTag, destTag reflect.StructTag) (source string, dest string)
func NewConversionFuncs() ConversionFuncs {
return ConversionFuncs{fns: make(map[typePair]reflect.Value)}
return ConversionFuncs{
fns: make(map[typePair]reflect.Value),
untyped: make(map[typePair]ConversionFunc),
}
}
type ConversionFuncs struct {
fns map[typePair]reflect.Value
fns map[typePair]reflect.Value
untyped map[typePair]ConversionFunc
}
// Add adds the provided conversion functions to the lookup table - they must have the signature
......@@ -183,6 +191,21 @@ func (c ConversionFuncs) Add(fns ...interface{}) error {
return nil
}
// AddUntyped adds the provided conversion function to the lookup table for the types that are
// supplied as a and b. a and b must be pointers or an error is returned. This method overwrites
// previously defined functions.
func (c ConversionFuncs) AddUntyped(a, b interface{}, fn ConversionFunc) error {
tA, tB := reflect.TypeOf(a), reflect.TypeOf(b)
if tA.Kind() != reflect.Ptr {
return fmt.Errorf("the type %T must be a pointer to register as an untyped conversion", a)
}
if tB.Kind() != reflect.Ptr {
return fmt.Errorf("the type %T must be a pointer to register as an untyped conversion", b)
}
c.untyped[typePair{tA, tB}] = fn
return nil
}
// Merge returns a new ConversionFuncs that contains all conversions from
// both other and c, with other conversions taking precedence.
func (c ConversionFuncs) Merge(other ConversionFuncs) ConversionFuncs {
......@@ -193,6 +216,12 @@ func (c ConversionFuncs) Merge(other ConversionFuncs) ConversionFuncs {
for k, v := range other.fns {
merged.fns[k] = v
}
for k, v := range c.untyped {
merged.untyped[k] = v
}
for k, v := range other.untyped {
merged.untyped[k] = v
}
return merged
}
......@@ -355,16 +384,32 @@ func verifyConversionFunctionSignature(ft reflect.Type) error {
// // conversion logic...
// return nil
// })
// DEPRECATED: Will be removed in favor of RegisterUntypedConversionFunc
func (c *Converter) RegisterConversionFunc(conversionFunc interface{}) error {
return c.conversionFuncs.Add(conversionFunc)
}
// Similar to RegisterConversionFunc, but registers conversion function that were
// automatically generated.
// DEPRECATED: Will be removed in favor of RegisterGeneratedUntypedConversionFunc
func (c *Converter) RegisterGeneratedConversionFunc(conversionFunc interface{}) error {
return c.generatedConversionFuncs.Add(conversionFunc)
}
// RegisterUntypedConversionFunc registers a function that converts between a and b by passing objects of those
// types to the provided function. The function *must* accept objects of a and b - this machinery will not enforce
// any other guarantee.
func (c *Converter) RegisterUntypedConversionFunc(a, b interface{}, fn ConversionFunc) error {
return c.conversionFuncs.AddUntyped(a, b, fn)
}
// RegisterGeneratedUntypedConversionFunc registers a function that converts between a and b by passing objects of those
// types to the provided function. The function *must* accept objects of a and b - this machinery will not enforce
// any other guarantee.
func (c *Converter) RegisterGeneratedUntypedConversionFunc(a, b interface{}, fn ConversionFunc) error {
return c.generatedConversionFuncs.AddUntyped(a, b, fn)
}
// RegisterIgnoredConversion registers a "no-op" for conversion, where any requested
// conversion between from and to is ignored.
func (c *Converter) RegisterIgnoredConversion(from, to interface{}) error {
......@@ -380,39 +425,6 @@ func (c *Converter) RegisterIgnoredConversion(from, to interface{}) error {
return nil
}
// IsConversionIgnored returns true if the specified objects should be dropped during
// conversion.
func (c *Converter) IsConversionIgnored(inType, outType reflect.Type) bool {
_, found := c.ignoredConversions[typePair{inType, outType}]
return found
}
func (c *Converter) HasConversionFunc(inType, outType reflect.Type) bool {
_, found := c.conversionFuncs.fns[typePair{inType, outType}]
return found
}
func (c *Converter) ConversionFuncValue(inType, outType reflect.Type) (reflect.Value, bool) {
value, found := c.conversionFuncs.fns[typePair{inType, outType}]
return value, found
}
// SetStructFieldCopy registers a correspondence. Whenever a struct field is encountered
// which has a type and name matching srcFieldType and srcFieldName, it wil be copied
// into the field in the destination struct matching destFieldType & Name, if such a
// field exists.
// May be called multiple times, even for the same source field & type--all applicable
// copies will be performed.
func (c *Converter) SetStructFieldCopy(srcFieldType interface{}, srcFieldName string, destFieldType interface{}, destFieldName string) error {
st := reflect.TypeOf(srcFieldType)
dt := reflect.TypeOf(destFieldType)
srcKey := typeNamePair{st, srcFieldName}
destKey := typeNamePair{dt, destFieldName}
c.structFieldDests[srcKey] = append(c.structFieldDests[srcKey], destKey)
c.structFieldSources[destKey] = append(c.structFieldSources[destKey], srcKey)
return nil
}
// RegisterInputDefaults registers a field name mapping function, used when converting
// from maps to structs. Inputs to the conversion methods are checked for this type and a mapping
// applied automatically if the input matches in. A set of default flags for the input conversion
......@@ -468,8 +480,8 @@ func (f FieldMatchingFlags) IsSet(flag FieldMatchingFlags) bool {
// it is not used by Convert() other than storing it in the scope.
// Not safe for objects with cyclic references!
func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, meta *Meta) error {
// TODO: deprecated, will be removed in favor of untyped conversion
if len(c.genericConversions) > 0 {
// TODO: avoid scope allocation
s := &scope{converter: c, flags: flags, meta: meta}
for _, fn := range c.genericConversions {
if ok, err := fn(src, dest, s); ok {
......@@ -495,6 +507,21 @@ func (c *Converter) DefaultConvert(src, dest interface{}, flags FieldMatchingFla
type conversionFunc func(sv, dv reflect.Value, scope *scope) error
func (c *Converter) doConversion(src, dest interface{}, flags FieldMatchingFlags, meta *Meta, f conversionFunc) error {
pair := typePair{reflect.TypeOf(src), reflect.TypeOf(dest)}
scope := &scope{
converter: c,
flags: flags,
meta: meta,
}
if fn, ok := c.conversionFuncs.untyped[pair]; ok {
return fn(src, dest, scope)
}
if fn, ok := c.generatedConversionFuncs.untyped[pair]; ok {
return fn(src, dest, scope)
}
// TODO: consider everything past this point deprecated - we want to support only point to point top level
// conversions
dv, err := EnforcePtr(dest)
if err != nil {
return err
......@@ -506,15 +533,10 @@ func (c *Converter) doConversion(src, dest interface{}, flags FieldMatchingFlags
if err != nil {
return err
}
s := &scope{
converter: c,
flags: flags,
meta: meta,
}
// Leave something on the stack, so that calls to struct tag getters never fail.
s.srcStack.push(scopeStackElem{})
s.destStack.push(scopeStackElem{})
return f(sv, dv, s)
scope.srcStack.push(scopeStackElem{})
scope.destStack.push(scopeStackElem{})
return f(sv, dv, scope)
}
// callCustom calls 'custom' with sv & dv. custom must be a conversion function.
......
staging/src/k8s.io/apimachinery/pkg/conversion/converter_test.go
View file @ bd0e4edd
......@@ -732,95 +732,3 @@ func TestConverter_flags(t *testing.T) {
}
}
}
func TestConverter_FieldRename(t *testing.T) {
type WeirdMeta struct {
Name string
Type string
}
type NameMeta struct {
Name string
}
type TypeMeta struct {
Type string
}
type A struct {
WeirdMeta
}
type B struct {
TypeMeta
NameMeta
}
c := NewConverter(DefaultNameFunc)
err := c.SetStructFieldCopy(WeirdMeta{}, "WeirdMeta", TypeMeta{}, "TypeMeta")
if err != nil {
t.Fatalf("unexpected error %v", err)
}
err = c.SetStructFieldCopy(WeirdMeta{}, "WeirdMeta", NameMeta{}, "NameMeta")
if err != nil {
t.Fatalf("unexpected error %v", err)
}
err = c.SetStructFieldCopy(TypeMeta{}, "TypeMeta", WeirdMeta{}, "WeirdMeta")
if err != nil {
t.Fatalf("unexpected error %v", err)
}
err = c.SetStructFieldCopy(NameMeta{}, "NameMeta", WeirdMeta{}, "WeirdMeta")
if err != nil {
t.Fatalf("unexpected error %v", err)
}
c.Debug = testLogger(t)
aVal := &A{
WeirdMeta: WeirdMeta{
Name: "Foo",
Type: "Bar",
},
}
bVal := &B{
TypeMeta: TypeMeta{"Bar"},
NameMeta: NameMeta{"Foo"},
}
table := map[string]struct {
from, to, expect interface{}
flags FieldMatchingFlags
}{
"to": {
aVal,
&B{},
bVal,
AllowDifferentFieldTypeNames | SourceToDest | IgnoreMissingFields,
},
"from": {
bVal,
&A{},
aVal,
AllowDifferentFieldTypeNames | SourceToDest,
},
"toDestFirst": {
aVal,
&B{},
bVal,
AllowDifferentFieldTypeNames,
},
"fromDestFirst": {
bVal,
&A{},
aVal,
AllowDifferentFieldTypeNames | IgnoreMissingFields,
},
}
for name, item := range table {
err := c.Convert(item.from, item.to, item.flags, nil)
if err != nil {
t.Errorf("%v: unexpected error: %v", name, err)
continue
}
if e, a := item.expect, item.to; !reflect.DeepEqual(e, a) {
t.Errorf("%v: unexpected diff: %v", name, diff.ObjectDiff(e, a))
}
}
}
staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go
View file @ bd0e4edd
......@@ -300,6 +300,7 @@ func (s *Scheme) New(kind schema.GroupVersionKind) (Object, error) {
// (for two conversion types) to the converter. These functions are checked first during
// a normal conversion, but are otherwise not called. Use AddConversionFuncs when registering
// typed conversions.
// DEPRECATED: Use AddConversionFunc
func (s *Scheme) AddGenericConversionFunc(fn conversion.GenericConversionFunc) {
s.converter.AddGenericConversionFunc(fn)
}
......@@ -366,6 +367,20 @@ func (s *Scheme) AddGeneratedConversionFuncs(conversionFuncs ...interface{}) err
return nil
}
// AddConversionFunc registers a function that converts between a and b by passing objects of those
// types to the provided function. The function *must* accept objects of a and b - this machinery will not enforce
// any other guarantee.
func (s *Scheme) AddConversionFunc(a, b interface{}, fn conversion.ConversionFunc) error {
return s.converter.RegisterUntypedConversionFunc(a, b, fn)
}
// AddGeneratedConversionFunc registers a function that converts between a and b by passing objects of those
// types to the provided function. The function *must* accept objects of a and b - this machinery will not enforce
// any other guarantee.
func (s *Scheme) AddGeneratedConversionFunc(a, b interface{}, fn conversion.ConversionFunc) error {
return s.converter.RegisterGeneratedUntypedConversionFunc(a, b, fn)
}
// AddFieldLabelConversionFunc adds a conversion function to convert field selectors
// of the given kind from the given version to internal version representation.
func (s *Scheme) AddFieldLabelConversionFunc(gvk schema.GroupVersionKind, conversionFunc FieldLabelConversionFunc) error {
......@@ -373,14 +388,6 @@ func (s *Scheme) AddFieldLabelConversionFunc(gvk schema.GroupVersionKind, conver
return nil
}
// AddStructFieldConversion allows you to specify a mechanical copy for a moved
// or renamed struct field without writing an entire conversion function. See
// the comment in conversion.Converter.SetStructFieldCopy for parameter details.
// Call as many times as needed, even on the same fields.
func (s *Scheme) AddStructFieldConversion(srcFieldType interface{}, srcFieldName string, destFieldType interface{}, destFieldName string) error {
return s.converter.SetStructFieldCopy(srcFieldType, srcFieldName, destFieldType, destFieldName)
}
// RegisterInputDefaults sets the provided field mapping function and field matching
// as the defaults for the provided input type. The fn may be nil, in which case no
// mapping will happen by default. Use this method to register a mechanism for handling
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment