Skip to content

Commit

Permalink
Merge pull request #155 from jpbetz/freelist-optimization
Browse files Browse the repository at this point in the history
Use freelists instead of sync.Pool for value object recycling
  • Loading branch information
k8s-ci-robot authored Feb 6, 2020
2 parents b8515d0 + 0e517d5 commit 1791273
Show file tree
Hide file tree
Showing 20 changed files with 559 additions and 259 deletions.
34 changes: 18 additions & 16 deletions fieldpath/fromvalue.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,20 @@ func SetFromValue(v value.Value) *Set {
s := NewSet()

w := objectWalker{
path: Path{},
value: v,
do: func(p Path) { s.Insert(p) },
path: Path{},
value: v,
allocator: value.NewFreelistAllocator(),
do: func(p Path) { s.Insert(p) },
}

w.walk()
return s
}

type objectWalker struct {
path Path
value value.Value
path Path
value value.Value
allocator value.Allocator

do func(Path)
}
Expand All @@ -55,14 +57,14 @@ func (w *objectWalker) walk() {
case w.value.IsList():
// If the list were atomic, we'd break here, but we don't have
// a schema, so we can't tell.
l := w.value.AsList()
defer l.Recycle()
iter := l.Range()
defer iter.Recycle()
l := w.value.AsListUsing(w.allocator)
defer w.allocator.Free(l)
iter := l.RangeUsing(w.allocator)
defer w.allocator.Free(iter)
for iter.Next() {
i, value := iter.Item()
w2 := *w
w2.path = append(w.path, GuessBestListPathElement(i, value))
w2.path = append(w.path, w.GuessBestListPathElement(i, value))
w2.value = value
w2.walk()
}
Expand All @@ -71,9 +73,9 @@ func (w *objectWalker) walk() {
// If the map/struct were atomic, we'd break here, but we don't
// have a schema, so we can't tell.

m := w.value.AsMap()
defer m.Recycle()
m.Iterate(func(k string, val value.Value) bool {
m := w.value.AsMapUsing(w.allocator)
defer w.allocator.Free(m)
m.IterateUsing(w.allocator, func(k string, val value.Value) bool {
w2 := *w
w2.path = append(w.path, PathElement{FieldName: &k})
w2.value = val
Expand Down Expand Up @@ -102,16 +104,16 @@ var AssociativeListCandidateFieldNames = []string{
// referencing by index is acceptable. Currently this is done by checking
// whether item has any of the fields listed in
// AssociativeListCandidateFieldNames which have scalar values.
func GuessBestListPathElement(index int, item value.Value) PathElement {
func (w *objectWalker) GuessBestListPathElement(index int, item value.Value) PathElement {
if !item.IsMap() {
// Non map items could be parts of sets or regular "atomic"
// lists. We won't try to guess whether something should be a
// set or not.
return PathElement{Index: &index}
}

m := item.AsMap()
defer m.Recycle()
m := item.AsMapUsing(w.allocator)
defer w.allocator.Free(m)
var keys value.FieldList
for _, name := range AssociativeListCandidateFieldNames {
f, ok := m.Get(name)
Expand Down
18 changes: 9 additions & 9 deletions typed/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,19 +154,19 @@ func handleAtom(a schema.Atom, tr schema.TypeRef, ah atomHandler) ValidationErro
}

// Returns the list, or an error. Reminder: nil is a valid list and might be returned.
func listValue(val value.Value) (value.List, error) {
func listValue(a value.Allocator, val value.Value) (value.List, error) {
if val.IsNull() {
// Null is a valid list.
return nil, nil
}
if !val.IsList() {
return nil, fmt.Errorf("expected list, got %v", val)
}
return val.AsList(), nil
return val.AsListUsing(a), nil
}

// Returns the map, or an error. Reminder: nil is a valid map and might be returned.
func mapValue(val value.Value) (value.Map, error) {
func mapValue(a value.Allocator, val value.Value) (value.Map, error) {
if val == nil {
return nil, fmt.Errorf("expected map, got nil")
}
Expand All @@ -177,10 +177,10 @@ func mapValue(val value.Value) (value.Map, error) {
if !val.IsMap() {
return nil, fmt.Errorf("expected map, got %v", val)
}
return val.AsMap(), nil
return val.AsMapUsing(a), nil
}

func keyedAssociativeListItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
func keyedAssociativeListItemToPathElement(a value.Allocator, list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
pe := fieldpath.PathElement{}
if child.IsNull() {
// For now, the keys are required which means that null entries
Expand All @@ -191,8 +191,8 @@ func keyedAssociativeListItemToPathElement(list *schema.List, index int, child v
return pe, errors.New("associative list with keys may not have non-map elements")
}
keyMap := value.FieldList{}
m := child.AsMap()
defer m.Recycle()
m := child.AsMapUsing(a)
defer a.Free(m)
for _, fieldName := range list.Keys {
if val, ok := m.Get(fieldName); ok {
keyMap = append(keyMap, value.Field{Name: fieldName, Value: val})
Expand Down Expand Up @@ -225,10 +225,10 @@ func setItemToPathElement(list *schema.List, index int, child value.Value) (fiel
}
}

func listItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
func listItemToPathElement(a value.Allocator, list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
if list.ElementRelationship == schema.Associative {
if len(list.Keys) > 0 {
return keyedAssociativeListItemToPathElement(list, index, child)
return keyedAssociativeListItemToPathElement(a, list, index, child)
}

// If there's no keys, then we must be a set of primitives.
Expand Down
20 changes: 11 additions & 9 deletions typed/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ type mergingWalker struct {

// Allocate only as many walkers as needed for the depth by storing them here.
spareWalkers *[]*mergingWalker

allocator value.Allocator
}

// merge rules examine w.lhs and w.rhs (up to one of which may be nil) and
Expand Down Expand Up @@ -152,7 +154,7 @@ func (w *mergingWalker) derefMap(prefix string, v value.Value) (value.Map, Valid
if v == nil {
return nil, nil
}
m, err := mapValue(v)
m, err := mapValue(w.allocator, v)
if err != nil {
return nil, errorf("%v: %v", prefix, err)
}
Expand Down Expand Up @@ -181,7 +183,7 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
if rhs != nil {
for i := 0; i < rhs.Length(); i++ {
child := rhs.At(i)
pe, err := listItemToPathElement(t, i, child)
pe, err := listItemToPathElement(w.allocator, t, i, child)
if err != nil {
errs = append(errs, errorf("rhs: element %v: %v", i, err.Error())...)
// If we can't construct the path element, we can't
Expand All @@ -202,7 +204,7 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
if lhs != nil {
for i := 0; i < lhs.Length(); i++ {
child := lhs.At(i)
pe, err := listItemToPathElement(t, i, child)
pe, err := listItemToPathElement(w.allocator, t, i, child)
if err != nil {
errs = append(errs, errorf("lhs: element %v: %v", i, err.Error())...)
// If we can't construct the path element, we can't
Expand Down Expand Up @@ -254,7 +256,7 @@ func (w *mergingWalker) derefList(prefix string, v value.Value) (value.List, Val
if v == nil {
return nil, nil
}
l, err := listValue(v)
l, err := listValue(w.allocator, v)
if err != nil {
return nil, errorf("%v: %v", prefix, err)
}
Expand All @@ -264,11 +266,11 @@ func (w *mergingWalker) derefList(prefix string, v value.Value) (value.List, Val
func (w *mergingWalker) doList(t *schema.List) (errs ValidationErrors) {
lhs, _ := w.derefList("lhs: ", w.lhs)
if lhs != nil {
defer lhs.Recycle()
defer w.allocator.Free(lhs)
}
rhs, _ := w.derefList("rhs: ", w.rhs)
if rhs != nil {
defer rhs.Recycle()
defer w.allocator.Free(rhs)
}

// If both lhs and rhs are empty/null, treat it as a
Expand Down Expand Up @@ -310,7 +312,7 @@ func (w *mergingWalker) visitMapItem(t *schema.Map, out map[string]interface{},
func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs ValidationErrors) {
out := map[string]interface{}{}

value.MapZip(lhs, rhs, value.Unordered, func(key string, lhsValue, rhsValue value.Value) bool {
value.MapZipUsing(w.allocator, lhs, rhs, value.Unordered, func(key string, lhsValue, rhsValue value.Value) bool {
errs = append(errs, w.visitMapItem(t, out, key, lhsValue, rhsValue)...)
return true
})
Expand All @@ -325,11 +327,11 @@ func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs V
func (w *mergingWalker) doMap(t *schema.Map) (errs ValidationErrors) {
lhs, _ := w.derefMap("lhs: ", w.lhs)
if lhs != nil {
defer lhs.Recycle()
defer w.allocator.Free(lhs)
}
rhs, _ := w.derefMap("rhs: ", w.rhs)
if rhs != nil {
defer rhs.Recycle()
defer w.allocator.Free(rhs)
}
// If both lhs and rhs are empty/null, treat it as a
// leaf: this helps preserve the empty/null
Expand Down
30 changes: 16 additions & 14 deletions typed/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,19 @@ import (
)

type removingWalker struct {
value value.Value
out interface{}
schema *schema.Schema
toRemove *fieldpath.Set
value value.Value
out interface{}
schema *schema.Schema
toRemove *fieldpath.Set
allocator value.Allocator
}

func removeItemsWithSchema(val value.Value, toRemove *fieldpath.Set, schema *schema.Schema, typeRef schema.TypeRef) value.Value {
w := &removingWalker{
value: val,
schema: schema,
toRemove: toRemove,
value: val,
schema: schema,
toRemove: toRemove,
allocator: value.NewFreelistAllocator(),
}
resolveSchema(schema, typeRef, val, w)
return value.NewValueInterface(w.out)
Expand All @@ -42,20 +44,20 @@ func (w *removingWalker) doScalar(t *schema.Scalar) ValidationErrors {
}

func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
l := w.value.AsList()
defer l.Recycle()
l := w.value.AsListUsing(w.allocator)
defer w.allocator.Free(l)
// If list is null, empty, or atomic just return
if l == nil || l.Length() == 0 || t.ElementRelationship == schema.Atomic {
return nil
}

var newItems []interface{}
iter := l.Range()
defer iter.Recycle()
iter := l.RangeUsing(w.allocator)
defer w.allocator.Free(iter)
for iter.Next() {
i, item := iter.Item()
// Ignore error because we have already validated this list
pe, _ := listItemToPathElement(t, i, item)
pe, _ := listItemToPathElement(w.allocator, t, i, item)
path, _ := fieldpath.MakePath(pe)
if w.toRemove.Has(path) {
continue
Expand All @@ -72,9 +74,9 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
}

func (w *removingWalker) doMap(t *schema.Map) ValidationErrors {
m := w.value.AsMap()
m := w.value.AsMapUsing(w.allocator)
if m != nil {
defer m.Recycle()
defer w.allocator.Free(m)
}
// If map is null, empty, or atomic just return
if m == nil || m.Empty() || t.ElementRelationship == schema.Atomic {
Expand Down
14 changes: 9 additions & 5 deletions typed/tofieldset.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func (tv TypedValue) toFieldSetWalker() *toFieldSetWalker {
v.schema = tv.schema
v.typeRef = tv.typeRef
v.set = &fieldpath.Set{}
v.allocator = value.NewFreelistAllocator()
return v
}

Expand All @@ -55,6 +56,7 @@ type toFieldSetWalker struct {

// Allocate only as many walkers as needed for the depth by storing them here.
spareWalkers *[]*toFieldSetWalker
allocator value.Allocator
}

func (v *toFieldSetWalker) prepareDescent(pe fieldpath.PathElement, tr schema.TypeRef) *toFieldSetWalker {
Expand Down Expand Up @@ -94,7 +96,7 @@ func (v *toFieldSetWalker) doScalar(t *schema.Scalar) ValidationErrors {
func (v *toFieldSetWalker) visitListItems(t *schema.List, list value.List) (errs ValidationErrors) {
for i := 0; i < list.Length(); i++ {
child := list.At(i)
pe, _ := listItemToPathElement(t, i, child)
pe, _ := listItemToPathElement(v.allocator, t, i, child)
v2 := v.prepareDescent(pe, t.ElementType)
v2.value = child
errs = append(errs, v2.toFieldSet()...)
Expand All @@ -106,8 +108,10 @@ func (v *toFieldSetWalker) visitListItems(t *schema.List, list value.List) (errs
}

func (v *toFieldSetWalker) doList(t *schema.List) (errs ValidationErrors) {
list, _ := listValue(v.value)

list, _ := listValue(v.allocator, v.value)
if list != nil {
defer v.allocator.Free(list)
}
if t.ElementRelationship == schema.Atomic {
v.set.Insert(v.path)
return nil
Expand Down Expand Up @@ -143,9 +147,9 @@ func (v *toFieldSetWalker) visitMapItems(t *schema.Map, m value.Map) (errs Valid
}

func (v *toFieldSetWalker) doMap(t *schema.Map) (errs ValidationErrors) {
m, _ := mapValue(v.value)
m, _ := mapValue(v.allocator, v.value)
if m != nil {
defer m.Recycle()
defer v.allocator.Free(m)
}
if t.ElementRelationship == schema.Atomic {
v.set.Insert(v.path)
Expand Down
3 changes: 3 additions & 0 deletions typed/typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ func merge(lhs, rhs *TypedValue, rule, postRule mergeRule) (*TypedValue, error)
mw.typeRef = lhs.typeRef
mw.rule = rule
mw.postItemHook = postRule
if mw.allocator == nil {
mw.allocator = value.NewFreelistAllocator()
}

errs := mw.merge(nil)
if len(errs) > 0 {
Expand Down
Loading

0 comments on commit 1791273

Please sign in to comment.