Increase performance of nested keys search

* Fixed: insensitiviseMaps and tests

All keys (even nested ones) are now lower-cased recursively.

On the way, map[interface{}]interface{} are cast to map[string]interface{}

* Changed: simplified find() fast path and increase performance

Removed searchMapForKey(), fast path directly integrated into searchMap() and
searchMapWithPathPrefixes()
=> more generic (searchMapForKey() wasn't called everywhere it should have)

At the same time, significantly speed up searchMap() and searchMapWithPathPrefixes(),
which are still used for nested keys: the assumption that map keys are all
lower-cased allows to perform
    val = m[key]
instead of
    for k, v := range m {
      if strings.ToLower(k) == strings.ToLower(key) {
        val = v
      }
    }
=> i.e., directly access the map instead of enumerate the keys
This commit is contained in:
Benoît Masson 2016-10-14 11:24:45 +02:00 committed by Bjørn Erik Pedersen
parent 44208030b3
commit 50515b700e
3 changed files with 46 additions and 74 deletions

18
util.go
View file

@ -41,15 +41,23 @@ func (pe ConfigParseError) Error() string {
func insensitiviseMap(m map[string]interface{}) { func insensitiviseMap(m map[string]interface{}) {
for key, val := range m { for key, val := range m {
switch val.(type) {
case map[interface{}]interface{}:
// nested map: cast and recursively insensitivise
val = cast.ToStringMap(val)
insensitiviseMap(val.(map[string]interface{}))
case map[string]interface{}:
// nested map: recursively insensitivise
insensitiviseMap(val.(map[string]interface{}))
}
lower := strings.ToLower(key) lower := strings.ToLower(key)
if key != lower { if key != lower {
// remove old key (not lower-cased)
delete(m, key) delete(m, key)
}
// update map
m[lower] = val m[lower] = val
if m2, ok := val.(map[string]interface{}); ok {
// nested map: recursively insensitivise
insensitiviseMap(m2)
}
}
} }
} }

View file

@ -405,49 +405,22 @@ func (v *Viper) providerPathExists(p *defaultRemoteProvider) bool {
return false return false
} }
// searchMapForKey may end up traversing the map if the key references a nested
// item (foo.bar), but will use a fast path for the common case.
// Note: This assumes that the key given is already lowercase.
func (v *Viper) searchMapForKey(source map[string]interface{}, lcaseKey string) interface{} {
if !strings.Contains(lcaseKey, v.keyDelim) {
v, ok := source[lcaseKey]
if ok {
return v
}
return nil
}
path := strings.Split(lcaseKey, v.keyDelim)
return v.searchMap(source, path)
}
// searchMap recursively searches for a value for path in source map. // searchMap recursively searches for a value for path in source map.
// Returns nil if not found. // Returns nil if not found.
// Note: This assumes that the path entries are lower cased. // Note: This assumes that the path entries and map keys are lower cased.
func (v *Viper) searchMap(source map[string]interface{}, path []string) interface{} { func (v *Viper) searchMap(source map[string]interface{}, path []string) interface{} {
if len(path) == 0 { if len(path) == 0 {
return source return source
} }
next, ok := source[path[0]]
if ok {
// Fast path // Fast path
if len(path) == 1 { if len(path) == 1 {
if v, ok := source[path[0]]; ok { return next
return v
}
return nil
} }
var ok bool // Nested case
var next interface{}
for k, v := range source {
if k == path[0] {
ok = true
next = v
break
}
}
if ok {
switch next.(type) { switch next.(type) {
case map[interface{}]interface{}: case map[interface{}]interface{}:
return v.searchMap(cast.ToStringMap(next), path[1:]) return v.searchMap(cast.ToStringMap(next), path[1:])
@ -456,9 +429,6 @@ func (v *Viper) searchMap(source map[string]interface{}, path []string) interfac
// if the type of `next` is the same as the type being asserted // if the type of `next` is the same as the type being asserted
return v.searchMap(next.(map[string]interface{}), path[1:]) return v.searchMap(next.(map[string]interface{}), path[1:])
default: default:
if len(path) == 1 {
return next
}
// got a value but nested key expected, return "nil" for not found // got a value but nested key expected, return "nil" for not found
return nil return nil
} }
@ -475,6 +445,8 @@ func (v *Viper) searchMap(source map[string]interface{}, path []string) interfac
// //
// This should be useful only at config level (other maps may not contain dots // This should be useful only at config level (other maps may not contain dots
// in their keys). // in their keys).
//
// Note: This assumes that the path entries and map keys are lower cased.
func (v *Viper) searchMapWithPathPrefixes(source map[string]interface{}, path []string) interface{} { func (v *Viper) searchMapWithPathPrefixes(source map[string]interface{}, path []string) interface{} {
if len(path) == 0 { if len(path) == 0 {
return source return source
@ -484,17 +456,14 @@ func (v *Viper) searchMapWithPathPrefixes(source map[string]interface{}, path []
for i := len(path); i > 0; i-- { for i := len(path); i > 0; i-- {
prefixKey := strings.ToLower(strings.Join(path[0:i], v.keyDelim)) prefixKey := strings.ToLower(strings.Join(path[0:i], v.keyDelim))
var ok bool next, ok := source[prefixKey]
var next interface{} if ok {
for k, v := range source { // Fast path
if strings.ToLower(k) == prefixKey { if i == len(path) {
ok = true return next
next = v
break
}
} }
if ok { // Nested case
var val interface{} var val interface{}
switch next.(type) { switch next.(type) {
case map[interface{}]interface{}: case map[interface{}]interface{}:
@ -504,9 +473,6 @@ func (v *Viper) searchMapWithPathPrefixes(source map[string]interface{}, path []
// if the type of `next` is the same as the type being asserted // if the type of `next` is the same as the type being asserted
val = v.searchMapWithPathPrefixes(next.(map[string]interface{}), path[i:]) val = v.searchMapWithPathPrefixes(next.(map[string]interface{}), path[i:])
default: default:
if len(path) == i {
val = next
}
// got a value but nested key expected, do nothing and look for next prefix // got a value but nested key expected, do nothing and look for next prefix
} }
if val != nil { if val != nil {
@ -626,7 +592,8 @@ func (v *Viper) Get(key string) interface{} {
valType := val valType := val
if v.typeByDefValue { if v.typeByDefValue {
// TODO(bep) this branch isn't covered by a single test. // TODO(bep) this branch isn't covered by a single test.
defVal := v.searchMapForKey(v.defaults, lcaseKey) path := strings.Split(lcaseKey, v.keyDelim)
defVal := v.searchMap(v.defaults, path)
if defVal != nil { if defVal != nil {
valType = defVal valType = defVal
} }
@ -889,16 +856,14 @@ func (v *Viper) find(lcaseKey string) interface{} {
// if the requested key is an alias, then return the proper key // if the requested key is an alias, then return the proper key
lcaseKey = v.realKey(lcaseKey) lcaseKey = v.realKey(lcaseKey)
// Set() override first
val = v.searchMapForKey(v.override, lcaseKey)
if val != nil {
return val
}
path = strings.Split(lcaseKey, v.keyDelim) path = strings.Split(lcaseKey, v.keyDelim)
nested = len(path) > 1 nested = len(path) > 1
// Set() override first
val = v.searchMap(v.override, path)
if val != nil {
return val
}
if nested && v.isPathShadowedInDeepMap(path, v.override) != "" { if nested && v.isPathShadowedInDeepMap(path, v.override) != "" {
return nil return nil
} }
@ -918,7 +883,6 @@ func (v *Viper) find(lcaseKey string) interface{} {
return flag.ValueString() return flag.ValueString()
} }
} }
if nested && v.isPathShadowedInFlatMap(path, v.pflags) != "" { if nested && v.isPathShadowedInFlatMap(path, v.pflags) != "" {
return nil return nil
} }
@ -940,7 +904,7 @@ func (v *Viper) find(lcaseKey string) interface{} {
return val return val
} }
} }
if shadow := v.isPathShadowedInFlatMap(path, v.env); shadow != "" { if nested && v.isPathShadowedInFlatMap(path, v.env) != "" {
return nil return nil
} }
@ -949,7 +913,7 @@ func (v *Viper) find(lcaseKey string) interface{} {
if val != nil { if val != nil {
return val return val
} }
if shadow := v.isPathShadowedInDeepMap(path, v.config); shadow != "" { if nested && v.isPathShadowedInDeepMap(path, v.config) != "" {
return nil return nil
} }
@ -958,7 +922,7 @@ func (v *Viper) find(lcaseKey string) interface{} {
if val != nil { if val != nil {
return val return val
} }
if shadow := v.isPathShadowedInDeepMap(path, v.kvstore); shadow != "" { if nested && v.isPathShadowedInDeepMap(path, v.kvstore) != "" {
return nil return nil
} }
@ -967,7 +931,7 @@ func (v *Viper) find(lcaseKey string) interface{} {
if val != nil { if val != nil {
return val return val
} }
if shadow := v.isPathShadowedInDeepMap(path, v.defaults); shadow != "" { if nested && v.isPathShadowedInDeepMap(path, v.defaults) != "" {
return nil return nil
} }

View file

@ -271,7 +271,7 @@ func TestUnmarshalling(t *testing.T) {
assert.False(t, InConfig("state")) assert.False(t, InConfig("state"))
assert.Equal(t, "steve", Get("name")) assert.Equal(t, "steve", Get("name"))
assert.Equal(t, []interface{}{"skateboarding", "snowboarding", "go"}, Get("hobbies")) assert.Equal(t, []interface{}{"skateboarding", "snowboarding", "go"}, Get("hobbies"))
assert.Equal(t, map[string]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[interface{}]interface{}{"size": "large"}}, Get("clothing")) assert.Equal(t, map[string]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[string]interface{}{"size": "large"}}, Get("clothing"))
assert.Equal(t, 35, Get("age")) assert.Equal(t, 35, Get("age"))
} }
@ -639,10 +639,10 @@ func TestFindsNestedKeys(t *testing.T) {
"age": 35, "age": 35,
"owner": map[string]interface{}{ "owner": map[string]interface{}{
"organization": "MongoDB", "organization": "MongoDB",
"Bio": "MongoDB Chief Developer Advocate & Hacker at Large", "bio": "MongoDB Chief Developer Advocate & Hacker at Large",
"dob": dob, "dob": dob,
}, },
"owner.Bio": "MongoDB Chief Developer Advocate & Hacker at Large", "owner.bio": "MongoDB Chief Developer Advocate & Hacker at Large",
"type": "donut", "type": "donut",
"id": "0001", "id": "0001",
"name": "Cake", "name": "Cake",
@ -651,7 +651,7 @@ func TestFindsNestedKeys(t *testing.T) {
"clothing": map[string]interface{}{ "clothing": map[string]interface{}{
"jacket": "leather", "jacket": "leather",
"trousers": "denim", "trousers": "denim",
"pants": map[interface{}]interface{}{ "pants": map[string]interface{}{
"size": "large", "size": "large",
}, },
}, },
@ -697,7 +697,7 @@ func TestReadBufConfig(t *testing.T) {
assert.False(t, v.InConfig("state")) assert.False(t, v.InConfig("state"))
assert.Equal(t, "steve", v.Get("name")) assert.Equal(t, "steve", v.Get("name"))
assert.Equal(t, []interface{}{"skateboarding", "snowboarding", "go"}, v.Get("hobbies")) assert.Equal(t, []interface{}{"skateboarding", "snowboarding", "go"}, v.Get("hobbies"))
assert.Equal(t, map[string]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[interface{}]interface{}{"size": "large"}}, v.Get("clothing")) assert.Equal(t, map[string]interface{}{"jacket": "leather", "trousers": "denim", "pants": map[string]interface{}{"size": "large"}}, v.Get("clothing"))
assert.Equal(t, 35, v.Get("age")) assert.Equal(t, 35, v.Get("age"))
} }