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>
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
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")
}
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")
}
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")
}
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")
}
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.
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
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")
}
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")
}
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")
}
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")
}
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.
//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)
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
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
}
// 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)
}
}
// 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)
}