proposal: reflect: add direct MapIter accessors
ianlancetaylor opened this issue · 5 comments
Proposal Details
Currently if a map uses integer keys, a call to (*reflect.MapIter).Key
has to allocate memory in order to return a reflect.Value
. If the caller knows that the key is an integer, they have to call mapiter.Key().Int()
to retrieve that integer. That allocates new memory, stores an integer to it, then fetches it, and then throws the memory away.
The same applies to (*reflect.MapIter).Value
as well.
This is enough of a problem that the code in https://github.com/gonum/gonum/blob/master/graph/iterator/map.go includes a copy of map iterators and calls directly into unexported reflect and runtime package functions. Obviously that code should not do that, as it is unsupported and may break in future Go releases. But they aren't doing it for fun, they are doing it because it makes a measurable difference in their benchmarks.
I propose that we add direct accessors to reflect.MapIter
to give people a standard way to write more efficient code.
// KeyInt returns the key of iter's current map entry, as an int64.
// It panics if the map key type Kind is not [Int], [Int8], [Int16], [Int32], or [Int64].
func (iter *MapIter) KeyInt() int64
// KeyUint returns the key of iter's current map entry, as a uint64.
// It panics if the map key type Kind is not [Uint], [Uintptr], [Uint8], [Uint16], [Uint32], or [Uint64].
func (iter *MapIter) KeyUint() uint64
// KeyFloat returns the key of iter's current map entry, as a float64.
// It panics if the map key type Kind is not [Float32] or [Float64].
func (iter *MapIter) KeyFloat() float64
// KeyComplex returns the key of iter's current map entry, as a complex128.
// It panics if the map key type Kind is not [Complex64] or [Complex128].
func (iter *MapIter) KeyComplex() complex128
// KeyString returns the key of iter's current map entry, as a string.
// It panics if the map key type Kind is not [String].
func (iter *MapIter) KeyString() string
// ValueInt returns the value of iter's current map entry, as an int64.
// It panics if the map value type Kind is not [Int], [Int8], [Int16], [Int32], or [Int64].
func (iter *MapIter) ValueInt() int64
// ValueUint returns the value of iter's current map entry, as a uint64.
// It panics if the map value type Kind is not [Uint], [Uintptr], [Uint8], [Uint16], [Uint32], or [Uint64].
func (iter *MapIter) ValueUint() uint64
// ValueFloat returns the value of iter's current map entry, as a float64.
// It panics if the map value type Kind is not [Float32] or [Float64].
func (iter *MapIter) ValueFloat() float64
// ValueComplex returns the value of iter's current map entry, as a complex128.
// It panics if the map value type Kind is not [Complex64] or [Complex128].
func (iter *MapIter) ValueComplex() complex128
// ValueString returns the value of iter's current map entry, as a string.
// It panics if the map value type Kind is not [String].
func (iter *MapIter) ValueString() string
A Pointer
method is not necessary, as the Key
and Value
methods are reasonably efficient for pointer values. But we could add one for consistency if we want.
We added (Value).SetIterKey
and (Value).SetIterValue
a while ago. #48294. Does this not satisfy this need?
Some related discussion at gonum/gonum#1937 (comment).
@randall77 Ah, thanks, I misunderstood how those work. Let me see how it affects the benchmarks.
In the v2 JSON code, I make use of Value.SetIterKey
and Value.SetIterValue
to avoid this problem. It does require that I allocate at least two reflect.Value
s (one for the key and value), but then I get to share it across all items in the Go map. Thus, the allocation cost is fixed per map.
The proposed API (which is a relatively large API surface) would theoretically avoid all allocations for common kinds, but I'm not sure it's worth the cost.
It's still not quite as fast as the unsafe code, but retracting this proposal. Thanks.