Generate unsafe conversions

cmd/libs/go2idl/conversion-gen/generators/conversion.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"k8s.io/gengo/args"
@@ -36,10 +37,11 @@ import (
 // generator.
 type CustomArgs struct {
 	ExtraPeerDirs []string // Always consider these as last-ditch possibilities for conversions.
-	// SkipDefaulters indicates whether defaulter functions should be a part of conversion
+	// Skipunsafe indicates whether to generate unsafe conversions to improve the efficiency
-	// This field was introduced to ease the transition to removing defaulters from conversion.
+	// of these operations. The unsafe operation is a direct pointer assignment via unsafe
-	// It will be removed in 1.6.
+	// (within the allowed uses of unsafe) and is equivalent to a proposed Golang change to
-	SkipDefaulters bool
+	// allow structs that are identical to be assigned to each other.
+	SkipUnsafe bool
 }
 
 // This is the comment tag that carries parameters for conversion generation.
@@ -49,6 +51,16 @@ func extractTag(comments []string) []string {
 	return types.ExtractCommentTags("+", comments)[tagName]
 }
 
+func isCopyOnly(comments []string) bool {
+	values := types.ExtractCommentTags("+", comments)["k8s:conversion-fn"]
+	return len(values) == 1 && values[0] == "copy-only"
+}
+
+func isDrop(comments []string) bool {
+	values := types.ExtractCommentTags("+", comments)["k8s:conversion-fn"]
+	return len(values) == 1 && values[0] == "drop"
+}
+
 // TODO: This is created only to reduce number of changes in a single PR.
 // Remove it and use PublicNamer instead.
 func conversionNamer() *namer.NameStrategy {
@@ -156,56 +168,6 @@ func getManualConversionFunctions(context *generator.Context, pkg *types.Package
 	}
 }
 
-// All of the types in conversions map are of type "DeclarationOf" with
-// the underlying type being "Func".
-type defaulterFuncMap map[*types.Type]*types.Type
-
-// Returns all manually-defined defaulting functions in the package.
-func getManualDefaultingFunctions(context *generator.Context, pkg *types.Package, manualMap defaulterFuncMap) {
-	buffer := &bytes.Buffer{}
-	sw := generator.NewSnippetWriter(buffer, context, "$", "$")
-
-	for _, f := range pkg.Functions {
-		if f.Underlying == nil || f.Underlying.Kind != types.Func {
-			glog.Errorf("Malformed function: %#v", f)
-			continue
-		}
-		if f.Underlying.Signature == nil {
-			glog.Errorf("Function without signature: %#v", f)
-			continue
-		}
-		signature := f.Underlying.Signature
-		// Check whether the function is conversion function.
-		// Note that all of them have signature:
-		// func Convert_inType_To_outType(inType, outType, conversion.Scope) error
-		if signature.Receiver != nil {
-			continue
-		}
-		if len(signature.Parameters) != 1 {
-			continue
-		}
-		if len(signature.Results) != 0 {
-			continue
-		}
-		inType := signature.Parameters[0]
-		if inType.Kind != types.Pointer {
-			continue
-		}
-		// Now check if the name satisfies the convention.
-		args := defaultingArgsFromType(inType.Elem)
-		sw.Do("$.inType|defaultfn$", args)
-		if f.Name.Name == buffer.String() {
-			key := inType.Elem
-			// We might scan the same package twice, and that's OK.
-			if v, ok := manualMap[key]; ok && v != nil && v.Name.Package != pkg.Path {
-				panic(fmt.Sprintf("duplicate static defaulter defined: %#v", key))
-			}
-			manualMap[key] = f
-		}
-		buffer.Reset()
-	}
-}
-
 func Packages(context *generator.Context, arguments *args.GeneratorArgs) generator.Packages {
 	boilerplate, err := arguments.LoadGoBoilerplate()
 	if err != nil {
@@ -217,10 +179,17 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 	header := append([]byte(fmt.Sprintf("// +build !%s\n\n", arguments.GeneratedBuildTag)), boilerplate...)
 	header = append(header, []byte("\n// This file was autogenerated by conversion-gen. Do not edit it manually!\n\n")...)
 
-	// Accumulate pre-existing conversion and default functions.
+	// Accumulate pre-existing conversion functions.
 	// TODO: This is too ad-hoc.  We need a better way.
 	manualConversions := conversionFuncMap{}
-	manualDefaults := defaulterFuncMap{}
+
+	// Record types that are memory equivalent. A type is memory equivalent
+	// if it has the same memory layout and no nested manual conversion is
+	// defined.
+	// TODO: in the future, relax the nested manual conversion requirement
+	//   if we can show that a large enough types are memory identical but
+	//   have non-trivial conversion
+	memoryEquivalentTypes := equalMemoryTypes{}
 
 	// We are generating conversions only for packages that are explicitly
 	// passed as InputDir.
@@ -234,7 +203,6 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 
 		// Add conversion and defaulting functions.
 		getManualConversionFunctions(context, pkg, manualConversions)
-		getManualDefaultingFunctions(context, pkg, manualDefaults)
 
 		// Only generate conversions for packages which explicitly request it
 		// by specifying one or more "+k8s:conversion-gen=<peer-pkg>"
@@ -246,18 +214,23 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 			glog.V(5).Infof("  no tag")
 			continue
 		}
-		skipDefaulters := false
+		skipUnsafe := false
 		if customArgs, ok := arguments.CustomArgs.(*CustomArgs); ok {
 			if len(customArgs.ExtraPeerDirs) > 0 {
 				peerPkgs = append(peerPkgs, customArgs.ExtraPeerDirs...)
 			}
-			skipDefaulters = customArgs.SkipDefaulters
+			skipUnsafe = customArgs.SkipUnsafe
 		}
+
 		// Make sure our peer-packages are added and fully parsed.
 		for _, pp := range peerPkgs {
 			context.AddDir(pp)
 			getManualConversionFunctions(context, context.Universe[pp], manualConversions)
-			getManualDefaultingFunctions(context, context.Universe[pp], manualDefaults)
+		}
+
+		unsafeEquality := TypesEqual(memoryEquivalentTypes)
+		if skipUnsafe {
+			unsafeEquality = noEquality{}
 		}
 
 		packages = append(packages,
@@ -267,7 +240,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 				HeaderText:  header,
 				GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) {
 					return []generator.Generator{
-						NewGenConversion(arguments.OutputFileBaseName, pkg.Path, manualConversions, manualDefaults, peerPkgs, !skipDefaulters),
+						NewGenConversion(arguments.OutputFileBaseName, pkg.Path, manualConversions, peerPkgs, unsafeEquality),
 					}
 				},
 				FilterFunc: func(c *generator.Context, t *types.Type) bool {
@@ -275,9 +248,79 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 				},
 			})
 	}
+
+	// If there is a manual conversion defined between two types, exclude it
+	// from being a candidate for unsafe conversion
+	for k, v := range manualConversions {
+		if isCopyOnly(v.CommentLines) {
+			glog.V(5).Infof("Conversion function %s will not block memory copy because it is copy-only", v.Name)
+			continue
+		}
+		// this type should be excluded from all equivalence, because the converter must be called.
+		memoryEquivalentTypes.Skip(k.inType, k.outType)
+	}
+
 	return packages
 }
 
+type equalMemoryTypes map[conversionPair]bool
+
+func (e equalMemoryTypes) Skip(a, b *types.Type) {
+	e[conversionPair{a, b}] = false
+	e[conversionPair{b, a}] = false
+}
+
+func (e equalMemoryTypes) Equal(a, b *types.Type) bool {
+	if a == b {
+		return true
+	}
+	if equal, ok := e[conversionPair{a, b}]; ok {
+		return equal
+	}
+	if equal, ok := e[conversionPair{b, a}]; ok {
+		return equal
+	}
+	result := e.equal(a, b)
+	e[conversionPair{a, b}] = result
+	e[conversionPair{b, a}] = result
+	return result
+}
+
+func (e equalMemoryTypes) equal(a, b *types.Type) bool {
+	in, out := unwrapAlias(a), unwrapAlias(b)
+	switch {
+	case in == out:
+		return true
+	case in.Kind == out.Kind:
+		switch in.Kind {
+		case types.Struct:
+			if len(in.Members) != len(out.Members) {
+				return false
+			}
+			for i, inMember := range in.Members {
+				outMember := out.Members[i]
+				if !e.Equal(inMember.Type, outMember.Type) {
+					return false
+				}
+			}
+			return true
+		case types.Pointer:
+			return e.Equal(in.Elem, out.Elem)
+		case types.Map:
+			return e.Equal(in.Key, out.Key) && e.Equal(in.Elem, out.Elem)
+		case types.Slice:
+			return e.Equal(in.Elem, out.Elem)
+		case types.Interface:
+			// TODO: determine whether the interfaces are actually equivalent - for now, they must have the
+			// same type.
+			return false
+		case types.Builtin:
+			return in.Name.Name == out.Name.Name
+		}
+	}
+	return false
+}
+
 func findMember(t *types.Type, name string) (types.Member, bool) {
 	if t.Kind != types.Struct {
 		return types.Member{}, false
@@ -303,20 +346,27 @@ const (
 	conversionPackagePath = "k8s.io/kubernetes/pkg/conversion"
 )
 
+type noEquality struct{}
+
+func (noEquality) Equal(_, _ *types.Type) bool { return false }
+
+type TypesEqual interface {
+	Equal(a, b *types.Type) bool
+}
+
 // genConversion produces a file with a autogenerated conversions.
 type genConversion struct {
 	generator.DefaultGen
 	targetPackage     string
 	peerPackages      []string
 	manualConversions conversionFuncMap
-	manualDefaulters  defaulterFuncMap
 	imports           namer.ImportTracker
 	types             []*types.Type
 	skippedFields     map[*types.Type][]string
-	includeDefaulters bool
+	useUnsafe         TypesEqual
 }
 
-func NewGenConversion(sanitizedName, targetPackage string, manualConversions conversionFuncMap, manualDefaulters defaulterFuncMap, peerPkgs []string, includeDefaulters bool) generator.Generator {
+func NewGenConversion(sanitizedName, targetPackage string, manualConversions conversionFuncMap, peerPkgs []string, useUnsafe TypesEqual) generator.Generator {
 	return &genConversion{
 		DefaultGen: generator.DefaultGen{
 			OptionalName: sanitizedName,
@@ -324,11 +374,10 @@ func NewGenConversion(sanitizedName, targetPackage string, manualConversions con
 		targetPackage:     targetPackage,
 		peerPackages:      peerPkgs,
 		manualConversions: manualConversions,
-		manualDefaulters:  manualDefaulters,
 		imports:           generator.NewImportTracker(),
 		types:             []*types.Type{},
 		skippedFields:     map[*types.Type][]string{},
-		includeDefaulters: includeDefaulters,
+		useUnsafe:         useUnsafe,
 	}
 }
 
@@ -439,6 +488,22 @@ func (g *genConversion) preexists(inType, outType *types.Type) (*types.Type, boo
 }
 
 func (g *genConversion) Init(c *generator.Context, w io.Writer) error {
+	if glog.V(5) {
+		if m, ok := g.useUnsafe.(equalMemoryTypes); ok {
+			var result []string
+			glog.Infof("All objects without identical memory layout:")
+			for k, v := range m {
+				if v {
+					continue
+				}
+				result = append(result, fmt.Sprintf("  %s -> %s = %t", k.inType, k.outType, v))
+			}
+			sort.Strings(result)
+			for _, s := range result {
+				glog.Infof(s)
+			}
+		}
+	}
 	sw := generator.NewSnippetWriter(w, c, "$", "$")
 	sw.Do("func init() {\n", nil)
 	sw.Do("SchemeBuilder.Register(RegisterConversions)\n", nil)
@@ -477,10 +542,6 @@ func (g *genConversion) generateConversion(inType, outType *types.Type, sw *gene
 		With("Scope", types.Ref(conversionPackagePath, "Scope"))
 
 	sw.Do("func auto"+nameTmpl+"(in *$.inType|raw$, out *$.outType|raw$, s $.Scope|raw$) error {\n", args)
-	// if no defaulter of form SetDefaults_XXX is defined, do not inline a check for defaulting.
-	if function, ok := g.manualDefaulters[inType]; ok && g.includeDefaulters {
-		sw.Do("$.|raw$(in)\n", function)
-	}
 	g.generateFor(inType, outType, sw)
 	sw.Do("return nil\n", nil)
 	sw.Do("}\n\n", nil)
@@ -508,6 +569,7 @@ func (g *genConversion) generateConversion(inType, outType *types.Type, sw *gene
 func (g *genConversion) generateFor(inType, outType *types.Type, sw *generator.SnippetWriter) {
 	glog.V(5).Infof("generating %v -> %v", inType, outType)
 	var f func(*types.Type, *types.Type, *generator.SnippetWriter)
+
 	switch inType.Kind {
 	case types.Builtin:
 		f = g.doBuiltin
@@ -524,6 +586,7 @@ func (g *genConversion) generateFor(inType, outType *types.Type, sw *generator.S
 	default:
 		f = g.doUnknown
 	}
+
 	f(inType, outType, sw)
 }
 
@@ -638,19 +701,52 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			outMemberType = &copied
 		}
 
-		args := map[string]interface{}{
+		args := argsFromType(inMemberType, outMemberType).With("name", inMember.Name)
-			"inType":  inMemberType,
+
-			"outType": outMemberType,
+		// try a direct memory copy for any type that has exactly equivalent values
-			"name":    inMember.Name,
+		if g.useUnsafe.Equal(inMemberType, outMemberType) {
+			args = args.
+				With("Pointer", types.Ref("unsafe", "Pointer")).
+				With("SliceHeader", types.Ref("reflect", "SliceHeader"))
+			switch inMemberType.Kind {
+			case types.Pointer:
+				sw.Do("out.$.name$ = ($.outType|raw$)($.Pointer|raw$(in.$.name$))\n", args)
+				continue
+			case types.Map:
+				sw.Do("{\n", nil)
+				sw.Do("m := (*$.outType|raw$)($.Pointer|raw$(&in.$.name$))\n", args)
+				sw.Do("out.$.name$ = *m\n", args)
+				sw.Do("}\n", nil)
+				continue
+			case types.Slice:
+				sw.Do("{\n", nil)
+				sw.Do("outHdr := (*$.SliceHeader|raw$)($.Pointer|raw$(&out.$.name$))\n", args)
+				sw.Do("inHdr := (*$.SliceHeader|raw$)($.Pointer|raw$(&in.$.name$))\n", args)
+				sw.Do("*outHdr = *inHdr\n", nil)
+				sw.Do("}\n", nil)
+				continue
 			}
+		}
+
 		// check based on the top level name, not the underlying names
 		if function, ok := g.preexists(inMember.Type, outMember.Type); ok {
+			if isDrop(function.CommentLines) {
+				continue
+			}
+			// copy-only functions that are directly assignable can be inlined instead of invoked.
+			// As an example, conversion functions exist that allow types with private fields to be
+			// correctly copied between types. These functions are equivalent to a memory assignment,
+			// and are necessary for the reflection path, but should not block memory conversion.
+			// Convert_unversioned_Time_to_unversioned_Time is an example of this logic.
+			if !isCopyOnly(function.CommentLines) || !g.isFastConversion(inMemberType, outMemberType) {
 				args["function"] = function
 				sw.Do("if err := $.function|raw$(&in.$.name$, &out.$.name$, s); err != nil {\n", args)
 				sw.Do("return err\n", nil)
 				sw.Do("}\n", nil)
 				continue
 			}
+			glog.V(5).Infof("Skipped function %s because it is copy-only and we can use direct assignment", function.Name)
+		}
 
 		// If we can't auto-convert, punt before we emit any code.
 		if inMemberType.Kind != outMemberType.Kind {
@@ -722,6 +818,17 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 	}
 }
 
+func (g *genConversion) isFastConversion(inType, outType *types.Type) bool {
+	switch inType.Kind {
+	case types.Builtin:
+		return true
+	case types.Map, types.Slice, types.Pointer, types.Struct, types.Alias:
+		return g.isDirectlyAssignable(inType, outType)
+	default:
+		return false
+	}
+}
+
 func (g *genConversion) isDirectlyAssignable(inType, outType *types.Type) bool {
 	return unwrapAlias(inType) == unwrapAlias(outType)
 }

cmd/libs/go2idl/conversion-gen/main.go

@@ -60,7 +60,6 @@ func main() {
 			"k8s.io/kubernetes/pkg/conversion",
 			"k8s.io/kubernetes/pkg/runtime",
 		},
-		SkipDefaulters: true,
 	}
 	pflag.CommandLine.StringSliceVar(&customArgs.ExtraPeerDirs, "extra-peer-dirs", customArgs.ExtraPeerDirs,
 		"Comma-separated list of import paths which are considered, after tag-specified peers, for conversions.")