1
0
mirror of https://github.com/golang/go synced 2024-11-11 16:51:50 -07:00

reflect: remove calling mapiterkey, mapiterelem

It makes use of the hiter structure which matches runtime.hiter's.

This change mainly improves the performance of Next method of MapIter.

goos: darwin
goarch: arm64
pkg: reflect
cpu: Apple M2
              │  ./old.txt  │              ./new.txt              │
              │   sec/op    │   sec/op     vs base                │
MapIterNext-8   61.95n ± 0%   54.95n ± 0%  -11.28% (p=0.000 n=10)

for the change of `test/escape_reflect.go`:
removing mapiterkey, mapiterelem would cause leaking MapIter content
when calling SetIterKey and SetIterValue,
and this may cause map bucket to be allocated on heap instead of stack.
Reproduce:
```
{
  m := map[int]int{1: 2} // escapes to heap after this change
  it := reflect.ValueOf(m).MapRange()
  it.Next()
  var k, v int
  reflect.ValueOf(&k).Elem().SetIterKey(it)
  reflect.ValueOf(&v).Elem().SetIterValue(it)
  println(k, v)
}
```
This CL would not introduce abi.NoEscape to fix this. It may need futher
optimization and tests on hiter field usage and its escape analysis.

Fixes #69416

Change-Id: Ibaa33bcf86228070b4a505b9512680791aa59f04
Reviewed-on: https://go-review.googlesource.com/c/go/+/612616
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Kyle Xiao 2024-09-12 22:52:56 +08:00 committed by Gopher Robot
parent c71b5ff76a
commit 7ba074fe43
5 changed files with 18 additions and 24 deletions

View File

@ -223,7 +223,7 @@ func (v Value) MapKeys() []Value {
a := make([]Value, mlen)
var i int
for i = 0; i < len(a); i++ {
key := mapiterkey(&it)
key := it.key
if key == nil {
// Someone deleted an entry from the map since we
// called maplen above. It's a data race, but nothing
@ -274,7 +274,7 @@ func (iter *MapIter) Key() Value {
if !iter.hiter.initialized() {
panic("MapIter.Key called before Next")
}
iterkey := mapiterkey(&iter.hiter)
iterkey := iter.hiter.key
if iterkey == nil {
panic("MapIter.Key called on exhausted iterator")
}
@ -292,7 +292,7 @@ func (v Value) SetIterKey(iter *MapIter) {
if !iter.hiter.initialized() {
panic("reflect: Value.SetIterKey called before Next")
}
iterkey := mapiterkey(&iter.hiter)
iterkey := iter.hiter.key
if iterkey == nil {
panic("reflect: Value.SetIterKey called on exhausted iterator")
}
@ -317,7 +317,7 @@ func (iter *MapIter) Value() Value {
if !iter.hiter.initialized() {
panic("MapIter.Value called before Next")
}
iterelem := mapiterelem(&iter.hiter)
iterelem := iter.hiter.elem
if iterelem == nil {
panic("MapIter.Value called on exhausted iterator")
}
@ -335,7 +335,7 @@ func (v Value) SetIterValue(iter *MapIter) {
if !iter.hiter.initialized() {
panic("reflect: Value.SetIterValue called before Next")
}
iterelem := mapiterelem(&iter.hiter)
iterelem := iter.hiter.elem
if iterelem == nil {
panic("reflect: Value.SetIterValue called on exhausted iterator")
}
@ -365,12 +365,12 @@ func (iter *MapIter) Next() bool {
if !iter.hiter.initialized() {
mapiterinit(iter.m.typ(), iter.m.pointer(), &iter.hiter)
} else {
if mapiterkey(&iter.hiter) == nil {
if iter.hiter.key == nil {
panic("MapIter.Next called on exhausted iterator")
}
mapiternext(&iter.hiter)
}
return mapiterkey(&iter.hiter) != nil
return iter.hiter.key != nil
}
// Reset modifies iter to iterate over v.

View File

@ -224,7 +224,7 @@ func (v Value) MapKeys() []Value {
a := make([]Value, mlen)
var i int
for i = 0; i < len(a); i++ {
key := mapiterkey(&it)
key := it.key
if key == nil {
// Someone deleted an entry from the map since we
// called maplen above. It's a data race, but nothing
@ -275,7 +275,7 @@ func (iter *MapIter) Key() Value {
if !iter.hiter.initialized() {
panic("MapIter.Key called before Next")
}
iterkey := mapiterkey(&iter.hiter)
iterkey := iter.hiter.key
if iterkey == nil {
panic("MapIter.Key called on exhausted iterator")
}
@ -293,7 +293,7 @@ func (v Value) SetIterKey(iter *MapIter) {
if !iter.hiter.initialized() {
panic("reflect: Value.SetIterKey called before Next")
}
iterkey := mapiterkey(&iter.hiter)
iterkey := iter.hiter.key
if iterkey == nil {
panic("reflect: Value.SetIterKey called on exhausted iterator")
}
@ -318,7 +318,7 @@ func (iter *MapIter) Value() Value {
if !iter.hiter.initialized() {
panic("MapIter.Value called before Next")
}
iterelem := mapiterelem(&iter.hiter)
iterelem := iter.hiter.elem
if iterelem == nil {
panic("MapIter.Value called on exhausted iterator")
}
@ -336,7 +336,7 @@ func (v Value) SetIterValue(iter *MapIter) {
if !iter.hiter.initialized() {
panic("reflect: Value.SetIterValue called before Next")
}
iterelem := mapiterelem(&iter.hiter)
iterelem := iter.hiter.elem
if iterelem == nil {
panic("reflect: Value.SetIterValue called on exhausted iterator")
}
@ -366,12 +366,12 @@ func (iter *MapIter) Next() bool {
if !iter.hiter.initialized() {
mapiterinit(iter.m.typ(), iter.m.pointer(), &iter.hiter)
} else {
if mapiterkey(&iter.hiter) == nil {
if iter.hiter.key == nil {
panic("MapIter.Next called on exhausted iterator")
}
mapiternext(&iter.hiter)
}
return mapiterkey(&iter.hiter) != nil
return iter.hiter.key != nil
}
// Reset modifies iter to iterate over v.

View File

@ -3596,12 +3596,6 @@ func mapdelete_faststr(t *abi.Type, m unsafe.Pointer, key string)
//go:noescape
func mapiterinit(t *abi.Type, m unsafe.Pointer, it *hiter)
//go:noescape
func mapiterkey(it *hiter) (key unsafe.Pointer)
//go:noescape
func mapiterelem(it *hiter) (elem unsafe.Pointer)
//go:noescape
func mapiternext(it *hiter)

View File

@ -1528,7 +1528,7 @@ func reflect_mapiternext(it *hiter) {
mapiternext(it)
}
// reflect_mapiterkey is for package reflect,
// reflect_mapiterkey was for package reflect,
// but widely used packages access it using linkname.
// Notable members of the hall of shame include:
// - github.com/goccy/go-json
@ -1542,7 +1542,7 @@ func reflect_mapiterkey(it *hiter) unsafe.Pointer {
return it.key
}
// reflect_mapiterelem is for package reflect,
// reflect_mapiterelem was for package reflect,
// but widely used packages access it using linkname.
// Notable members of the hall of shame include:
// - github.com/goccy/go-json

View File

@ -423,7 +423,7 @@ func mapdelete(m map[string]string, k string) { // ERROR "m does not escape" "le
}
// Unfortunate: v doesn't need to leak.
func setiterkey1(v reflect.Value, it *reflect.MapIter) { // ERROR "leaking param: v$" "it does not escape"
func setiterkey1(v reflect.Value, it *reflect.MapIter) { // ERROR "leaking param: v$" "leaking param content: it$"
v.SetIterKey(it)
}
@ -434,7 +434,7 @@ func setiterkey2(v reflect.Value, m map[string]string) { // ERROR "leaking param
}
// Unfortunate: v doesn't need to leak.
func setitervalue1(v reflect.Value, it *reflect.MapIter) { // ERROR "leaking param: v$" "it does not escape"
func setitervalue1(v reflect.Value, it *reflect.MapIter) { // ERROR "leaking param: v$" "leaking param content: it$"
v.SetIterValue(it)
}