Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove all reflection for locating defaulting functions #24966

Merged
merged 4 commits into from
May 8, 2016
Merged

Remove all reflection for locating defaulting functions #24966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
123f698
Always inline default functions on all packages
smarterclayton Apr 29, 2016
7104be8
Regenerate conversions without reflection defaulting
smarterclayton Apr 29, 2016
db5a851
Inline default methods by hand in custom conversions
smarterclayton Apr 29, 2016
9ebfa1d
Add a proto full decode benchmark
smarterclayton Apr 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
95 changes: 86 additions & 9 deletions cmd/libs/go2idl/conversion-gen/generators/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,21 @@ func conversionNamer() *namer.NameStrategy {
}
}

func defaultFnNamer() *namer.NameStrategy {
return &namer.NameStrategy{
Prefix: "SetDefaults_",
Join: func(pre string, in []string, post string) string {
return pre + strings.Join(in, "_") + post
},
}
}

// NameSystems returns the name system used by the generators in this package.
func NameSystems() namer.NameSystems {
return namer.NameSystems{
"public": conversionNamer(),
"raw": namer.NewRawNamer("", nil),
"public": conversionNamer(),
"raw": namer.NewRawNamer("", nil),
"defaultfn": defaultFnNamer(),
}
}

Expand Down Expand Up @@ -126,7 +136,64 @@ func existingConversionFunctions(context *generator.Context) conversions {
args := argsFromType(inType.Elem, outType.Elem)
sw.Do("Convert_$.inType|public$_To_$.outType|public$", args)
if f.Name.Name == buffer.String() {
preexisting[conversionType{inType.Elem, outType.Elem}] = f
key := conversionType{inType.Elem, outType.Elem}
if v, ok := preexisting[key]; ok && v != nil {
panic(fmt.Sprintf("duplicate static conversion defined: %#v", key))
}
preexisting[key] = f
}
buffer.Reset()
}
}
return preexisting
}

// All of the types in conversions map are of type "DeclarationOf" with
// the underlying type being "Func".
type defaulters map[*types.Type]*types.Type

// Returns all already existing defaulting functions that we are able to find.
func existingDefaultingFunctions(context *generator.Context) defaulters {
buffer := &bytes.Buffer{}
sw := generator.NewSnippetWriter(buffer, context, "$", "$")

preexisting := make(defaulters)
for _, p := range context.Universe {
for _, f := range p.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
if v, ok := preexisting[key]; ok && v != nil {
panic(fmt.Sprintf("duplicate static defaulter defined: %#v", key))
}
preexisting[key] = f
}
buffer.Reset()
}
Expand Down Expand Up @@ -154,6 +221,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat

// Compute all pre-existing conversion functions.
preexisting := existingConversionFunctions(context)
preexistingDefaults := existingDefaultingFunctions(context)

// We are generating conversions only for packages that are explicitly
// passed as InputDir, and only for those that have a corresponding type
Expand Down Expand Up @@ -205,7 +273,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) {
generators = []generator.Generator{}
generators = append(
generators, NewGenConversion("conversion_generated", path, preexisting))
generators, NewGenConversion("conversion_generated", path, preexisting, preexistingDefaults))
return generators
},
FilterFunc: func(c *generator.Context, t *types.Type) bool {
Expand Down Expand Up @@ -308,17 +376,19 @@ type genConversion struct {
generator.DefaultGen
targetPackage string
preexisting conversions
defaulters defaulters
imports namer.ImportTracker
typesForInit []conversionType
}

func NewGenConversion(sanitizedName, targetPackage string, preexisting conversions) generator.Generator {
func NewGenConversion(sanitizedName, targetPackage string, preexisting conversions, defaulters defaulters) generator.Generator {
return &genConversion{
DefaultGen: generator.DefaultGen{
OptionalName: sanitizedName,
},
targetPackage: targetPackage,
preexisting: preexisting,
defaulters: defaulters,
imports: generator.NewImportTracker(),
typesForInit: make([]conversionType, 0),
}
Expand Down Expand Up @@ -388,7 +458,7 @@ func (g *genConversion) isOtherPackage(pkg string) bool {
}

func (g *genConversion) Imports(c *generator.Context) (imports []string) {
importLines := []string{"reflect \"reflect\""}
var importLines []string
if g.isOtherPackage(apiPackagePath) {
importLines = append(importLines, "api \""+apiPackagePath+"\"")
}
Expand All @@ -410,6 +480,11 @@ func argsFromType(inType, outType *types.Type) interface{} {
}
}

func defaultingArgsFromType(inType *types.Type) interface{} {
return map[string]interface{}{
"inType": inType,
}
}
func (g *genConversion) funcNameTmpl(inType, outType *types.Type) string {
tmpl := "Convert_$.inType|public$_To_$.outType|public$"
g.imports.AddType(inType)
Expand Down Expand Up @@ -461,9 +536,11 @@ func (g *genConversion) generateConversion(inType, outType *types.Type, sw *gene
} else {
sw.Do(fmt.Sprintf("func auto%s(in *$.inType|raw$, out *$.outType|raw$, s conversion.Scope) error {\n", funcName), argsFromType(inType, outType))
}
sw.Do("if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {\n", nil)
sw.Do("defaulting.(func(*$.|raw$))(in)\n", inType)
sw.Do("}\n", nil)
// if no defaulter of form SetDefaults_XXX is defined, do not inline a check for defaulting.
if function, ok := g.defaulters[inType]; ok {
sw.Do("$.|raw$(in)\n", function)
}

g.generateFor(inType, outType, sw)
sw.Do("return nil\n", nil)
sw.Do("}\n\n", nil)
Expand Down
37 changes: 0 additions & 37 deletions federation/apis/federation/v1alpha1/conversion_generated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
unversioned "k8s.io/kubernetes/pkg/api/unversioned"
v1 "k8s.io/kubernetes/pkg/api/v1"
conversion "k8s.io/kubernetes/pkg/conversion"
reflect "reflect"
)

func init() {
Expand All @@ -51,9 +50,6 @@ func init() {
}

func autoConvert_v1alpha1_Cluster_To_federation_Cluster(in *Cluster, out *federation.Cluster, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*Cluster))(in)
}
if err := api.Convert_unversioned_TypeMeta_To_unversioned_TypeMeta(&in.TypeMeta, &out.TypeMeta, s); err != nil {
return err
}
Expand All @@ -75,9 +71,6 @@ func Convert_v1alpha1_Cluster_To_federation_Cluster(in *Cluster, out *federation
}

func autoConvert_federation_Cluster_To_v1alpha1_Cluster(in *federation.Cluster, out *Cluster, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*federation.Cluster))(in)
}
if err := api.Convert_unversioned_TypeMeta_To_unversioned_TypeMeta(&in.TypeMeta, &out.TypeMeta, s); err != nil {
return err
}
Expand All @@ -99,9 +92,6 @@ func Convert_federation_Cluster_To_v1alpha1_Cluster(in *federation.Cluster, out
}

func autoConvert_v1alpha1_ClusterCondition_To_federation_ClusterCondition(in *ClusterCondition, out *federation.ClusterCondition, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*ClusterCondition))(in)
}
out.Type = federation.ClusterConditionType(in.Type)
out.Status = api.ConditionStatus(in.Status)
if err := api.Convert_unversioned_Time_To_unversioned_Time(&in.LastProbeTime, &out.LastProbeTime, s); err != nil {
Expand All @@ -120,9 +110,6 @@ func Convert_v1alpha1_ClusterCondition_To_federation_ClusterCondition(in *Cluste
}

func autoConvert_federation_ClusterCondition_To_v1alpha1_ClusterCondition(in *federation.ClusterCondition, out *ClusterCondition, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*federation.ClusterCondition))(in)
}
out.Type = ClusterConditionType(in.Type)
out.Status = v1.ConditionStatus(in.Status)
if err := api.Convert_unversioned_Time_To_unversioned_Time(&in.LastProbeTime, &out.LastProbeTime, s); err != nil {
Expand All @@ -141,9 +128,6 @@ func Convert_federation_ClusterCondition_To_v1alpha1_ClusterCondition(in *federa
}

func autoConvert_v1alpha1_ClusterList_To_federation_ClusterList(in *ClusterList, out *federation.ClusterList, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*ClusterList))(in)
}
if err := api.Convert_unversioned_TypeMeta_To_unversioned_TypeMeta(&in.TypeMeta, &out.TypeMeta, s); err != nil {
return err
}
Expand All @@ -169,9 +153,6 @@ func Convert_v1alpha1_ClusterList_To_federation_ClusterList(in *ClusterList, out
}

func autoConvert_federation_ClusterList_To_v1alpha1_ClusterList(in *federation.ClusterList, out *ClusterList, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*federation.ClusterList))(in)
}
if err := api.Convert_unversioned_TypeMeta_To_unversioned_TypeMeta(&in.TypeMeta, &out.TypeMeta, s); err != nil {
return err
}
Expand All @@ -197,9 +178,6 @@ func Convert_federation_ClusterList_To_v1alpha1_ClusterList(in *federation.Clust
}

func autoConvert_v1alpha1_ClusterMeta_To_federation_ClusterMeta(in *ClusterMeta, out *federation.ClusterMeta, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*ClusterMeta))(in)
}
out.Version = in.Version
return nil
}
Expand All @@ -209,9 +187,6 @@ func Convert_v1alpha1_ClusterMeta_To_federation_ClusterMeta(in *ClusterMeta, out
}

func autoConvert_federation_ClusterMeta_To_v1alpha1_ClusterMeta(in *federation.ClusterMeta, out *ClusterMeta, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*federation.ClusterMeta))(in)
}
out.Version = in.Version
return nil
}
Expand All @@ -221,9 +196,6 @@ func Convert_federation_ClusterMeta_To_v1alpha1_ClusterMeta(in *federation.Clust
}

func autoConvert_v1alpha1_ClusterSpec_To_federation_ClusterSpec(in *ClusterSpec, out *federation.ClusterSpec, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*ClusterSpec))(in)
}
if in.ServerAddressByClientCIDRs != nil {
in, out := &in.ServerAddressByClientCIDRs, &out.ServerAddressByClientCIDRs
*out = make([]unversioned.ServerAddressByClientCIDR, len(*in))
Expand All @@ -245,9 +217,6 @@ func Convert_v1alpha1_ClusterSpec_To_federation_ClusterSpec(in *ClusterSpec, out
}

func autoConvert_federation_ClusterSpec_To_v1alpha1_ClusterSpec(in *federation.ClusterSpec, out *ClusterSpec, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*federation.ClusterSpec))(in)
}
if in.ServerAddressByClientCIDRs != nil {
in, out := &in.ServerAddressByClientCIDRs, &out.ServerAddressByClientCIDRs
*out = make([]unversioned.ServerAddressByClientCIDR, len(*in))
Expand All @@ -269,9 +238,6 @@ func Convert_federation_ClusterSpec_To_v1alpha1_ClusterSpec(in *federation.Clust
}

func autoConvert_v1alpha1_ClusterStatus_To_federation_ClusterStatus(in *ClusterStatus, out *federation.ClusterStatus, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*ClusterStatus))(in)
}
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]federation.ClusterCondition, len(*in))
Expand Down Expand Up @@ -300,9 +266,6 @@ func Convert_v1alpha1_ClusterStatus_To_federation_ClusterStatus(in *ClusterStatu
}

func autoConvert_federation_ClusterStatus_To_v1alpha1_ClusterStatus(in *federation.ClusterStatus, out *ClusterStatus, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*federation.ClusterStatus))(in)
}
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]ClusterCondition, len(*in))
Expand Down
26 changes: 26 additions & 0 deletions pkg/api/serialization_proto_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,32 @@ func BenchmarkEncodeProtobufGeneratedMarshal(b *testing.B) {
b.StopTimer()
}

// BenchmarkDecodeCodecToInternalProtobuf measures the cost of performing a codec decode,
// including conversions and any type setting. This is a "full" decode.
func BenchmarkDecodeCodecToInternalProtobuf(b *testing.B) {
items := benchmarkItems()
width := len(items)
s := protobuf.NewSerializer(api.Scheme, runtime.ObjectTyperToTyper(api.Scheme), "application/arbitrary.content.type")
encoder := api.Codecs.EncoderForVersion(s, v1.SchemeGroupVersion)
var encoded [][]byte
for i := range items {
data, err := runtime.Encode(encoder, &items[i])
if err != nil {
b.Fatal(err)
}
encoded = append(encoded, data)
}

decoder := api.Codecs.DecoderToVersion(s, api.SchemeGroupVersion)
b.ResetTimer()
for i := 0; i < b.N; i++ {
if _, err := runtime.Decode(decoder, encoded[i%width]); err != nil {
b.Fatal(err)
}
}
b.StopTimer()
}

// BenchmarkDecodeJSON provides a baseline for regular JSON decode performance
func BenchmarkDecodeIntoProtobuf(b *testing.B) {
items := benchmarkItems()
Expand Down