Skip to content

Commit c5edd5f

Browse files
dsnetgopherbot
authored andcommitted
reflect: make Value.MapRange inlineable
This allows the caller to decide whether MapIter should be stack allocated or heap allocated based on whether it escapes. In most cases, it does not escape and thus removes the utility of MapIter.Reset (#46293). In fact, use of sync.Pool with MapIter and calling MapIter.Reset is likely to be slower. Change-Id: Ic93e7d39e5dd4c83e7fca9e0bdfbbcd70777f0e1 Reviewed-on: https://go-review.googlesource.com/c/go/+/400675 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent f49e802 commit c5edd5f

File tree

3 files changed

+17
-4
lines changed

3 files changed

+17
-4
lines changed

src/cmd/compile/internal/test/inl_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ func TestIntendedInlining(t *testing.T) {
136136
"Value.CanSet",
137137
"Value.CanInterface",
138138
"Value.IsValid",
139+
"Value.MapRange",
139140
"Value.pointer",
140141
"add",
141142
"align",

src/reflect/all_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,11 @@ func TestMapIterSet(t *testing.T) {
370370
e.SetIterValue(iter)
371371
}
372372
}))
373-
// Making a *MapIter allocates. This should be the only allocation.
374-
if got != 1 {
375-
t.Errorf("wanted 1 alloc, got %d", got)
373+
// Calling MapRange should not allocate even though it returns a *MapIter.
374+
// The function is inlineable, so if the local usage does not escape
375+
// the *MapIter, it can remain stack allocated.
376+
if got != 0 {
377+
t.Errorf("wanted 0 alloc, got %d", got)
376378
}
377379
}
378380

src/reflect/value.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1833,10 +1833,20 @@ func (iter *MapIter) Reset(v Value) {
18331833
// ...
18341834
// }
18351835
func (v Value) MapRange() *MapIter {
1836-
v.mustBe(Map)
1836+
// This is inlinable to take advantage of "function outlining".
1837+
// The allocation of MapIter can be stack allocated if the caller
1838+
// does not allow it to escape.
1839+
// See https://blog.filippo.io/efficient-go-apis-with-the-inliner/
1840+
if v.kind() != Map {
1841+
v.panicNotMap()
1842+
}
18371843
return &MapIter{m: v}
18381844
}
18391845

1846+
func (f flag) panicNotMap() {
1847+
f.mustBe(Map)
1848+
}
1849+
18401850
// copyVal returns a Value containing the map key or value at ptr,
18411851
// allocating a new variable as needed.
18421852
func copyVal(typ *rtype, fl flag, ptr unsafe.Pointer) Value {

0 commit comments

Comments
 (0)