Skip to content

Commit e060266

Browse files
jpbetzjpbetz
committed
Directly reuse objects objects instead of relying on sync.Pool when looping
1 parent 925b736 commit e060266

File tree

5 files changed

+121
-36
lines changed

5 files changed

+121
-36
lines changed

‎value/map.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,16 @@ func MapCompare(lhs, rhs Map) int {
8181
}
8282
litem, _ := lhs.Get(lorder[i])
8383
ritem, _ := rhs.Get(rorder[i])
84-
if c := Compare(litem, ritem); c != 0 {
84+
c := Compare(litem, ritem)
85+
if litem != nil {
86+
litem.Recycle()
87+
}
88+
if ritem != nil {
89+
ritem.Recycle()
90+
}
91+
if c != 0 {
8592
return c
8693
}
87-
litem.Recycle()
88-
ritem.Recycle()
8994
// The items are equal; continue.
9095
i++
9196
}
@@ -103,11 +108,11 @@ func MapEquals(lhs, rhs Map) bool {
103108
if !ok {
104109
return false
105110
}
106-
if !Equals(v, vo) {
107-
vo.Recycle()
111+
equal := Equals(v, vo)
112+
vo.Recycle()
113+
if !equal {
108114
return false
109115
}
110-
vo.Recycle()
111116
return true
112117
})
113118
}

‎value/mapreflect.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ limitations under the License.
1616

1717
package value
1818

19-
import "reflect"
19+
import (
20+
"reflect"
21+
)
2022

2123
type mapReflect struct {
2224
valueReflect
@@ -61,10 +63,10 @@ func (r mapReflect) toMapKey(key string) reflect.Value {
6163
}
6264

6365
func (r mapReflect) Iterate(fn func(string, Value) bool) bool {
66+
vp := newTempValuePooler()
67+
defer vp.Recycle()
6468
return eachMapEntry(r.Value, func(s string, value reflect.Value) bool {
65-
mapVal := mustWrapValueReflect(value)
66-
defer mapVal.Recycle()
67-
return fn(s, mapVal)
69+
return fn(s, vp.NewValueReflect(value))
6870
})
6971
}
7072

‎value/mapunstructured.go

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,19 @@ func (m mapUnstructuredInterface) Delete(key string) {
4040
}
4141

4242
func (m mapUnstructuredInterface) Iterate(fn func(key string, value Value) bool) bool {
43+
if len(m) == 0 {
44+
return true
45+
}
46+
vv := viPool.Get().(*valueUnstructured)
47+
defer vv.Recycle()
4348
for k, v := range m {
4449
if ks, ok := k.(string); !ok {
4550
continue
4651
} else {
47-
vv := NewValueInterface(v)
52+
vv.Value = v
4853
if !fn(ks, vv) {
49-
vv.Recycle()
5054
return false
5155
}
52-
vv.Recycle()
5356
}
5457
}
5558
return true
@@ -63,6 +66,11 @@ func (m mapUnstructuredInterface) Equals(other Map) bool {
6366
if m.Length() != other.Length() {
6467
return false
6568
}
69+
if len(m) == 0 {
70+
return true
71+
}
72+
vv := viPool.Get().(*valueUnstructured)
73+
defer vv.Recycle()
6674
for k, v := range m {
6775
ks, ok := k.(string)
6876
if !ok {
@@ -72,14 +80,12 @@ func (m mapUnstructuredInterface) Equals(other Map) bool {
7280
if !ok {
7381
return false
7482
}
75-
vv := NewValueInterface(v)
83+
vv.Value = v
7684
if !Equals(vv, vo) {
77-
vv.Recycle()
7885
vo.Recycle()
7986
return false
8087
}
8188
vo.Recycle()
82-
vv.Recycle()
8389
}
8490
return true
8591
}
@@ -108,13 +114,16 @@ func (m mapUnstructuredString) Delete(key string) {
108114
}
109115

110116
func (m mapUnstructuredString) Iterate(fn func(key string, value Value) bool) bool {
117+
if len(m) == 0 {
118+
return true
119+
}
120+
vv := viPool.Get().(*valueUnstructured)
121+
defer vv.Recycle()
111122
for k, v := range m {
112-
vv := NewValueInterface(v)
123+
vv.Value = v
113124
if !fn(k, vv) {
114-
vv.Recycle()
115125
return false
116126
}
117-
vv.Recycle()
118127
}
119128
return true
120129
}
@@ -127,19 +136,22 @@ func (m mapUnstructuredString) Equals(other Map) bool {
127136
if m.Length() != other.Length() {
128137
return false
129138
}
139+
if len(m) == 0 {
140+
return true
141+
}
142+
vv := viPool.Get().(*valueUnstructured)
143+
defer vv.Recycle()
130144
for k, v := range m {
131145
vo, ok := other.Get(k)
132146
if !ok {
133147
return false
134148
}
135-
vv := NewValueInterface(v)
149+
vv.Value = v
136150
if !Equals(vv, vo) {
137151
vo.Recycle()
138-
vv.Recycle()
139152
return false
140153
}
141154
vo.Recycle()
142-
vv.Recycle()
143155
}
144156
return true
145157
}

‎value/structreflect.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,10 @@ func (r structReflect) update(fieldEntry *fieldCacheEntry, key string, oldVal, n
198198
}
199199

200200
func (r structReflect) Iterate(fn func(string, Value) bool) bool {
201+
vp := newTempValuePooler()
202+
defer vp.Recycle()
201203
return eachStructField(r.Value, func(s string, value reflect.Value) bool {
202-
v := mustWrapValueReflect(value)
203-
defer v.Recycle()
204-
return fn(s, v)
204+
return fn(s, vp.NewValueReflect(value))
205205
})
206206
}
207207

@@ -239,13 +239,15 @@ func (r structReflect) Equals(m Map) bool {
239239
}
240240
structCacheEntry := getStructCacheEntry(r.Value.Type())
241241

242+
vp := newTempValuePooler()
243+
defer vp.Recycle()
242244
return m.Iterate(func(s string, value Value) bool {
243245
fieldCacheEntry, ok := structCacheEntry[s]
244246
if !ok {
245247
return false
246248
}
247249
lhsVal := fieldCacheEntry.getFieldFromStruct(r.Value)
248-
return Equals(mustWrapValueReflect(lhsVal), value)
250+
return Equals(vp.NewValueReflect(lhsVal), value)
249251
})
250252
}
251253

‎value/valuereflect.go

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ func wrapValueReflect(parentMap, parentMapKey *reflect.Value, value reflect.Valu
5252
// TODO: conversion of json.Marshaller interface types is expensive. This can be mostly optimized away by
5353
// introducing conversion functions that do not require going through JSON and using those here.
5454
if marshaler, ok := getMarshaler(value); ok {
55-
return toUnstructured(marshaler, value)
55+
vv := viPool.Get().(*valueUnstructured)
56+
return toUnstructured(vv, marshaler, value)
5657
}
5758
value = dereference(value)
5859
val := reflectPool.Get().(*valueReflect)
@@ -259,7 +260,7 @@ var (
259260
falseBytes = []byte("false")
260261
)
261262

262-
func toUnstructured(marshaler json.Marshaler, sv reflect.Value) (Value, error) {
263+
func toUnstructured(into *valueUnstructured, marshaler json.Marshaler, sv reflect.Value) (Value, error) {
263264
data, err := marshaler.MarshalJSON()
264265
if err != nil {
265266
return nil, err
@@ -270,37 +271,37 @@ func toUnstructured(marshaler json.Marshaler, sv reflect.Value) (Value, error) {
270271

271272
case bytes.Equal(data, nullBytes):
272273
// We're done - we don't need to store anything.
273-
return NewValueInterface(nil), nil
274+
into.Value = nil
274275

275276
case bytes.Equal(data, trueBytes):
276-
return NewValueInterface(true), nil
277+
into.Value = true
277278

278279
case bytes.Equal(data, falseBytes):
279-
return NewValueInterface(false), nil
280+
into.Value = false
280281

281282
case data[0] == '"':
282283
var result string
283284
err := json.Unmarshal(data, &result)
284285
if err != nil {
285286
return nil, fmt.Errorf("error decoding string from json: %v", err)
286287
}
287-
return NewValueInterface(result), nil
288+
into.Value = result
288289

289290
case data[0] == '{':
290291
result := make(map[string]interface{})
291292
err := json.Unmarshal(data, &result)
292293
if err != nil {
293294
return nil, fmt.Errorf("error decoding object from json: %v", err)
294295
}
295-
return NewValueInterface(result), nil
296+
into.Value = result
296297

297298
case data[0] == '[':
298299
result := make([]interface{}, 0)
299300
err := json.Unmarshal(data, &result)
300301
if err != nil {
301302
return nil, fmt.Errorf("error decoding array from json: %v", err)
302303
}
303-
return NewValueInterface(result), nil
304+
into.Value = result
304305

305306
default:
306307
var (
@@ -309,11 +310,74 @@ func toUnstructured(marshaler json.Marshaler, sv reflect.Value) (Value, error) {
309310
err error
310311
)
311312
if err = json.Unmarshal(data, &resultInt); err == nil {
312-
return NewValueInterface(resultInt), nil
313+
into.Value = resultInt
314+
return into, nil
313315
}
314316
if err = json.Unmarshal(data, &resultFloat); err == nil {
315-
return NewValueInterface(resultFloat), nil
317+
into.Value = resultFloat
318+
return into, nil
316319
}
317320
return nil, fmt.Errorf("error decoding number from json: %v", err)
318321
}
322+
return into, nil
323+
}
324+
325+
// tempValuePooler manages sync.Pool usage for reflect iterators that need either a temporary valueReflect or a
326+
// valueUnstructured for each iterator callback invocation. Directly reusing value objects for each iterator callback
327+
// invocation is much more efficient than using getting and putting objects to a sync.Pool.
328+
type tempValuePooler struct {
329+
vr *valueReflect
330+
vu *valueUnstructured
331+
}
332+
333+
func newTempValuePooler() *tempValuePooler {
334+
return &tempValuePooler{}
335+
}
336+
337+
func (ir *tempValuePooler) pooledValueReflect() *valueReflect {
338+
if ir.vr == nil {
339+
ir.vr = reflectPool.Get().(*valueReflect)
340+
}
341+
return ir.vr
342+
}
343+
344+
func (ir *tempValuePooler) pooledValueInterface() *valueUnstructured {
345+
if ir.vu == nil {
346+
ir.vu = viPool.Get().(*valueUnstructured)
347+
}
348+
return ir.vu
349+
}
350+
351+
// Recycle calls Recycle on all underlying pooled value objects.
352+
func (ir *tempValuePooler) Recycle() {
353+
if ir.vr != nil {
354+
ir.vr.Recycle()
355+
}
356+
if ir.vu != nil {
357+
ir.vu.Recycle()
358+
}
359+
}
360+
361+
// NewValueReflect returns a Value wrapping the given reflect.Value. The returned Value is valid only temporarily.
362+
// The returned Value becomes invalid on the next NewValueReflect call.
363+
func (ir *tempValuePooler) NewValueReflect(value reflect.Value) Value {
364+
if marshaler, ok := getMarshaler(value); ok {
365+
vi := ir.pooledValueInterface()
366+
marshaled, err := toUnstructured(vi, marshaler, value)
367+
if err != nil {
368+
panic(err)
369+
}
370+
return marshaled
371+
}
372+
vr := ir.pooledValueReflect()
373+
vr.Value = dereference(value)
374+
return vr
375+
}
376+
377+
// NewValueInterface returns a Value wrapping the given interface{}. The returned Value is valid only temporarily.
378+
// // The returned Value becomes invalid on the next NewValueInterface call.
379+
func (ir *tempValuePooler) NewValueInterface(value interface{}) Value {
380+
vi := ir.pooledValueInterface()
381+
vi.Value = value
382+
return vi
319383
}

0 commit comments

Comments
 (0)