Merge pull request #24870 from smarterclayton/conversion_updated

Automatic merge from submit-queue

Optimize group version allocations

Avoid allocation in strings.Split() for most common cases.

Extracted from #24845

@wojtek-t or @deads2k

pkg/api/unversioned/group_version.go

@@ -29,8 +29,8 @@ import (
 // `resource.group.com` -> `group=com, version=group, resource=resource` and `group=group.com, resource=resource`
 func ParseResourceArg(arg string) (*GroupVersionResource, GroupResource) {
 	var gvr *GroupVersionResource
-	s := strings.SplitN(arg, ".", 3)
+	if strings.Count(arg, ".") >= 2 {
-	if len(s) == 3 {
+		s := strings.SplitN(arg, ".", 3)
 		gvr = &GroupVersionResource{Group: s[2], Version: s[1], Resource: s[0]}
 	}
 
@@ -64,12 +64,11 @@ func (gr *GroupResource) String() string {
 // ParseGroupResource turns "resource.group" string into a GroupResource struct.  Empty strings are allowed
 // for each field.
 func ParseGroupResource(gr string) GroupResource {
-	s := strings.SplitN(gr, ".", 2)
+	if i := strings.Index(gr, "."); i == -1 {
-	if len(s) == 1 {
+		return GroupResource{Resource: gr}
-		return GroupResource{Resource: s[0]}
+	} else {
+		return GroupResource{Group: gr[i+1:], Resource: gr[:i]}
 	}
-
-	return GroupResource{Group: s[1], Resource: s[0]}
 }
 
 // GroupVersionResource unambiguously identifies a resource.  It doesn't anonymously include GroupVersion
@@ -189,18 +188,14 @@ func ParseGroupVersion(gv string) (GroupVersion, error) {
 		return GroupVersion{}, nil
 	}
 
-	s := strings.Split(gv, "/")
+	switch strings.Count(gv, "/") {
-	// "v1" is the only special case. Otherwise GroupVersion is expected to contain
+	case 0:
-	// one "/" dividing the string into two parts.
+		return GroupVersion{"", gv}, nil
-	switch {
+	case 1:
-	case len(s) == 1 && gv == "v1":
+		i := strings.Index(gv, "/")
-		return GroupVersion{"", "v1"}, nil
+		return GroupVersion{gv[:i], gv[i+1:]}, nil
-	case len(s) == 1:
-		return GroupVersion{"", s[0]}, nil
-	case len(s) == 2:
-		return GroupVersion{s[0], s[1]}, nil
 	default:
-		return GroupVersion{}, fmt.Errorf("Unexpected GroupVersion string: %v", gv)
+		return GroupVersion{}, fmt.Errorf("unexpected GroupVersion string: %v", gv)
 	}
 }
 

pkg/api/unversioned/group_version_test.go

@@ -24,6 +24,77 @@ import (
 	"github.com/ugorji/go/codec"
 )
 
+func TestGroupVersionParse(t *testing.T) {
+	tests := []struct {
+		input string
+		out   GroupVersion
+		err   func(error) bool
+	}{
+		{input: "v1", out: GroupVersion{Version: "v1"}},
+		{input: "v2", out: GroupVersion{Version: "v2"}},
+		{input: "/v1", out: GroupVersion{Version: "v1"}},
+		{input: "v1/", out: GroupVersion{Group: "v1"}},
+		{input: "/v1/", err: func(err error) bool { return err.Error() == "unexpected GroupVersion string: /v1/" }},
+		{input: "v1/a", out: GroupVersion{Group: "v1", Version: "a"}},
+	}
+	for i, test := range tests {
+		out, err := ParseGroupVersion(test.input)
+		if test.err == nil && err != nil || err == nil && test.err != nil {
+			t.Errorf("%d: unexpected error: %v", i, err)
+			continue
+		}
+		if test.err != nil && !test.err(err) {
+			t.Errorf("%d: unexpected error: %v", i, err)
+			continue
+		}
+		if out != test.out {
+			t.Errorf("%d: unexpected output: %#v", i, out)
+		}
+	}
+}
+
+func TestGroupResourceParse(t *testing.T) {
+	tests := []struct {
+		input string
+		out   GroupResource
+	}{
+		{input: "v1", out: GroupResource{Resource: "v1"}},
+		{input: ".v1", out: GroupResource{Group: "v1"}},
+		{input: "v1.", out: GroupResource{Resource: "v1"}},
+		{input: "v1.a", out: GroupResource{Group: "a", Resource: "v1"}},
+		{input: "b.v1.a", out: GroupResource{Group: "v1.a", Resource: "b"}},
+	}
+	for i, test := range tests {
+		out := ParseGroupResource(test.input)
+		if out != test.out {
+			t.Errorf("%d: unexpected output: %#v", i, out)
+		}
+	}
+}
+
+func TestParseResourceArg(t *testing.T) {
+	tests := []struct {
+		input string
+		gvr   *GroupVersionResource
+		gr    GroupResource
+	}{
+		{input: "v1", gr: GroupResource{Resource: "v1"}},
+		{input: ".v1", gr: GroupResource{Group: "v1"}},
+		{input: "v1.", gr: GroupResource{Resource: "v1"}},
+		{input: "v1.a", gr: GroupResource{Group: "a", Resource: "v1"}},
+		{input: "b.v1.a", gvr: &GroupVersionResource{Group: "a", Version: "v1", Resource: "b"}, gr: GroupResource{Group: "v1.a", Resource: "b"}},
+	}
+	for i, test := range tests {
+		gvr, gr := ParseResourceArg(test.input)
+		if (gvr != nil && test.gvr == nil) || (gvr == nil && test.gvr != nil) || (test.gvr != nil && *gvr != *test.gvr) {
+			t.Errorf("%d: unexpected output: %#v", i, gvr)
+		}
+		if gr != test.gr {
+			t.Errorf("%d: unexpected output: %#v", i, gr)
+		}
+	}
+}
+
 type GroupVersionHolder struct {
 	GV GroupVersion `json:"val"`
 }