Skip to content

Commit

Permalink
Change the backing data structure of VarSet from slice to map
Browse files Browse the repository at this point in the history
This will speed up most operations performed on a large set of Vars
  • Loading branch information
ian-howell authored and jbrette committed Jun 21, 2019
1 parent 8bf2052 commit b1cdf58
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pkg/accumulator/resaccumulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func MakeEmptyAccumulator() *ResAccumulator {
ra := &ResAccumulator{}
ra.resMap = resmap.New()
ra.tConfig = &config.TransformerConfig{}
ra.varSet = types.VarSet{}
ra.varSet = types.NewVarSet()
return ra
}

Expand Down
56 changes: 34 additions & 22 deletions pkg/types/var.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,26 +70,45 @@ func (v *Var) defaulting() {
}
}

// VarSet is a slice of Vars where no var.Name is repeated.
// VarSet is a set of Vars where no var.Name is repeated.
type VarSet struct {
set []Var
set map[string]Var
}

// NewVarSet returns an initialized VarSet
func NewVarSet() VarSet {
return VarSet{set: map[string]Var{}}
}

// AsSlice returns the vars as a slice.
func (vs *VarSet) AsSlice() []Var {
s := make([]Var, len(vs.set))
copy(s, vs.set)
i := 0
for _, v := range vs.set {
s[i] = v
i++
}
sort.Sort(ByName(s))
return s
}

// Copy returns a copy of the var set.
func (vs *VarSet) Copy() VarSet {
return VarSet{set: vs.AsSlice()}
newSet := make(map[string]Var, len(vs.set))
for k, v := range vs.set {
newSet[k] = v
}
return VarSet{set: newSet}
}

// MergeSet absorbs other vars with error on name collision.
func (vs *VarSet) MergeSet(incoming VarSet) error {
return vs.MergeSlice(incoming.set)
for _, incomingVar := range incoming.set {
if err := vs.Merge(incomingVar); err != nil {
return err
}
}
return nil
}

// MergeSlice absorbs a Var slice with error on name collision.
Expand All @@ -111,33 +130,19 @@ func (vs *VarSet) Merge(v Var) error {
"var '%s' already encountered", v.Name)
}
v.defaulting()
vs.insert(v)
vs.set[v.Name] = v
return nil
}

func (vs *VarSet) insert(v Var) {
index := sort.Search(
len(vs.set),
func(i int) bool { return vs.set[i].Name > v.Name })
// make room
vs.set = append(vs.set, Var{})
// shift right at index.
// copy will not increase size of destination.
copy(vs.set[index+1:], vs.set[index:])
vs.set[index] = v
}

// Contains is true if the set has the other var.
func (vs *VarSet) Contains(other Var) bool {
return vs.Get(other.Name) != nil
}

// Get returns the var with the given name, else nil.
func (vs *VarSet) Get(name string) *Var {
for _, v := range vs.set {
if v.Name == name {
return &v
}
if v, found := vs.set[name]; found {
return &v
}
return nil
}
Expand All @@ -157,3 +162,10 @@ func (t *Target) GVK() gvk.Gvk {
}
return t.Gvk
}

// ByName is a sort interface which sorts Vars by name alphabetically
type ByName []Var

func (v ByName) Len() int { return len(v) }
func (v ByName) Swap(i, j int) { v[i], v[j] = v[j], v[i] }
func (v ByName) Less(i, j int) bool { return v[i].Name < v[j].Name }
35 changes: 33 additions & 2 deletions pkg/types/var_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestDefaulting(t *testing.T) {
}

func TestVarSet(t *testing.T) {
set := VarSet{}
set := NewVarSet()
vars := []Var{
{
Name: "SHELLVARS",
Expand Down Expand Up @@ -123,7 +123,7 @@ func TestVarSet(t *testing.T) {
t.Fatalf("set %v should contain var %v", set.AsSlice(), v)
}
}
set2 := VarSet{}
set2 := NewVarSet()
err = set2.MergeSet(set)
if err != nil {
t.Fatalf("unexpected err: %v", err)
Expand Down Expand Up @@ -151,3 +151,34 @@ func TestVarSet(t *testing.T) {
t.Fatalf("unexpected order in : %v", names)
}
}

func TestVarSetCopy(t *testing.T) {
set1 := NewVarSet()
vars := []Var{
{Name: "First"},
{Name: "Second"},
{Name: "Third"},
}
err := set1.MergeSlice(vars)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
// Confirm copying
set2 := set1.Copy()
for _, varInSet1 := range set1.AsSlice() {
if v := set2.Get(varInSet1.Name); v == nil {
t.Fatalf("set %v should contain a Var named %s", set2.AsSlice(), varInSet1)
} else if !set2.Contains(*v) {
t.Fatalf("set %v should contain %v", set2.AsSlice(), v)
}
}
// Confirm that the copy is deep
w := Var{Name: "Only in set2"}
set2.Merge(w)
if !set2.Contains(w) {
t.Fatalf("set %v should contain %v", set2.AsSlice(), w)
}
if set1.Contains(w) {
t.Fatalf("set %v should not contain %v", set1.AsSlice(), w)
}
}

0 comments on commit b1cdf58

Please sign in to comment.