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

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

Merged
merged 1 commit into from
Dec 4, 2015
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 26 additions & 30 deletions helper/schema/field_reader_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -55,20 +57,18 @@ func (r *ConfigFieldReader) readField(
continue
}

// Get the code
code, err := strconv.ParseInt(address[i+1], 0, 0)
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)
if err != nil {
return FieldReadResult{}, err
indexMap, ok := r.indexMaps[strings.Join(address[:i+1], ".")]
if !ok {
// Get the set so we can get the index map that tells us the
// mapping of the hash code to the list index
_, 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
}
Expand All @@ -87,8 +87,7 @@ func (r *ConfigFieldReader) readField(
case TypeMap:
return r.readMap(k)
case TypeSet:
result, _, err := r.readSet(address, schema)
return result, err
return r.readSet(address, schema)
case typeObject:
return readObjectField(
&nestedConfigFieldReader{r},
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -198,17 +197,17 @@ func (r *ConfigFieldReader) readPrimitive(
}

func (r *ConfigFieldReader) readSet(
address []string, schema *Schema) (FieldReadResult, map[int]int, error) {
indexMap := make(map[int]int)
address []string, schema *Schema) (FieldReadResult, error) {
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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions helper/schema/field_reader_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ func TestConfigFieldReader_ComputedSet(t *testing.T) {
"set, normal": {
[]string{"strSet"},
FieldReadResult{
Value: map[int]interface{}{
2356372769: "foo",
Value: map[string]interface{}{
"2356372769": "foo",
},
Exists: true,
Computed: false,
Expand Down
3 changes: 1 addition & 2 deletions helper/schema/field_writer_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, codeStr), elem); err != nil {
if err := w.set(append(addrCopy, code), elem); err != nil {
return err
}
}
Expand Down
17 changes: 5 additions & 12 deletions helper/schema/resource_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 != "" {
parts = strings.Split(key, ".")
parts2 = strings.Split(key, ".")
if k != "" {
parts = strings.Split(k, ".")
parts2 = strings.Split(k, ".")
}

o := d.get(parts, oldLevel)
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions helper/schema/resource_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1509,9 +1509,9 @@ func TestResourceDataSet(t *testing.T) {

Key: "ports",
Value: &Set{
m: map[int]interface{}{
1: 1,
2: 2,
m: map[string]interface{}{
"1": 1,
"2": 2,
},
},

Expand Down Expand Up @@ -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
Expand Down
13 changes: 3 additions & 10 deletions helper/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
33 changes: 19 additions & 14 deletions helper/schema/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"reflect"
"sort"
"strconv"
"sync"

"github.com/hashicorp/terraform/helper/hashcode"
Expand Down Expand Up @@ -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
}

Expand All @@ -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.
Expand Down Expand Up @@ -157,48 +158,52 @@ 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
}

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))
for k, _ := range s.m {
keys := make([]string, 0, len(s.m))
for k := range s.m {
keys = append(keys, k)
}
sort.Sort(sort.IntSlice(keys))
sort.Sort(sort.StringSlice(keys))
return keys
}
4 changes: 2 additions & 2 deletions helper/schema/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down