More efficient selector

pkg/api/helpers.go

@@ -417,7 +417,7 @@ func NodeSelectorRequirementsAsSelector(nsm []NodeSelectorRequirement) (labels.S
 		default:
 			return nil, fmt.Errorf("%q is not a valid node selector operator", expr.Operator)
 		}
-		r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...))
+		r, err := labels.NewRequirement(expr.Key, op, expr.Values)
 		if err != nil {
 			return nil, err
 		}

pkg/api/unversioned/helpers.go

@@ -21,7 +21,6 @@ import (
 
 	"k8s.io/kubernetes/pkg/labels"
 	"k8s.io/kubernetes/pkg/selection"
-	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 // LabelSelectorAsSelector converts the LabelSelector api type into a struct that implements
@@ -36,7 +35,7 @@ func LabelSelectorAsSelector(ps *LabelSelector) (labels.Selector, error) {
 	}
 	selector := labels.NewSelector()
 	for k, v := range ps.MatchLabels {
-		r, err := labels.NewRequirement(k, selection.Equals, sets.NewString(v))
+		r, err := labels.NewRequirement(k, selection.Equals, []string{v})
 		if err != nil {
 			return nil, err
 		}
@@ -56,7 +55,7 @@ func LabelSelectorAsSelector(ps *LabelSelector) (labels.Selector, error) {
 		default:
 			return nil, fmt.Errorf("%q is not a valid pod selector operator", expr.Operator)
 		}
-		r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...))
+		r, err := labels.NewRequirement(expr.Key, op, append([]string(nil), expr.Values...))
 		if err != nil {
 			return nil, err
 		}

pkg/labels/selector.go

@@ -91,9 +91,12 @@ func (a ByKey) Less(i, j int) bool { return a[i].key < a[j].key }
 // Requirement implements both set based match and exact match
 // Requirement should be initialized via NewRequirement constructor for creating a valid Requirement.
 type Requirement struct {
-	key       string
-	operator  selection.Operator
-	strValues sets.String
+	key      string
+	operator selection.Operator
+	// In huge majority of cases we have at most one value here.
+	// It is generally faster to operate on a single-element slice
+	// than on a single-element map, so we have a slice here.
+	strValues []string
 }
 
 // NewRequirement is the constructor for a Requirement.
@@ -107,7 +110,7 @@ type Requirement struct {
 //     of characters. See validateLabelKey for more details.
 //
 // The empty string is a valid value in the input values set.
-func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requirement, error) {
+func NewRequirement(key string, op selection.Operator, vals []string) (*Requirement, error) {
 	if err := validateLabelKey(key); err != nil {
 		return nil, err
 	}
@@ -128,8 +131,8 @@ func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requi
 		if len(vals) != 1 {
 			return nil, fmt.Errorf("for 'Gt', 'Lt' operators, exactly one value is required")
 		}
-		for val := range vals {
-			if _, err := strconv.ParseInt(val, 10, 64); err != nil {
+		for i := range vals {
+			if _, err := strconv.ParseInt(vals[i], 10, 64); err != nil {
 				return nil, fmt.Errorf("for 'Gt', 'Lt' operators, the value must be an integer")
 			}
 		}
@@ -137,14 +140,24 @@ func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requi
 		return nil, fmt.Errorf("operator '%v' is not recognized", op)
 	}
 
-	for v := range vals {
-		if err := validateLabelValue(v); err != nil {
+	for i := range vals {
+		if err := validateLabelValue(vals[i]); err != nil {
 			return nil, err
 		}
 	}
+	sort.Strings(vals)
 	return &Requirement{key: key, operator: op, strValues: vals}, nil
 }
 
+func (r *Requirement) hasValue(value string) bool {
+	for i := range r.strValues {
+		if r.strValues[i] == value {
+			return true
+		}
+	}
+	return false
+}
+
 // Matches returns true if the Requirement matches the input Labels.
 // There is a match in the following cases:
 // (1) The operator is Exists and Labels has the Requirement's key.
@@ -162,12 +175,12 @@ func (r *Requirement) Matches(ls Labels) bool {
 		if !ls.Has(r.key) {
 			return false
 		}
-		return r.strValues.Has(ls.Get(r.key))
+		return r.hasValue(ls.Get(r.key))
 	case selection.NotIn, selection.NotEquals:
 		if !ls.Has(r.key) {
 			return true
 		}
-		return !r.strValues.Has(ls.Get(r.key))
+		return !r.hasValue(ls.Get(r.key))
 	case selection.Exists:
 		return ls.Has(r.key)
 	case selection.DoesNotExist:
@@ -189,10 +202,10 @@ func (r *Requirement) Matches(ls Labels) bool {
 		}
 
 		var rValue int64
-		for strValue := range r.strValues {
-			rValue, err = strconv.ParseInt(strValue, 10, 64)
+		for i := range r.strValues {
+			rValue, err = strconv.ParseInt(r.strValues[i], 10, 64)
 			if err != nil {
-				glog.V(10).Infof("ParseInt failed for value %+v in requirement %#v, for 'Gt', 'Lt' operators, the value must be an integer", strValue, r)
+				glog.V(10).Infof("ParseInt failed for value %+v in requirement %#v, for 'Gt', 'Lt' operators, the value must be an integer", r.strValues[i], r)
 				return false
 			}
 		}
@@ -210,8 +223,8 @@ func (r *Requirement) Operator() selection.Operator {
 }
 func (r *Requirement) Values() sets.String {
 	ret := sets.String{}
-	for k := range r.strValues {
-		ret.Insert(k)
+	for i := range r.strValues {
+		ret.Insert(r.strValues[i])
 	}
 	return ret
 }
@@ -258,9 +271,9 @@ func (r *Requirement) String() string {
 		buffer.WriteString("(")
 	}
 	if len(r.strValues) == 1 {
-		buffer.WriteString(r.strValues.List()[0])
+		buffer.WriteString(r.strValues[0])
 	} else { // only > 1 since == 0 prohibited by NewRequirement
-		buffer.WriteString(strings.Join(r.strValues.List(), ","))
+		buffer.WriteString(strings.Join(r.strValues, ","))
 	}
 
 	switch r.operator {
@@ -561,7 +574,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) {
 		return nil, err
 	}
 	if operator == selection.Exists || operator == selection.DoesNotExist { // operator found lookahead set checked
-		return NewRequirement(key, operator, nil)
+		return NewRequirement(key, operator, []string{})
 	}
 	operator, err = p.parseOperator()
 	if err != nil {
@@ -577,7 +590,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) {
 	if err != nil {
 		return nil, err
 	}
-	return NewRequirement(key, operator, values)
+	return NewRequirement(key, operator, values.List())
 
 }
 
@@ -784,7 +797,7 @@ func SelectorFromSet(ls Set) Selector {
 	}
 	var requirements internalSelector
 	for label, value := range ls {
-		if r, err := NewRequirement(label, selection.Equals, sets.NewString(value)); err != nil {
+		if r, err := NewRequirement(label, selection.Equals, []string{value}); err != nil {
 			//TODO: double check errors when input comes from serialization?
 			return internalSelector{}
 		} else {
@@ -805,7 +818,7 @@ func SelectorFromValidatedSet(ls Set) Selector {
 	}
 	var requirements internalSelector
 	for label, value := range ls {
-		requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: sets.NewString(value)})
+		requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: []string{value}})
 	}
 	// sort to have deterministic string representation
 	sort.Sort(ByKey(requirements))

pkg/labels/selector_test.go

@@ -320,7 +320,7 @@ func TestRequirementConstructor(t *testing.T) {
 		{strings.Repeat("a", 254), selection.Exists, nil, false}, //breaks DNS rule that len(key) <= 253
 	}
 	for _, rc := range requirementConstructorTests {
-		if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals); err == nil && !rc.Success {
+		if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals.List()); err == nil && !rc.Success {
 			t.Errorf("expected error with key:%#v op:%v vals:%v, got no error", rc.Key, rc.Op, rc.Vals)
 		} else if err != nil && rc.Success {
 			t.Errorf("expected no error with key:%#v op:%v vals:%v, got:%v", rc.Key, rc.Op, rc.Vals, err)
@@ -525,7 +525,7 @@ func TestSetSelectorParser(t *testing.T) {
 }
 
 func getRequirement(key string, op selection.Operator, vals sets.String, t *testing.T) Requirement {
-	req, err := NewRequirement(key, op, vals)
+	req, err := NewRequirement(key, op, vals.List())
 	if err != nil {
 		t.Errorf("NewRequirement(%v, %v, %v) resulted in error:%v", key, op, vals, err)
 		return Requirement{}
@@ -548,22 +548,22 @@ func TestAdd(t *testing.T) {
 			"key",
 			selection.In,
 			[]string{"value"},
-			internalSelector{Requirement{"key", selection.In, sets.NewString("value")}},
+			internalSelector{Requirement{"key", selection.In, []string{"value"}}},
 		},
 		{
 			"keyEqualsOperator",
-			internalSelector{Requirement{"key", selection.In, sets.NewString("value")}},
+			internalSelector{Requirement{"key", selection.In, []string{"value"}}},
 			"key2",
 			selection.Equals,
 			[]string{"value2"},
 			internalSelector{
-				Requirement{"key", selection.In, sets.NewString("value")},
-				Requirement{"key2", selection.Equals, sets.NewString("value2")},
+				Requirement{"key", selection.In, []string{"value"}},
+				Requirement{"key2", selection.Equals, []string{"value2"}},
 			},
 		},
 	}
 	for _, ts := range testCases {
-		req, err := NewRequirement(ts.key, ts.operator, sets.NewString(ts.values...))
+		req, err := NewRequirement(ts.key, ts.operator, ts.values)
 		if err != nil {
 			t.Errorf("%s - Unable to create labels.Requirement", ts.name)
 		}

pkg/registry/generic/registry/store_test.go

@@ -43,7 +43,6 @@ import (
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
 	"k8s.io/kubernetes/pkg/storage/storagebackend/factory"
 	storagetesting "k8s.io/kubernetes/pkg/storage/testing"
-	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/util/wait"
 )
@@ -109,7 +108,7 @@ func NewTestGenericStoreRegistry(t *testing.T) (factory.DestroyFunc, *Store) {
 func matchPodName(names ...string) storage.SelectionPredicate {
 	// Note: even if pod name is a field, we have to use labels,
 	// because field selector doesn't support "IN" operator.
-	l, err := labels.NewRequirement("name", selection.In, sets.NewString(names...))
+	l, err := labels.NewRequirement("name", selection.In, names)
 	if err != nil {
 		panic("Labels requirement must validate successfully")
 	}