Skip to content

Commit

Permalink
MapStr accessor optimisations (#6837)
Browse files Browse the repository at this point in the history 
* MapStr accessor optimisations

Replace walkMap + interface callbacks with mapFind function, providing
all the information required by the different accessor operations.

The mapFind function uses a while loop, searching for the next sub-key
in the key-path. This is a tail-call optimisation on walkMapRecursive.

By searching for the next sub-string and using slices, allocations on
key-paths with dots are greatly reduced.

Benchmark results:

benchmark                          old ns/op     new ns/op     delta
BenchmarkWalkMap/Get-4             1125          30.8          -97.26%
BenchmarkWalkMap/Put-4             890           458           -48.54%
BenchmarkWalkMap/PutMissing-4      813           401           -50.68%
BenchmarkWalkMap/HasKey-4          565           157           -72.21%
BenchmarkWalkMap/HasKeyFirst-4     23.9          12.4          -48.12%
BenchmarkWalkMap/Delete-4          860           457           -46.86%

benchmark                          old allocs     new allocs     delta
BenchmarkWalkMap/Get-4             7              0              -100.00%
BenchmarkWalkMap/Put-4             10             4              -60.00%
BenchmarkWalkMap/PutMissing-4      10             4              -60.00%
BenchmarkWalkMap/HasKey-4          5              0              -100.00%
BenchmarkWalkMap/HasKeyFirst-4     0              0              +0.00%
BenchmarkWalkMap/Delete-4          10             4              -60.00%

benchmark                          old bytes     new bytes     delta
BenchmarkWalkMap/Get-4             432           0             -100.00%
BenchmarkWalkMap/Put-4             1120          672           -40.00%
BenchmarkWalkMap/PutMissing-4      1120          672           -40.00%
BenchmarkWalkMap/HasKey-4          160           0             -100.00%
BenchmarkWalkMap/HasKeyFirst-4     0             0             +0.00%
BenchmarkWalkMap/Delete-4          1120          672           -40.00%
  • Loading branch information
@ph
Steffen Siering authored and ph committed Apr 13, 2018
1 parent 4d96fc7 commit 41a118f
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 139 deletions.
181 changes: 69 additions & 112 deletions libbeat/common/mapstr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,28 @@ func deepUpdateValue(old interface{}, val MapStr) interface{} {

// Delete deletes the given key from the map.
func (m MapStr) Delete(key string) error {
_, err := walkMap(key, m, opDelete)
return err
k, d, _, found, err := mapFind(key, m, false)
if err != nil {
return err
}
if !found {
return ErrKeyNotFound
}

delete(d, k)
return nil
}

// CopyFieldsTo copies the field specified by key to the given map. It will
// overwrite the key if it exists. An error is returned if the key does not
// exist in the source map.
func (m MapStr) CopyFieldsTo(to MapStr, key string) error {
v, err := walkMap(key, m, opGet)
v, err := m.GetValue(key)
if err != nil {
return err
}

_, err = walkMap(key, to, mapStrOperation{putOperation{v}, true})
_, err = to.Put(key, v)
return err
}

Expand All @@ -115,18 +123,21 @@ func (m MapStr) Clone() MapStr {
// HasKey returns true if the key exist. If an error occurs then false is
// returned with a non-nil error.
func (m MapStr) HasKey(key string) (bool, error) {
hasKey, err := walkMap(key, m, opHasKey)
if err != nil {
return false, err
}

return hasKey.(bool), nil
_, _, _, hasKey, err := mapFind(key, m, false)
return hasKey, err
}

// GetValue gets a value from the map. If the key does not exist then an error
// is returned.
func (m MapStr) GetValue(key string) (interface{}, error) {
return walkMap(key, m, opGet)
_, _, v, found, err := mapFind(key, m, false)
if err != nil {
return nil, err
}
if !found {
return nil, ErrKeyNotFound
}
return v, nil
}

// Put associates the specified value with the specified key. If the map
Expand All @@ -138,7 +149,13 @@ func (m MapStr) GetValue(key string) (interface{}, error) {
// to insert values (e.g. m[key] = value).
func (m MapStr) Put(key string, value interface{}) (interface{}, error) {
// XXX `safemapstr.Put` mimics this implementation, both should be updated to have similar behavior
return walkMap(key, m, mapStrOperation{putOperation{value}, true})
k, d, old, _, err := mapFind(key, m, true)
if err != nil {
return nil, err
}

d[k] = value
return old, nil
}

// StringToPrint returns the MapStr as pretty JSON.
Expand Down Expand Up @@ -316,110 +333,50 @@ func tryToMapStr(v interface{}) (MapStr, bool) {
}
}

// walkMapRecursive walks the data MapStr to arrive at the value specified by the key.
// The key is expressed in dot-notation (eg. x.y.z). When the key is found then
// the given mapStrOperation is invoked.
func walkMapRecursive(key string, data MapStr, op mapStrOperation) (interface{}, error) {

// Splits up the key in two parts: full key and first part before the dot
_, exists := data[key]
// If leave node or key exists directly
if exists {
// Execute the mapStrOperator on the leaf object.
return op.Do(key, data)
}

keyParts := strings.SplitN(key, ".", 2)
// If leave node or key exists directly
if len(keyParts) == 1 {
// Execute the mapStrOperator on the leaf object.
return op.Do(key, data)
}

// Checks if first part of the key exists
k := keyParts[0]
d, keyExist := data[k]
if !keyExist {
if op.CreateMissingKeys {
d = MapStr{}
data[k] = d
} else {
return nil, ErrKeyNotFound
// mapFind iterates a MapStr based on a the given dotted key, finding the final
// subMap and subKey to operate on.
// An error is returned if some intermediate is no map or the key doesn't exist.
// If createMissing is set to true, intermediate maps are created.
// The final map and un-dotted key to run further operations on are returned in
// subKey and subMap. The subMap already contains a value for subKey, the
// present flag is set to true and the oldValue return will hold
// the original value.
func mapFind(
key string,
data MapStr,
createMissing bool,
) (subKey string, subMap MapStr, oldValue interface{}, present bool, err error) {
// XXX `safemapstr.mapFind` mimics this implementation, both should be updated to have similar behavior

for {
// Fast path, key is present as is.
if v, exists := data[key]; exists {
return key, data, v, true, nil
}
}

v, err := toMapStr(d)
if err != nil {
return nil, err
}

return walkMapRecursive(keyParts[1], v, op)
}

func walkMap(key string, data MapStr, op mapStrOperation) (interface{}, error) {
v, err := walkMapRecursive(key, data, op)
if err != nil {
// Add key to error
err = errors.Wrapf(err, "key=%v", key)
}
return v, err
}

// mapStrOperation types

// These are static mapStrOperation types that store no state and are reusable.
var (
opDelete = mapStrOperation{deleteOperation{}, false}
opGet = mapStrOperation{getOperation{}, false}
opHasKey = mapStrOperation{hasKeyOperation{}, false}
)

// mapStrOperation represents an operation that can be applied to map.
type mapStrOperation struct {
mapStrOperator
CreateMissingKeys bool
}

// mapStrOperator is an interface with a single function that performs an
// operation on a MapStr.
type mapStrOperator interface {
Do(key string, data MapStr) (value interface{}, err error)
}

type deleteOperation struct{}
idx := strings.IndexRune(key, '.')
if idx < 0 {
return key, data, nil, false, nil
}

func (op deleteOperation) Do(key string, data MapStr) (interface{}, error) {
value, found := data[key]
if !found {
return nil, ErrKeyNotFound
}
delete(data, key)
return value, nil
}
k := key[:idx]
d, exists := data[k]
if !exists {
if createMissing {
d = MapStr{}
data[k] = d
} else {
return "", nil, nil, false, ErrKeyNotFound
}
}

type getOperation struct{}
v, err := toMapStr(d)
if err != nil {
return "", nil, nil, false, err
}

func (op getOperation) Do(key string, data MapStr) (interface{}, error) {
value, found := data[key]
if !found {
return nil, ErrKeyNotFound
// advance to sub-map
key = key[idx+1:]
data = v
}
return value, nil
}

type hasKeyOperation struct{}

func (op hasKeyOperation) Do(key string, data MapStr) (interface{}, error) {
_, found := data[key]
return found, nil
}

type putOperation struct {
Value interface{}
}

func (op putOperation) Do(key string, data MapStr) (interface{}, error) {
existingValue, _ := data[key]
data[key] = op.Value
return existingValue, nil
}
81 changes: 54 additions & 27 deletions libbeat/common/safemapstr/safemapstr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,39 +22,66 @@ const alternativeKey = "value"
// `.value`
func Put(data common.MapStr, key string, value interface{}) error {
// XXX This implementation mimics `common.MapStr.Put`, both should be updated to have similar behavior
keyParts := strings.SplitN(key, ".", 2)

// If leaf node or key exists directly
if len(keyParts) == 1 {
oldValue, exists := data[key]
if exists {
oldMap, ok := tryToMapStr(oldValue)
if ok {
// This would replace a whole object, change its key to avoid that:
oldMap[alternativeKey] = value
return nil
d, k := mapFind(data, key, alternativeKey)
d[k] = value
return nil
}

// mapFind walk the map based on the given dotted key and returns the final map
// and key to operate on. This function adds intermediate maps, if the key is
// missing from the original map.

// mapFind iterates a MapStr based on the given dotted key, finding the final
// subMap and subKey to operate on.
// If a key is already used, but the used value is no map, an intermediate map will be inserted and
// the old value will be stored using the 'alternativeKey' in a new map.
// If the old value found under key is already an dictionary, subMap will be
// the old value and subKey will be set to alternativeKey.
func mapFind(data common.MapStr, key, alternativeKey string) (subMap common.MapStr, subKey string) {
// XXX This implementation mimics `common.mapFind`, both should be updated to have similar behavior

for {
if oldValue, exists := data[key]; exists {
if oldMap, ok := tryToMapStr(oldValue); ok {
return oldMap, alternativeKey
}
return data, key
}
data[key] = value
return nil
}

// Checks if first part of the key exists
k := keyParts[0]
d, exists := data[k]
if !exists {
d = common.MapStr{}
data[k] = d
}
idx := strings.IndexRune(key, '.')
if idx < 0 {
// if old value exists and is a dictionary, return the old dictionary and
// make sure we store the new value using the 'alternativeKey'
if oldValue, exists := data[key]; exists {
if oldMap, ok := tryToMapStr(oldValue); ok {
return oldMap, alternativeKey
}
}

v, ok := tryToMapStr(d)
if !ok {
// This would replace a leaf with an object, change its key to avoid that:
v = common.MapStr{alternativeKey: d}
data[k] = v
}
return data, key
}

// Check if first sub-key exists. Create an intermediate map if not.
k := key[:idx]
d, exists := data[k]
if !exists {
d = common.MapStr{}
data[k] = d
}

return Put(v, keyParts[1], value)
// store old value under 'alternativeKey' if the old value is no map.
// Do not overwrite old value.
v, ok := tryToMapStr(d)
if !ok {
v = common.MapStr{alternativeKey: d}
data[k] = v
}

// advance into sub-map
key = key[idx+1:]
data = v
}
}

func tryToMapStr(v interface{}) (common.MapStr, bool) {
Expand Down

0 comments on commit 41a118f

Please sign in to comment.