golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.94k stars 17.66k forks source link

reflect: remove calling mapiterkey and mapiterelem #69416

Closed xiaost closed 1 month ago

xiaost commented 1 month ago

Hi, Go team.

I'm working on optimizing iterating over map for an internal data encoding project. And in our case, I found that one of the bottlenecks is reflect.MapIter .Next. It can be improved by removing calling mapiterkey.

coz we have already copiedhiter struct from runtime for performance concern. To make it further, we can use key and elem of hiter struct directly without calling mapiterkey and mapiterelem

This mainly impacts the Next method of reflect.MapIter. I ran the BenchmarkMapIterNext benchmark in reflect/benchmark_test.go, and here is the output:

goos: darwin
goarch: arm64
pkg: reflect
cpu: Apple M2 Pro
               │  ./old.txt  │              ./new.txt              │
               │   sec/op    │   sec/op     vs base                │
MapIterNext-12   61.44n ± 1%   54.51n ± 1%  -11.29% (p=0.000 n=10)

I would like to raise CR if you're OK with it.

gabyhelp commented 1 month ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

earthboundkid commented 1 month ago

In general, performance improvements with no user visible effect can just be submitted. A proposal doesn't need to be approved first unless it's doing something unusual to get the performance.

gopherbot commented 1 month ago

Change https://go.dev/cl/612616 mentions this issue: reflect: remove calling mapiterkey, mapiterelem

ianlancetaylor commented 1 month ago

Taking this out of the proposal process.

gopherbot commented 1 month ago

Change https://go.dev/cl/613395 mentions this issue: reflect: allow inlining mapiterkey/mapiterelem