Skip to content

Commit

Permalink
reflect: ensure map keys match key type in MapIndex and SetMapIndex
Browse files Browse the repository at this point in the history
name                          old time/op    new time/op    delta
Map/StringKeys/MapIndex-8           2.36µs ± 5%    2.55µs ±11%  +7.98%  (p=0.006 n=10+9)
Map/StringKeys/SetMapIndex-8        4.86µs ± 7%    4.77µs ± 1%    ~     (p=0.211 n=10+9)
Map/StringKindKeys/MapIndex-8       2.29µs ± 3%    2.28µs ± 4%    ~     (p=0.631 n=10+10)
Map/StringKindKeys/SetMapIndex-8    4.44µs ± 3%    4.61µs ± 1%  +3.78%  (p=0.000 n=10+10)
Map/Uint64Keys/MapIndex-8           3.42µs ± 9%    3.11µs ± 2%  -9.20%  (p=0.000 n=10+9)
Map/Uint64Keys/SetMapIndex-8        5.17µs ± 3%    5.00µs ± 1%  -3.23%  (p=0.000 n=9+10)

Fixes #52379

Change-Id: I545c71ea3145280828ca4186aad036a6c02016ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/400635
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
  • Loading branch information
kortschak authored and gopherbot committed Apr 30, 2022
1 parent f2b6747 commit 11a650b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
12 changes: 12 additions & 0 deletions src/reflect/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,10 @@ func TestMap(t *testing.T) {
if m != nil {
t.Errorf("mv.Set(nil) failed")
}

type S string
shouldPanic("not assignable", func() { mv.MapIndex(ValueOf(S("key"))) })
shouldPanic("not assignable", func() { mv.SetMapIndex(ValueOf(S("key")), ValueOf(0)) })
}

func TestNilMap(t *testing.T) {
Expand Down Expand Up @@ -7265,11 +7269,14 @@ func BenchmarkNew(b *testing.B) {

func BenchmarkMap(b *testing.B) {
type V *int
type S string
value := ValueOf((V)(nil))
stringKeys := []string{}
mapOfStrings := map[string]V{}
uint64Keys := []uint64{}
mapOfUint64s := map[uint64]V{}
userStringKeys := []S{}
mapOfUserStrings := map[S]V{}
for i := 0; i < 100; i++ {
stringKey := fmt.Sprintf("key%d", i)
stringKeys = append(stringKeys, stringKey)
Expand All @@ -7278,6 +7285,10 @@ func BenchmarkMap(b *testing.B) {
uint64Key := uint64(i)
uint64Keys = append(uint64Keys, uint64Key)
mapOfUint64s[uint64Key] = nil

userStringKey := S(fmt.Sprintf("key%d", i))
userStringKeys = append(userStringKeys, userStringKey)
mapOfUserStrings[userStringKey] = nil
}

tests := []struct {
Expand All @@ -7286,6 +7297,7 @@ func BenchmarkMap(b *testing.B) {
}{
{"StringKeys", ValueOf(mapOfStrings), ValueOf(stringKeys), value},
{"Uint64Keys", ValueOf(mapOfUint64s), ValueOf(uint64Keys), value},
{"UserStringKeys", ValueOf(mapOfUserStrings), ValueOf(userStringKeys), value},
}

for _, tt := range tests {
Expand Down
6 changes: 4 additions & 2 deletions src/reflect/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,8 @@ func (v Value) lenNonSlice() int {
panic(&ValueError{"reflect.Value.Len", v.kind()})
}

var stringType = TypeOf("").(*rtype)

// MapIndex returns the value associated with key in the map v.
// It panics if v's Kind is not Map.
// It returns the zero Value if key is not found in the map or if v represents a nil map.
Expand All @@ -1652,7 +1654,7 @@ func (v Value) MapIndex(key Value) Value {
// of unexported fields.

var e unsafe.Pointer
if key.kind() == String && tt.key.Kind() == String && tt.elem.size <= maxValSize {
if (tt.key == stringType || key.kind() == String) && tt.key == key.typ && tt.elem.size <= maxValSize {
k := *(*string)(key.ptr)
e = mapaccess_faststr(v.typ, v.pointer(), k)
} else {
Expand Down Expand Up @@ -2278,7 +2280,7 @@ func (v Value) SetMapIndex(key, elem Value) {
key.mustBeExported()
tt := (*mapType)(unsafe.Pointer(v.typ))

if key.kind() == String && tt.key.Kind() == String && tt.elem.size <= maxValSize {
if (tt.key == stringType || key.kind() == String) && tt.key == key.typ && tt.elem.size <= maxValSize {
k := *(*string)(key.ptr)
if elem.typ == nil {
mapdelete_faststr(v.typ, v.pointer(), k)
Expand Down

0 comments on commit 11a650b

Please sign in to comment.