reflect: optimize for maps with string keys

Over 80% of all Go map types use a string as the key.
The Go runtime already has a specialized implementation for such maps
in runtime/map_faststr.go. However, the Go reflection implementation
has not historically made use of that implementation.

This CL plumbs the appropriate logic to be accessible from Go reflection
so that it can benefit as well.

    name                            old time/op    new time/op    delta
    Map/StringKeys/MapIndex-4       4.65us ± 5%    2.95us ± 3%  -36.50%  (p=0.016 n=4+5)
    Map/StringKeys/SetMapIndex-4    7.47us ± 5%    5.27us ± 2%  -29.40%  (p=0.008 n=5+5)
    Map/Uint64Keys/MapIndex-4       3.79us ± 3%    3.75us ± 2%     ~     (p=0.548 n=5+5)
    Map/Uint64Keys/SetMapIndex-4    6.13us ± 3%    6.09us ± 1%     ~     (p=0.746 n=5+5)

Change-Id: I5495d48948d8caf2d004a03ae1820ab5f8729670
Reviewed-on: https://go-review.googlesource.com/c/go/+/345486
Trust: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 293d036f6725c1d1e77bc43515d94fe418a9de0c..e92f71135c4630f7ced11d3bde55ca7a44936abc 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -7050,6 +7050,53 @@ func BenchmarkNew(b *testing.B) {
        })
 }
 
+func BenchmarkMap(b *testing.B) {
+       type V *int
+       value := ValueOf((V)(nil))
+       stringKeys := []string{}
+       mapOfStrings := map[string]V{}
+       uint64Keys := []uint64{}
+       mapOfUint64s := map[uint64]V{}
+       for i := 0; i < 100; i++ {
+               stringKey := fmt.Sprintf("key%d", i)
+               stringKeys = append(stringKeys, stringKey)
+               mapOfStrings[stringKey] = nil
+
+               uint64Key := uint64(i)
+               uint64Keys = append(uint64Keys, uint64Key)
+               mapOfUint64s[uint64Key] = nil
+       }
+
+       tests := []struct {
+               label          string
+               m, keys, value Value
+       }{
+               {"StringKeys", ValueOf(mapOfStrings), ValueOf(stringKeys), value},
+               {"Uint64Keys", ValueOf(mapOfUint64s), ValueOf(uint64Keys), value},
+       }
+
+       for _, tt := range tests {
+               b.Run(tt.label, func(b *testing.B) {
+                       b.Run("MapIndex", func(b *testing.B) {
+                               b.ReportAllocs()
+                               for i := 0; i < b.N; i++ {
+                                       for j := tt.keys.Len() - 1; j >= 0; j-- {
+                                               tt.m.MapIndex(tt.keys.Index(j))
+                                       }
+                               }
+                       })
+                       b.Run("SetMapIndex", func(b *testing.B) {
+                               b.ReportAllocs()
+                               for i := 0; i < b.N; i++ {
+                                       for j := tt.keys.Len() - 1; j >= 0; j-- {
+                                               tt.m.SetMapIndex(tt.keys.Index(j), tt.value)
+                                       }
+                               }
+                       })
+               })
+       }
+}
+
 func TestSwapper(t *testing.T) {
        type I int
        var a, b, c I

diff --git a/src/reflect/value.go b/src/reflect/value.go
index bf29d1bb3a8f34b4e8b2670e16525e01dc98ca80..6e9aaabe8a1298c9c7393016e96bbcf80fba1be9 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -1515,15 +1515,21 @@ func (v Value) MapIndex(key Value) Value {
        // considered unexported. This is consistent with the
        // behavior for structs, which allow read but not write
        // of unexported fields.
-       key = key.assignTo("reflect.Value.MapIndex", tt.key, nil)
 
-       var k unsafe.Pointer
-       if key.flag&flagIndir != 0 {
-               k = key.ptr
+       var e unsafe.Pointer
+       if key.kind() == String && tt.key.Kind() == String {
+               k := *(*string)(key.ptr)
+               e = mapaccess_faststr(v.typ, v.pointer(), k)
        } else {
-               k = unsafe.Pointer(&key.ptr)
+               key = key.assignTo("reflect.Value.MapIndex", tt.key, nil)
+               var k unsafe.Pointer
+               if key.flag&flagIndir != 0 {
+                       k = key.ptr
+               } else {
+                       k = unsafe.Pointer(&key.ptr)
+               }
+               e = mapaccess(v.typ, v.pointer(), k)
        }
-       e := mapaccess(v.typ, v.pointer(), k)
        if e == nil {
                return Value{}
        }
@@ -2121,6 +2127,25 @@ func (v Value) SetMapIndex(key, elem Value) {
        v.mustBeExported()
        key.mustBeExported()
        tt := (*mapType)(unsafe.Pointer(v.typ))
+
+       if key.kind() == String && tt.key.Kind() == String {
+               k := *(*string)(key.ptr)
+               if elem.typ == nil {
+                       mapdelete_faststr(v.typ, v.pointer(), k)
+                       return
+               }
+               elem.mustBeExported()
+               elem = elem.assignTo("reflect.Value.SetMapIndex", tt.elem, nil)
+               var e unsafe.Pointer
+               if elem.flag&flagIndir != 0 {
+                       e = elem.ptr
+               } else {
+                       e = unsafe.Pointer(&elem.ptr)
+               }
+               mapassign_faststr(v.typ, v.pointer(), k, e)
+               return
+       }
+
        key = key.assignTo("reflect.Value.SetMapIndex", tt.key, nil)
        var k unsafe.Pointer
        if key.flag&flagIndir != 0 {
@@ -3252,12 +3277,21 @@ func makemap(t *rtype, cap int) (m unsafe.Pointer)
 //go:noescape
 func mapaccess(t *rtype, m unsafe.Pointer, key unsafe.Pointer) (val unsafe.Pointer)
 
+//go:noescape
+func mapaccess_faststr(t *rtype, m unsafe.Pointer, key string) (val unsafe.Pointer)
+
 //go:noescape
 func mapassign(t *rtype, m unsafe.Pointer, key, val unsafe.Pointer)
 
+//go:noescape
+func mapassign_faststr(t *rtype, m unsafe.Pointer, key string, val unsafe.Pointer)
+
 //go:noescape
 func mapdelete(t *rtype, m unsafe.Pointer, key unsafe.Pointer)
 
+//go:noescape
+func mapdelete_faststr(t *rtype, m unsafe.Pointer, key string)
+
 //go:noescape
 func mapiterinit(t *rtype, m unsafe.Pointer, it *hiter)
 

diff --git a/src/runtime/map.go b/src/runtime/map.go
index 59b803d629586ba8555ecebc8a3054d79ed04b1c..985c297cd431742deb4383da64a26e961e31a2a9 100644 (file)
--- a/src/runtime/map.go
+++ b/src/runtime/map.go
@@ -1324,17 +1324,38 @@ func reflect_mapaccess(t *maptype, h *hmap, key unsafe.Pointer) unsafe.Pointer {
        return elem
 }
 
+//go:linkname reflect_mapaccess_faststr reflect.mapaccess_faststr
+func reflect_mapaccess_faststr(t *maptype, h *hmap, key string) unsafe.Pointer {
+       elem, ok := mapaccess2_faststr(t, h, key)
+       if !ok {
+               // reflect wants nil for a missing element
+               elem = nil
+       }
+       return elem
+}
+
 //go:linkname reflect_mapassign reflect.mapassign
 func reflect_mapassign(t *maptype, h *hmap, key unsafe.Pointer, elem unsafe.Pointer) {
        p := mapassign(t, h, key)
        typedmemmove(t.elem, p, elem)
 }
 
+//go:linkname reflect_mapassign_faststr reflect.mapassign_faststr
+func reflect_mapassign_faststr(t *maptype, h *hmap, key string, elem unsafe.Pointer) {
+       p := mapassign_faststr(t, h, key)
+       typedmemmove(t.elem, p, elem)
+}
+
 //go:linkname reflect_mapdelete reflect.mapdelete
 func reflect_mapdelete(t *maptype, h *hmap, key unsafe.Pointer) {
        mapdelete(t, h, key)
 }
 
+//go:linkname reflect_mapdelete_faststr reflect.mapdelete_faststr
+func reflect_mapdelete_faststr(t *maptype, h *hmap, key string) {
+       mapdelete_faststr(t, h, key)
+}
+
 //go:linkname reflect_mapiterinit reflect.mapiterinit
 func reflect_mapiterinit(t *maptype, h *hmap, it *hiter) {
        mapiterinit(t, h, it)