Merge pull request #3992 from svanharmelen/f-change-sets

core: change set internals and make (extreme) performance improvements

helper/schema/field_reader_config.go

@@ -18,10 +18,12 @@ type ConfigFieldReader struct {
 	Config *terraform.ResourceConfig
 	Schema map[string]*Schema
 
-	lock sync.Mutex
+	indexMaps map[string]map[string]int
+	once      sync.Once
 }
 
 func (r *ConfigFieldReader) ReadField(address []string) (FieldReadResult, error) {
+	r.once.Do(func() { r.indexMaps = make(map[string]map[string]int) })
 	return r.readField(address, false)
 }
 
@@ -55,20 +57,18 @@ func (r *ConfigFieldReader) readField(
 				continue
 			}
 
-			// Get the code
+			indexMap, ok := r.indexMaps[strings.Join(address[:i+1], ".")]
-			code, err := strconv.ParseInt(address[i+1], 0, 0)
+			if !ok {
-			if err != nil {
-				return FieldReadResult{}, err
-			}
-
 				// Get the set so we can get the index map that tells us the
 				// mapping of the hash code to the list index
-			_, indexMap, err := r.readSet(address[:i+1], v)
+				_, err := r.readSet(address[:i+1], v)
 				if err != nil {
 					return FieldReadResult{}, err
 				}
+				indexMap = r.indexMaps[strings.Join(address[:i+1], ".")]
+			}
 
-			index, ok := indexMap[int(code)]
+			index, ok := indexMap[address[i+1]]
 			if !ok {
 				return FieldReadResult{}, nil
 			}
@@ -87,8 +87,7 @@ func (r *ConfigFieldReader) readField(
 	case TypeMap:
 		return r.readMap(k)
 	case TypeSet:
-		result, _, err := r.readSet(address, schema)
+		return r.readSet(address, schema)
-		return result, err
 	case typeObject:
 		return readObjectField(
 			&nestedConfigFieldReader{r},
@@ -112,7 +111,7 @@ func (r *ConfigFieldReader) readMap(k string) (FieldReadResult, error) {
 	switch m := mraw.(type) {
 	case []interface{}:
 		for i, innerRaw := range m {
-			for ik, _ := range innerRaw.(map[string]interface{}) {
+			for ik := range innerRaw.(map[string]interface{}) {
 				key := fmt.Sprintf("%s.%d.%s", k, i, ik)
 				if r.Config.IsComputed(key) {
 					computed = true
@@ -125,7 +124,7 @@ func (r *ConfigFieldReader) readMap(k string) (FieldReadResult, error) {
 		}
 	case []map[string]interface{}:
 		for i, innerRaw := range m {
-			for ik, _ := range innerRaw {
+			for ik := range innerRaw {
 				key := fmt.Sprintf("%s.%d.%s", k, i, ik)
 				if r.Config.IsComputed(key) {
 					computed = true
@@ -137,7 +136,7 @@ func (r *ConfigFieldReader) readMap(k string) (FieldReadResult, error) {
 			}
 		}
 	case map[string]interface{}:
-		for ik, _ := range m {
+		for ik := range m {
 			key := fmt.Sprintf("%s.%s", k, ik)
 			if r.Config.IsComputed(key) {
 				computed = true
@@ -198,17 +197,17 @@ func (r *ConfigFieldReader) readPrimitive(
 }
 
 func (r *ConfigFieldReader) readSet(
-	address []string, schema *Schema) (FieldReadResult, map[int]int, error) {
+	address []string, schema *Schema) (FieldReadResult, error) {
-	indexMap := make(map[int]int)
+	indexMap := make(map[string]int)
 	// Create the set that will be our result
 	set := schema.ZeroValue().(*Set)
 
 	raw, err := readListField(&nestedConfigFieldReader{r}, address, schema)
 	if err != nil {
-		return FieldReadResult{}, indexMap, err
+		return FieldReadResult{}, err
 	}
 	if !raw.Exists {
-		return FieldReadResult{Value: set}, indexMap, nil
+		return FieldReadResult{Value: set}, nil
 	}
 
 	// If the list is computed, the set is necessarilly computed
@@ -217,7 +216,7 @@ func (r *ConfigFieldReader) readSet(
 			Value:    set,
 			Exists:   true,
 			Computed: raw.Computed,
-		}, indexMap, nil
+		}, nil
 	}
 
 	// Build up the set from the list elements
@@ -226,19 +225,16 @@ func (r *ConfigFieldReader) readSet(
 		computed := r.hasComputedSubKeys(
 			fmt.Sprintf("%s.%d", strings.Join(address, "."), i), schema)
 
-		code := set.add(v)
+		code := set.add(v, computed)
 		indexMap[code] = i
-		if computed {
-			set.m[-code] = set.m[code]
-			delete(set.m, code)
-			code = -code
-		}
 	}
 
+	r.indexMaps[strings.Join(address, ".")] = indexMap
+
 	return FieldReadResult{
 		Value:  set,
 		Exists: true,
-	}, indexMap, nil
+	}, nil
 }
 
 // hasComputedSubKeys walks through a schema and returns whether or not the

helper/schema/field_reader_config_test.go

@@ -228,8 +228,8 @@ func TestConfigFieldReader_ComputedSet(t *testing.T) {
 		"set, normal": {
 			[]string{"strSet"},
 			FieldReadResult{
-				Value: map[int]interface{}{
+				Value: map[string]interface{}{
-					2356372769: "foo",
+					"2356372769": "foo",
 				},
 				Exists:   true,
 				Computed: false,

helper/schema/field_writer_map.go

@@ -298,8 +298,7 @@ func (w *MapFieldWriter) setSet(
 	}
 
 	for code, elem := range value.(*Set).m {
-		codeStr := strconv.FormatInt(int64(code), 10)
+		if err := w.set(append(addrCopy, code), elem); err != nil {
-		if err := w.set(append(addrCopy, codeStr), elem); err != nil {
 			return err
 		}
 	}

helper/schema/resource_data.go

@@ -228,7 +228,7 @@ func (d *ResourceData) State() *terraform.InstanceState {
 	// attribute set as a map[string]interface{}, write it to a MapFieldWriter,
 	// and then use that map.
 	rawMap := make(map[string]interface{})
-	for k, _ := range d.schema {
+	for k := range d.schema {
 		source := getSourceSet
 		if d.partial {
 			source = getSourceState
@@ -343,13 +343,13 @@ func (d *ResourceData) diffChange(
 }
 
 func (d *ResourceData) getChange(
-	key string,
+	k string,
 	oldLevel getSource,
 	newLevel getSource) (getResult, getResult) {
 	var parts, parts2 []string
-	if key != "" {
+	if k != "" {
-		parts = strings.Split(key, ".")
+		parts = strings.Split(k, ".")
-		parts2 = strings.Split(key, ".")
+		parts2 = strings.Split(k, ".")
 	}
 
 	o := d.get(parts, oldLevel)
@@ -374,13 +374,6 @@ func (d *ResourceData) get(addr []string, source getSource) getResult {
 		level = "state"
 	}
 
-	// Build the address of the key we're looking for and ask the FieldReader
-	for i, v := range addr {
-		if v[0] == '~' {
-			addr[i] = v[1:]
-		}
-	}
-
 	var result FieldReadResult
 	var err error
 	if exact {

helper/schema/resource_data_test.go

@@ -1509,9 +1509,9 @@ func TestResourceDataSet(t *testing.T) {
 
 			Key: "ports",
 			Value: &Set{
-				m: map[int]interface{}{
+				m: map[string]interface{}{
-					1: 1,
+					"1": 1,
-					2: 2,
+					"2": 2,
 				},
 			},
 
@@ -1546,7 +1546,7 @@ func TestResourceDataSet(t *testing.T) {
 			Err:   true,
 
 			GetKey:   "ports",
-			GetValue: []interface{}{80, 100},
+			GetValue: []interface{}{100, 80},
 		},
 
 		// #11: Set with nested set

helper/schema/schema.go

@@ -866,23 +866,16 @@ func (m schemaMap) diffSet(
 
 	// Build the list of codes that will make up our set. This is the
 	// removed codes as well as all the codes in the new codes.
-	codes := make([][]int, 2)
+	codes := make([][]string, 2)
 	codes[0] = os.Difference(ns).listCode()
 	codes[1] = ns.listCode()
 	for _, list := range codes {
 		for _, code := range list {
-			// If the code is negative (first character is -) then
-			// replace it with "~" for our computed set stuff.
-			codeStr := strconv.Itoa(code)
-			if codeStr[0] == '-' {
-				codeStr = string('~') + codeStr[1:]
-			}
-
 			switch t := schema.Elem.(type) {
 			case *Resource:
 				// This is a complex resource
 				for k2, schema := range t.Schema {
-					subK := fmt.Sprintf("%s.%s.%s", k, codeStr, k2)
+					subK := fmt.Sprintf("%s.%s.%s", k, code, k2)
 					err := m.diff(subK, schema, diff, d, true)
 					if err != nil {
 						return err
@@ -896,7 +889,7 @@ func (m schemaMap) diffSet(
 
 				// This is just a primitive element, so go through each and
 				// just diff each.
-				subK := fmt.Sprintf("%s.%s", k, codeStr)
+				subK := fmt.Sprintf("%s.%s", k, code)
 				err := m.diff(subK, &t2, diff, d, true)
 				if err != nil {
 					return err

helper/schema/set.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
+	"strconv"
 	"sync"
 
 	"github.com/hashicorp/terraform/helper/hashcode"
@@ -43,7 +44,7 @@ func HashSchema(schema *Schema) SchemaSetFunc {
 type Set struct {
 	F SchemaSetFunc
 
-	m    map[int]interface{}
+	m    map[string]interface{}
 	once sync.Once
 }
 
@@ -65,7 +66,7 @@ func CopySet(otherSet *Set) *Set {
 
 // Add adds an item to the set if it isn't already in the set.
 func (s *Set) Add(item interface{}) {
-	s.add(item)
+	s.add(item, false)
 }
 
 // Remove removes an item if it's already in the set. Idempotent.
@@ -157,13 +158,17 @@ func (s *Set) GoString() string {
 }
 
 func (s *Set) init() {
-	s.m = make(map[int]interface{})
+	s.m = make(map[string]interface{})
 }
 
-func (s *Set) add(item interface{}) int {
+func (s *Set) add(item interface{}, computed bool) string {
 	s.once.Do(s.init)
 
 	code := s.hash(item)
+	if computed {
+		code = "~" + code
+	}
+
 	if _, ok := s.m[code]; !ok {
 		s.m[code] = item
 	}
@@ -171,34 +176,34 @@ func (s *Set) add(item interface{}) int {
 	return code
 }
 
-func (s *Set) hash(item interface{}) int {
+func (s *Set) hash(item interface{}) string {
 	code := s.F(item)
 	// Always return a nonnegative hashcode.
 	if code < 0 {
-		return -code
+		code = -code
 	}
-	return code
+	return strconv.Itoa(code)
 }
 
-func (s *Set) remove(item interface{}) int {
+func (s *Set) remove(item interface{}) string {
 	s.once.Do(s.init)
 
-	code := s.F(item)
+	code := s.hash(item)
 	delete(s.m, code)
 
 	return code
 }
 
 func (s *Set) index(item interface{}) int {
-	return sort.SearchInts(s.listCode(), s.hash(item))
+	return sort.SearchStrings(s.listCode(), s.hash(item))
 }
 
-func (s *Set) listCode() []int {
+func (s *Set) listCode() []string {
 	// Sort the hash codes so the order of the list is deterministic
-	keys := make([]int, 0, len(s.m))
+	keys := make([]string, 0, len(s.m))
-	for k, _ := range s.m {
+	for k := range s.m {
 		keys = append(keys, k)
 	}
-	sort.Sort(sort.IntSlice(keys))
+	sort.Sort(sort.StringSlice(keys))
 	return keys
 }

helper/schema/set_test.go

@@ -11,7 +11,7 @@ func TestSetAdd(t *testing.T) {
 	s.Add(5)
 	s.Add(25)
 
-	expected := []interface{}{1, 5, 25}
+	expected := []interface{}{1, 25, 5}
 	actual := s.List()
 	if !reflect.DeepEqual(actual, expected) {
 		t.Fatalf("bad: %#v", actual)
@@ -101,7 +101,7 @@ func TestSetUnion(t *testing.T) {
 	union := s1.Union(s2)
 	union.Add(2)
 
-	expected := []interface{}{1, 2, 5, 25}
+	expected := []interface{}{1, 2, 25, 5}
 	actual := union.List()
 	if !reflect.DeepEqual(actual, expected) {
 		t.Fatalf("bad: %#v", actual)