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.

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

Reduce allocations during conversion, enable new UnsafeConvertToVersion path #25018

Merged
merged 4 commits into from
May 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions pkg/api/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ func BenchmarkPodConversion(b *testing.B) {
var result *api.Pod
for i := 0; i < b.N; i++ {
pod := &items[i%width]
versionedObj, err := scheme.ConvertToVersion(pod, testapi.Default.GroupVersion().String())
versionedObj, err := scheme.UnsafeConvertToVersion(pod, *testapi.Default.GroupVersion())
if err != nil {
b.Fatalf("Conversion error: %v", err)
}
obj, err := scheme.ConvertToVersion(versionedObj, testapi.Default.InternalGroupVersion().String())
obj, err := scheme.UnsafeConvertToVersion(versionedObj, testapi.Default.InternalGroupVersion())
if err != nil {
b.Fatalf("Conversion error: %v", err)
}
Expand All @@ -76,11 +76,11 @@ func BenchmarkNodeConversion(b *testing.B) {
scheme := api.Scheme
var result *api.Node
for i := 0; i < b.N; i++ {
versionedObj, err := scheme.ConvertToVersion(&node, testapi.Default.GroupVersion().String())
versionedObj, err := scheme.UnsafeConvertToVersion(&node, *testapi.Default.GroupVersion())
if err != nil {
b.Fatalf("Conversion error: %v", err)
}
obj, err := scheme.ConvertToVersion(versionedObj, testapi.Default.InternalGroupVersion().String())
obj, err := scheme.UnsafeConvertToVersion(versionedObj, testapi.Default.InternalGroupVersion())
if err != nil {
b.Fatalf("Conversion error: %v", err)
}
Expand All @@ -104,11 +104,11 @@ func BenchmarkReplicationControllerConversion(b *testing.B) {
scheme := api.Scheme
var result *api.ReplicationController
for i := 0; i < b.N; i++ {
versionedObj, err := scheme.ConvertToVersion(&replicationController, testapi.Default.GroupVersion().String())
versionedObj, err := scheme.UnsafeConvertToVersion(&replicationController, *testapi.Default.GroupVersion())
if err != nil {
b.Fatalf("Conversion error: %v", err)
}
obj, err := scheme.ConvertToVersion(versionedObj, testapi.Default.InternalGroupVersion().String())
obj, err := scheme.UnsafeConvertToVersion(versionedObj, testapi.Default.InternalGroupVersion())
if err != nil {
b.Fatalf("Conversion error: %v", err)
}
Expand Down
16 changes: 2 additions & 14 deletions pkg/api/meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,33 +123,21 @@ type objectAccessor struct {
}

func (obj objectAccessor) GetKind() string {
if gvk := obj.GetObjectKind().GroupVersionKind(); gvk != nil {
return gvk.Kind
}
return ""
return obj.GetObjectKind().GroupVersionKind().Kind
}

func (obj objectAccessor) SetKind(kind string) {
gvk := obj.GetObjectKind().GroupVersionKind()
if gvk == nil {
gvk = &unversioned.GroupVersionKind{}
}
gvk.Kind = kind
obj.GetObjectKind().SetGroupVersionKind(gvk)
}

func (obj objectAccessor) GetAPIVersion() string {
if gvk := obj.GetObjectKind().GroupVersionKind(); gvk != nil {
return gvk.GroupVersion().String()
}
return ""
return obj.GetObjectKind().GroupVersionKind().GroupVersion().String()
}

func (obj objectAccessor) SetAPIVersion(version string) {
gvk := obj.GetObjectKind().GroupVersionKind()
if gvk == nil {
gvk = &unversioned.GroupVersionKind{}
}
gv, err := unversioned.ParseGroupVersion(version)
if err != nil {
gv = unversioned.GroupVersion{Version: version}
Expand Down
8 changes: 4 additions & 4 deletions pkg/api/meta/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,10 @@ type InternalObject struct {
}

func (obj *InternalObject) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *InternalObject) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *InternalObject) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.TypeMeta.APIVersion, obj.TypeMeta.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *InternalObject) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *InternalObject) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.TypeMeta.APIVersion, obj.TypeMeta.Kind)
}

Expand Down Expand Up @@ -610,10 +610,10 @@ type MyAPIObject struct {
}

func (obj *MyAPIObject) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *MyAPIObject) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *MyAPIObject) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.TypeMeta.APIVersion, obj.TypeMeta.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *MyAPIObject) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *MyAPIObject) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.TypeMeta.APIVersion, obj.TypeMeta.Kind)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/meta/restmapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (fakeConvertor) Convert(in, out interface{}) error {
return nil
}

func (fakeConvertor) ConvertToVersion(in runtime.Object, _ string) (runtime.Object, error) {
func (fakeConvertor) ConvertToVersion(in runtime.Object, _ unversioned.GroupVersion) (runtime.Object, error) {
return in, nil
}

Expand Down
14 changes: 4 additions & 10 deletions pkg/api/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ func GetReference(obj runtime.Object) (*ObjectReference, error) {

// if the object referenced is actually persisted, we can just get kind from meta
// if we are building an object reference to something not yet persisted, we should fallback to scheme
var kind string
if gvk != nil {
kind = gvk.Kind
}
kind := gvk.Kind
if len(kind) == 0 {
// TODO: this is wrong
gvk, err := Scheme.ObjectKind(obj)
Expand All @@ -68,10 +65,7 @@ func GetReference(obj runtime.Object) (*ObjectReference, error) {
}

// if the object referenced is actually persisted, we can also get version from meta
var version string
if gvk != nil {
version = gvk.GroupVersion().String()
}
version := gvk.GroupVersion().String()
if len(version) == 0 {
selfLink := meta.GetSelfLink()
if len(selfLink) == 0 {
Expand Down Expand Up @@ -111,9 +105,9 @@ func GetPartialReference(obj runtime.Object, fieldPath string) (*ObjectReference

// IsAnAPIObject allows clients to preemptively get a reference to an API object and pass it to places that
// intend only to get a reference to that object. This simplifies the event recording interface.
func (obj *ObjectReference) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *ObjectReference) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *ObjectReference) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *ObjectReference) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}
4 changes: 2 additions & 2 deletions pkg/api/serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func TestObjectWatchFraming(t *testing.T) {
secret.Data["binary"] = []byte{0x00, 0x10, 0x30, 0x55, 0xff, 0x00}
secret.Data["utf8"] = []byte("a string with \u0345 characters")
secret.Data["long"] = bytes.Repeat([]byte{0x01, 0x02, 0x03, 0x00}, 1000)
converted, _ := api.Scheme.ConvertToVersion(secret, "v1")
converted, _ := api.Scheme.ConvertToVersion(secret, v1.SchemeGroupVersion)
v1secret := converted.(*v1.Secret)
for _, streamingMediaType := range api.Codecs.SupportedStreamingMediaTypes() {
s, _ := api.Codecs.StreamingSerializerForMediaType(streamingMediaType, nil)
Expand Down Expand Up @@ -358,7 +358,7 @@ func benchmarkItems() []v1.Pod {
for i := range items {
var pod api.Pod
apiObjectFuzzer.Fuzz(&pod)
out, err := api.Scheme.ConvertToVersion(&pod, "v1")
out, err := api.Scheme.ConvertToVersion(&pod, v1.SchemeGroupVersion)
if err != nil {
panic(err)
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/api/unversioned/group_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,11 @@ func (gvk *GroupVersionKind) ToAPIVersionAndKind() (string, string) {
// do not use TypeMeta. This method exists to support test types and legacy serializations
// that have a distinct group and kind.
// TODO: further reduce usage of this method.
func FromAPIVersionAndKind(apiVersion, kind string) *GroupVersionKind {
func FromAPIVersionAndKind(apiVersion, kind string) GroupVersionKind {
if gv, err := ParseGroupVersion(apiVersion); err == nil {
return &GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: kind}
return GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: kind}
}
return &GroupVersionKind{Kind: kind}
return GroupVersionKind{Kind: kind}
}

// All objects that are serialized from a Scheme encode their type information. This interface is used
Expand All @@ -273,10 +273,10 @@ func FromAPIVersionAndKind(apiVersion, kind string) *GroupVersionKind {
type ObjectKind interface {
// SetGroupVersionKind sets or clears the intended serialized kind of an object. Passing kind nil
// should clear the current setting.
SetGroupVersionKind(kind *GroupVersionKind)
SetGroupVersionKind(kind GroupVersionKind)
// GroupVersionKind returns the stored group, version, and kind of an object, or nil if the object does
// not expose or provide these fields.
GroupVersionKind() *GroupVersionKind
GroupVersionKind() GroupVersionKind
}

// EmptyObjectKind implements the ObjectKind interface as a noop
Expand All @@ -286,7 +286,7 @@ var EmptyObjectKind = emptyObjectKind{}
type emptyObjectKind struct{}

// SetGroupVersionKind implements the ObjectKind interface
func (emptyObjectKind) SetGroupVersionKind(gvk *GroupVersionKind) {}
func (emptyObjectKind) SetGroupVersionKind(gvk GroupVersionKind) {}

// GroupVersionKind implements the ObjectKind interface
func (emptyObjectKind) GroupVersionKind() *GroupVersionKind { return nil }
func (emptyObjectKind) GroupVersionKind() GroupVersionKind { return GroupVersionKind{} }
4 changes: 2 additions & 2 deletions pkg/api/unversioned/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ func Kind(kind string) GroupKind {
}

// SetGroupVersionKind satisfies the ObjectKind interface for all objects that embed TypeMeta
func (obj *TypeMeta) SetGroupVersionKind(gvk *GroupVersionKind) {
func (obj *TypeMeta) SetGroupVersionKind(gvk GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}

// GroupVersionKind satisfies the ObjectKind interface for all objects that embed TypeMeta
func (obj *TypeMeta) GroupVersionKind() *GroupVersionKind {
func (obj *TypeMeta) GroupVersionKind() GroupVersionKind {
return FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func validateOwnerReference(ownerReference api.OwnerReference, fldPath *field.Pa
if len(ownerReference.UID) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("uid"), ownerReference.UID, "uid must not be empty"))
}
if _, ok := BannedOwners[*gvk]; ok {
if _, ok := BannedOwners[gvk]; ok {
allErrs = append(allErrs, field.Invalid(fldPath, ownerReference, fmt.Sprintf("%s is disallowed from being an owner", gvk)))
}
return allErrs
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func (c stripVersionEncoder) EncodeToStream(obj runtime.Object, w io.Writer, ove
}
gvk.Group = ""
gvk.Version = ""
roundTrippedObj.GetObjectKind().SetGroupVersionKind(gvk)
roundTrippedObj.GetObjectKind().SetGroupVersionKind(*gvk)
return c.serializer.EncodeToStream(roundTrippedObj, w)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/resthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
ctx := scope.ContextFunc(req)
ctx = api.WithNamespace(ctx, namespace)

versionedObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion().String())
versionedObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion())
if err != nil {
scope.err(err, res.ResponseWriter, req.Request)
return
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/resthandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (tc *patchTestCase) Run(t *testing.T) {

namer := &testNamer{namespace, name}

versionedObj, err := api.Scheme.ConvertToVersion(&api.Pod{}, "v1")
versionedObj, err := api.Scheme.ConvertToVersion(&api.Pod{}, unversioned.GroupVersion{Version: "v1"})
if err != nil {
t.Errorf("%s: unexpected error: %v", tc.name, err)
return
Expand Down
4 changes: 2 additions & 2 deletions pkg/client/unversioned/clientcmd/api/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func init() {
}

func (obj *Config) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *Config) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *Config) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *Config) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *Config) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}
4 changes: 2 additions & 2 deletions pkg/client/unversioned/clientcmd/api/v1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ func init() {
}

func (obj *Config) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *Config) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *Config) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *Config) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *Config) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}
3 changes: 0 additions & 3 deletions pkg/conversion/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,6 @@ func (c ConversionFuncs) Merge(other ConversionFuncs) ConversionFuncs {

// Meta is supplied by Scheme, when it calls Convert.
type Meta struct {
SrcVersion string
DestVersion string

// KeyNameMapping is an optional function which may map the listed key (field name)
// into a source and destination value.
KeyNameMapping FieldMappingFunc
Expand Down
6 changes: 3 additions & 3 deletions pkg/conversion/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ func TestConverter_meta(t *testing.T) {
checks := 0
err := c.RegisterConversionFunc(
func(in *Foo, out *Bar, s Scope) error {
if s.Meta() == nil || s.Meta().SrcVersion != "test" || s.Meta().DestVersion != "passes" {
if s.Meta() == nil {
t.Errorf("Meta did not get passed!")
}
checks++
Expand All @@ -624,7 +624,7 @@ func TestConverter_meta(t *testing.T) {
}
err = c.RegisterConversionFunc(
func(in *string, out *string, s Scope) error {
if s.Meta() == nil || s.Meta().SrcVersion != "test" || s.Meta().DestVersion != "passes" {
if s.Meta() == nil {
t.Errorf("Meta did not get passed a second time!")
}
checks++
Expand All @@ -634,7 +634,7 @@ func TestConverter_meta(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
err = c.Convert(&Foo{}, &Bar{}, 0, &Meta{SrcVersion: "test", DestVersion: "passes"})
err = c.Convert(&Foo{}, &Bar{}, 0, &Meta{})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/conversion/deep_copy_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ func DeepCopy_conversion_Equalities(in Equalities, out *Equalities, c *Cloner) e
}

func DeepCopy_conversion_Meta(in Meta, out *Meta, c *Cloner) error {
out.SrcVersion = in.SrcVersion
out.DestVersion = in.DestVersion
if in.KeyNameMapping == nil {
out.KeyNameMapping = nil
} else if newVal, err := c.DeepCopy(in.KeyNameMapping); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/annotate.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (o AnnotateOptions) RunAnnotate() error {
return err
}

obj, err := info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion().String())
obj, err := info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion())
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/kubectl/cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,24 +87,24 @@ type ExternalType2 struct {
}

func (obj *internalType) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *internalType) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *internalType) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *internalType) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *internalType) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}
func (obj *externalType) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *externalType) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *externalType) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *externalType) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *externalType) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}
func (obj *ExternalType2) GetObjectKind() unversioned.ObjectKind { return obj }
func (obj *ExternalType2) SetGroupVersionKind(gvk *unversioned.GroupVersionKind) {
func (obj *ExternalType2) SetGroupVersionKind(gvk unversioned.GroupVersionKind) {
obj.APIVersion, obj.Kind = gvk.ToAPIVersionAndKind()
}
func (obj *ExternalType2) GroupVersionKind() *unversioned.GroupVersionKind {
func (obj *ExternalType2) GroupVersionKind() unversioned.GroupVersionKind {
return unversioned.FromAPIVersionAndKind(obj.APIVersion, obj.Kind)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (o *ConvertOptions) RunConvert() error {
}

infos := []*resource.Info{info}
objects, err := resource.AsVersionedObject(infos, false, o.outputVersion.String(), o.encoder)
objects, err := resource.AsVersionedObject(infos, false, o.outputVersion, o.encoder)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
if err != nil {
return err
}
objs, err := resource.AsVersionedObjects(infos, defaultVersion.String(), encoder)
objs, err := resource.AsVersionedObjects(infos, defaultVersion, encoder)
if err != nil {
return err
}
Expand Down